From 3c8a51bdbf52a921b11a5cbb367f9698d6b14128 Mon Sep 17 00:00:00 2001 From: Lucas Bajolet Date: Wed, 14 Aug 2024 16:12:53 -0400 Subject: [PATCH] packer_test: split BuildPackerPlugin in build/get The interface for building a plugin through the test suite was confusing, as it would build the plugin and return its path, cache the path for the version built, and return the path regardless if something was built or not. While in the current state this is harmless as builds are idempotent, since the state of the plugin package/module does not change, this will in the future as we introduce customisation techniques on the plugin's directory and files, making this double-use potentially dangerous. Furthermore, the current behaviour is unclear, as the function hides that caching mechanism, which could come as a surprise for users attempting to build a plugin for the duration of a test, while the built plugin is linked to the test suite being run, and not the unit test being evaluated. Therefore this commit changes the sequence in which plugins are built and used. Now the `CompilePlugin` function builds a plugin, and does not return its path anymore, instead terminating the tests immediately if they fail. In normal test usage, a new `GetPluginPath` function is introduced, which looks-up the path in the suite's cache, failing immediately if invoked before the plugin is built. With this change, it is heavily advised to build plugins when initialising the suite, then in the tests, the GetPluginPath function should be used to get a plugin's path for interacting with packer commands. --- packer_test/lib/plugin.go | 52 ++++++++++++++++-------- packer_test/lib/suite.go | 2 +- packer_test/plugin_tests/init_test.go | 2 +- packer_test/plugin_tests/install_test.go | 8 ++-- packer_test/plugin_tests/loading_test.go | 8 ++-- 5 files changed, 45 insertions(+), 27 deletions(-) diff --git a/packer_test/lib/plugin.go b/packer_test/lib/plugin.go index bdc6a7ac3..2d3c379c6 100644 --- a/packer_test/lib/plugin.go +++ b/packer_test/lib/plugin.go @@ -61,9 +61,24 @@ func ExpectedInstalledName(versionStr string) string { runtime.GOOS, runtime.GOARCH, ext) } -// BuildSimplePlugin creates a plugin that essentially does nothing. +// GetPluginPath gets the path for a pre-compiled plugin in the current test suite. // -// The plugin's code is contained in a subdirectory of this, and lets us +// The version only is needed, as the path to a compiled version of the tester +// plugin will be returned, so it can be installed after the fact. +// +// If the version requested does not exist, the function will panic. +func (ts *PackerTestSuite) GetPluginPath(t *testing.T, version string) string { + path, ok := ts.compiledPlugins.Load(version) + if !ok { + t.Fatalf("tester plugin in version %q was not built, either build it during suite init, or with BuildTestPlugin", version) + } + + return path.(string) +} + +// CompilePlugin builds a tester plugin with the specified version. +// +// The plugin's code is contained in a subdirectory of this file, and lets us // change the attributes of the plugin binary itself, like the SDK version, // the plugin's version, etc. // @@ -73,16 +88,24 @@ func ExpectedInstalledName(versionStr string) string { // // The path to the plugin is returned, it won't be removed automatically // though, deletion is the caller's responsibility. -func (ts *PackerTestSuite) BuildSimplePlugin(versionString string, t *testing.T) string { - // Only build plugin binary if not already done beforehand - path, ok := ts.compiledPlugins.Load(versionString) - if ok { - return path.(string) +// +// Note: each tester plugin may only be compiled once for a specific version in +// a test suite. The version may include core (mandatory), pre-release and +// metadata. Unlike Packer core, metadata does matter for the version being built. +func (ts *PackerTestSuite) CompilePlugin(t *testing.T, versionString string) { + // Fail to build plugin if already built. + // + // Especially with customisations being a thing, relying on cache to get and + // build a plugin at once means that the function is not idempotent anymore, + // and therefore we cannot rely on it being called twice and producing the + // same result, so we forbid it. + if _, ok := ts.compiledPlugins.Load(versionString); ok { + t.Fatalf("plugin version %q was already built, use GetTestPlugin instead", versionString) } v := version.Must(version.NewSemver(versionString)) - t.Logf("Building plugin in version %v", v) + t.Logf("Building tester plugin in version %v", v) testDir, err := currentDir() if err != nil { @@ -99,8 +122,6 @@ func (ts *PackerTestSuite) BuildSimplePlugin(versionString string, t *testing.T) } ts.compiledPlugins.Store(v.String(), outBin) - - return outBin } // MakePluginDir installs a list of plugins into a temporary directory and returns its path @@ -112,8 +133,8 @@ func (ts *PackerTestSuite) BuildSimplePlugin(versionString string, t *testing.T) func (ts *PackerTestSuite) MakePluginDir(pluginVersions ...string) (pluginTempDir string, cleanup func()) { t := ts.T() - for _, ver := range pluginVersions { - ts.BuildSimplePlugin(ver, t) + for _, version := range pluginVersions { + _ = ts.GetPluginPath(t, version) } var err error @@ -133,11 +154,8 @@ func (ts *PackerTestSuite) MakePluginDir(pluginVersions ...string) (pluginTempDi } for _, pluginVersion := range pluginVersions { - path, ok := ts.compiledPlugins.Load(pluginVersion) - if !ok { - err = fmt.Errorf("failed to get path to version %q, was it compiled?", pluginVersion) - } - cmd := ts.PackerCommand().SetArgs("plugins", "install", "--path", path.(string), "github.com/hashicorp/tester").AddEnv("PACKER_PLUGIN_PATH", pluginTempDir) + path := ts.GetPluginPath(t, pluginVersion) + cmd := ts.PackerCommand().SetArgs("plugins", "install", "--path", path, "github.com/hashicorp/tester").AddEnv("PACKER_PLUGIN_PATH", pluginTempDir) cmd.Assert(MustSucceed()) out, stderr, cmdErr := cmd.Run() if cmdErr != nil { diff --git a/packer_test/lib/suite.go b/packer_test/lib/suite.go index ed2d10969..d9d3a0221 100644 --- a/packer_test/lib/suite.go +++ b/packer_test/lib/suite.go @@ -34,7 +34,7 @@ func (ts *PackerTestSuite) buildPluginVersion(waitgroup *sync.WaitGroup, version waitgroup.Add(1) go func() { defer waitgroup.Done() - ts.BuildSimplePlugin(versionString, t) + ts.CompilePlugin(t, versionString) }() } diff --git a/packer_test/plugin_tests/init_test.go b/packer_test/plugin_tests/init_test.go index e5c3fa464..b7f53e304 100644 --- a/packer_test/plugin_tests/init_test.go +++ b/packer_test/plugin_tests/init_test.go @@ -55,7 +55,7 @@ func (ts *PackerPluginTestSuite) TestPackerInitWithNonGithubSource() { ts.Run("manually install plugin to the expected source", func() { ts.PackerCommand().UsePluginDir(pluginPath). - SetArgs("plugins", "install", "--path", ts.BuildSimplePlugin("1.0.10", ts.T()), "hubgit.com/hashicorp/tester"). + SetArgs("plugins", "install", "--path", ts.GetPluginPath(ts.T(), "1.0.10"), "hubgit.com/hashicorp/tester"). Assert(lib.MustSucceed(), lib.Grep("packer-plugin-tester_v1.0.10", lib.GrepStdout)) }) diff --git a/packer_test/plugin_tests/install_test.go b/packer_test/plugin_tests/install_test.go index b32d6223e..80576b327 100644 --- a/packer_test/plugin_tests/install_test.go +++ b/packer_test/plugin_tests/install_test.go @@ -8,7 +8,7 @@ func (ts *PackerPluginTestSuite) TestInstallPluginWithMetadata() { ts.Run("install plugin with metadata in version", func() { ts.PackerCommand().UsePluginDir(tempPluginDir). - SetArgs("plugins", "install", "--path", ts.BuildSimplePlugin("1.0.0+metadata", ts.T()), "github.com/hashicorp/tester"). + SetArgs("plugins", "install", "--path", ts.GetPluginPath(ts.T(), "1.0.0+metadata"), "github.com/hashicorp/tester"). Assert(lib.MustSucceed(), lib.Grep("Successfully installed plugin", lib.GrepStdout)) }) @@ -37,13 +37,13 @@ func (ts *PackerPluginTestSuite) TestInstallPluginWithPath() { ts.Run("install plugin with pre-release only", func() { ts.PackerCommand().UsePluginDir(tempPluginDir). - SetArgs("plugins", "install", "--path", ts.BuildSimplePlugin("1.0.0-dev", ts.T()), "github.com/hashicorp/tester"). + SetArgs("plugins", "install", "--path", ts.GetPluginPath(ts.T(), "1.0.0-dev"), "github.com/hashicorp/tester"). Assert(lib.MustSucceed(), lib.Grep("Successfully installed plugin", lib.GrepStdout)) }) ts.Run("install same plugin with pre-release + metadata", func() { ts.PackerCommand().UsePluginDir(tempPluginDir). - SetArgs("plugins", "install", "--path", ts.BuildSimplePlugin("1.0.0-dev+metadata", ts.T()), "github.com/hashicorp/tester"). + SetArgs("plugins", "install", "--path", ts.GetPluginPath(ts.T(), "1.0.0-dev+metadata"), "github.com/hashicorp/tester"). Assert(lib.MustSucceed(), lib.Grep("Successfully installed plugin", lib.GrepStdout)) }) @@ -58,7 +58,7 @@ func (ts *PackerPluginTestSuite) TestInstallPluginWithPath() { } func (ts *PackerPluginTestSuite) TestInstallPluginPrerelease() { - pluginPath := ts.BuildSimplePlugin("1.0.1-alpha1", ts.T()) + pluginPath := ts.GetPluginPath(ts.T(), "1.0.1-alpha1") pluginDir, cleanup := ts.MakePluginDir() defer cleanup() diff --git a/packer_test/plugin_tests/loading_test.go b/packer_test/plugin_tests/loading_test.go index b98f3e5cf..2281d66dc 100644 --- a/packer_test/plugin_tests/loading_test.go +++ b/packer_test/plugin_tests/loading_test.go @@ -51,7 +51,7 @@ func (ts *PackerPluginTestSuite) TestLoadWithLegacyPluginName() { pluginDir, cleanup := ts.MakePluginDir() defer cleanup() - plugin := ts.BuildSimplePlugin("1.0.10", ts.T()) + plugin := ts.GetPluginPath(ts.T(), "1.0.10") lib.CopyFile(ts.T(), filepath.Join(pluginDir, "packer-plugin-tester"), plugin) @@ -96,7 +96,7 @@ func (ts *PackerPluginTestSuite) TestLoadWithLegacyPluginName() { } func (ts *PackerPluginTestSuite) TestLoadWithSHAMismatches() { - plugin := ts.BuildSimplePlugin("1.0.10", ts.T()) + plugin := ts.GetPluginPath(ts.T(), "1.0.10") ts.Run("move plugin with right name, but no SHA256SUM, should reject", func() { pluginDir, cleanup := ts.MakePluginDir("1.0.9") @@ -161,7 +161,7 @@ func (ts *PackerPluginTestSuite) TestInstallNonCanonicalPluginVersion() { lib.ManualPluginInstall(ts.T(), filepath.Join(pluginPath, "github.com", "hashicorp", "tester"), - ts.BuildSimplePlugin("1.0.10", ts.T()), + ts.GetPluginPath(ts.T(), "1.0.10"), "001.00.010") ts.Run("try listing plugins with non-canonical version installed - report none", func() { @@ -179,7 +179,7 @@ func (ts *PackerPluginTestSuite) TestLoadPluginWithMetadataInName() { lib.ManualPluginInstall(ts.T(), filepath.Join(pluginPath, "github.com", "hashicorp", "tester"), - ts.BuildSimplePlugin("1.0.10+metadata", ts.T()), + ts.GetPluginPath(ts.T(), "1.0.10+metadata"), "1.0.10+metadata") ts.Run("try listing plugins with metadata in name - report none", func() {