From 31b61094302546de8d7fdf4bcdab2d0f50514e68 Mon Sep 17 00:00:00 2001
From: Lucas Bajolet <lucas.bajolet@hashicorp.com>
Date: Thu, 15 Aug 2024 10:25:43 -0400
Subject: [PATCH] packer_test: hide run and introduce Output

When using a PackerCommand, the Run function was made public as a way to
access the contents of an execution.

This was clumsy as it had too many responsabilities, and was not needed
strictly as Assert was performing the executions, as many times as
required.

This could introduce cases in which one run as spent by the caller, then
the remainder were executed through Assert.

Therefore, we change this convention.

Now, run is private to the type, and only through Assert can a command
be executed.
If a test needs access to a command's output, stderr, or error, it can
do so through the Output function, which requires Assert to be called
first.
---
 packer_test/common/commands.go                | 19 +++++++++++++++++--
 packer_test/common/plugin.go                  |  5 ++++-
 .../plugin_tests/plugins_remove_test.go       |  2 +-
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/packer_test/common/commands.go b/packer_test/common/commands.go
index 9d1cf1b1813..1002aad61c6 100644
--- a/packer_test/common/commands.go
+++ b/packer_test/common/commands.go
@@ -125,7 +125,7 @@ func (pc *packerCommand) SetAssertFatal() *packerCommand {
 // Note: while originally "Run" was designed to be idempotent, with the
 // introduction of multiple runs for a command, this is not the case anymore
 // and the function should not be considered thread-safe anymore.
-func (pc *packerCommand) Run() (string, string, error) {
+func (pc *packerCommand) run() (string, string, error) {
 	if pc.runs <= 0 {
 		return pc.stdout.String(), pc.stderr.String(), pc.err
 	}
@@ -160,11 +160,26 @@ func (pc *packerCommand) Run() (string, string, error) {
 	return pc.stdout.String(), pc.stderr.String(), pc.err
 }
 
+// Output returns the results of the latest Run that was executed.
+//
+// In general there is only one run of the command, but as it can be changed
+// through the Runs function, only the latest run will be returned.
+//
+// If the command was not run (through Assert), this will make the test fail
+// immediately.
+func (pc *packerCommand) Output() (string, string, error) {
+	if pc.runs > 0 {
+		pc.t.Fatalf("command was not run, invoke Assert first, then Output.")
+	}
+
+	return pc.stdout.String(), pc.stderr.String(), pc.err
+}
+
 func (pc *packerCommand) Assert(checks ...check.Checker) {
 	attempt := 0
 	for pc.runs > 0 {
 		attempt++
-		stdout, stderr, err := pc.Run()
+		stdout, stderr, err := pc.run()
 
 		for _, checker := range checks {
 			checkErr := checker.Check(stdout, stderr, err)
diff --git a/packer_test/common/plugin.go b/packer_test/common/plugin.go
index 59a07a2472e..10a7b473031 100644
--- a/packer_test/common/plugin.go
+++ b/packer_test/common/plugin.go
@@ -131,6 +131,9 @@ func (ts *PackerTestSuite) CompilePlugin(t *testing.T, versionString string) {
 // packer will be able to use that directory for running its functions.
 //
 // Deletion of the directory is the caller's responsibility.
+//
+// Note: all of the plugin versions specified to be installed in this plugin directory
+// must have been compiled beforehand.
 func (ts *PackerTestSuite) MakePluginDir(pluginVersions ...string) (pluginTempDir string, cleanup func()) {
 	t := ts.T()
 
@@ -158,7 +161,7 @@ func (ts *PackerTestSuite) MakePluginDir(pluginVersions ...string) (pluginTempDi
 		path := ts.GetPluginPath(t, pluginVersion)
 		cmd := ts.PackerCommand().SetArgs("plugins", "install", "--path", path, "github.com/hashicorp/tester").AddEnv("PACKER_PLUGIN_PATH", pluginTempDir)
 		cmd.Assert(check.MustSucceed())
-		out, stderr, cmdErr := cmd.Run()
+		out, stderr, cmdErr := cmd.run()
 		if cmdErr != nil {
 			err = fmt.Errorf("failed to install tester plugin version %q: %s\nCommand stdout: %s\nCommand stderr: %s", pluginVersion, err, out, stderr)
 			return
diff --git a/packer_test/plugin_tests/plugins_remove_test.go b/packer_test/plugin_tests/plugins_remove_test.go
index 0584a8d07b1..fcaad4459e0 100644
--- a/packer_test/plugin_tests/plugins_remove_test.go
+++ b/packer_test/plugin_tests/plugins_remove_test.go
@@ -168,7 +168,7 @@ func InstalledPlugins(ts *PackerPluginTestSuite, dir string) []string {
 		SetAssertFatal()
 	cmd.Assert(check.MustSucceed())
 
-	out, _, _ := cmd.Run()
+	out, _, _ := cmd.Output()
 	// Output will be split on '\n' after trimming all other white space
 	out = strings.TrimSpace(out)
 	plugins := strings.Fields(out)