testing framework: perform a plan before starting any tests (#33561)

internal/command/test.go

@@ -334,6 +334,23 @@ func (runner *TestRunner) ExecuteTestFile(file *moduletest.File, globals map[str
 	mgr := new(TestStateManager)
 	mgr.runner = runner
 	mgr.State = states.NewState()
+
+	// We're going to check if the cleanupStates function call will actually
+	// work before we start the test.
+	diags := mgr.prepare(file, globals)
+	if diags.HasErrors() || runner.Cancelled || runner.Stopped {
+		file.Status = moduletest.Error
+		runner.View.File(file)
+		runner.View.Diagnostics(nil, file, diags)
+		for _, run := range file.Runs {
+			run.Status = moduletest.Skip
+			runner.View.Run(run, file)
+		}
+		return
+	}
+
+	// Make sure we clean up any states created during the execution of this
+	// file.
 	defer mgr.cleanupStates(file, globals)
 
 	file.Status = file.Status.Merge(moduletest.Pass)
@@ -375,6 +392,7 @@ func (runner *TestRunner) ExecuteTestFile(file *moduletest.File, globals map[str
 	}
 
 	runner.View.File(file)
+	runner.View.Diagnostics(nil, file, diags) // Print out any warnings from the preparation.
 	for _, run := range file.Runs {
 		runner.View.Run(run, file)
 	}
@@ -475,7 +493,7 @@ func (runner *TestRunner) ExecuteTestRun(mgr *TestStateManager, run *moduletest.
 		run.Diagnostics = run.Diagnostics.Append(diags)
 	}
 
-	variables, diags := buildInputVariablesForAssertions(run, file, config, globals)
+	variables, diags := buildInputVariablesForAssertions(run, file, globals)
 	run.Diagnostics = run.Diagnostics.Append(diags)
 	if diags.HasErrors() {
 		run.Status = moduletest.Error
@@ -726,6 +744,26 @@ type TestModuleState struct {
 	Run *moduletest.Run
 }
 
+// prepare makes some simple checks that increase our confidence that a later
+// clean up operation will succeed.
+//
+// When it comes time to execute cleanupStates below, we only have the
+// information available at the file level. Our run blocks may have executed
+// with additional data and configuration, so it's possible that we could
+// successfully execute all our run blocks and then find we cannot perform any
+// cleanup. We want to use this function to check that our cleanup can happen
+// using only the information available within the file.
+func (manager *TestStateManager) prepare(file *moduletest.File, globals map[string]backend.UnparsedVariableValue) tfdiags.Diagnostics {
+
+	// For now, the only thing we care about is making sure all the required
+	// variables have values.
+	_, diags := buildInputVariablesForTest(nil, file, manager.runner.Config, globals)
+
+	// Return the sum of diagnostics that might indicate a problem for any
+	// future attempted cleanup.
+	return diags
+}
+
 func (manager *TestStateManager) cleanupStates(file *moduletest.File, globals map[string]backend.UnparsedVariableValue) {
 	log.Printf("[TRACE] TestStateManager: cleaning up state for %s", file.Name)
 
@@ -821,13 +859,9 @@ func buildInputVariablesForTest(run *moduletest.Run, file *moduletest.File, conf
 //
 // Crucially, it differs from buildInputVariablesForTest in that the returned
 // input values include all variables available even if they are not defined
-// within the config.
-//
-// This does mean the returned diags might contain warnings about variables not
-// defined within the config. We might want to remove these warnings in the
-// future, since it is actually okay for test files to have variables defined
-// outside the configuration.
-func buildInputVariablesForAssertions(run *moduletest.Run, file *moduletest.File, config *configs.Config, globals map[string]backend.UnparsedVariableValue) (terraform.InputValues, tfdiags.Diagnostics) {
+// within the config. This allows the assertions to refer to variables defined
+// solely within the test file, and not only those within the configuration.
+func buildInputVariablesForAssertions(run *moduletest.Run, file *moduletest.File, globals map[string]backend.UnparsedVariableValue) (terraform.InputValues, tfdiags.Diagnostics) {
 	variables := make(map[string]backend.UnparsedVariableValue)
 
 	if run != nil {
@@ -863,5 +897,12 @@ func buildInputVariablesForAssertions(run *moduletest.Run, file *moduletest.File
 		variables[name] = variable
 	}
 
-	return backend.ParseVariableValues(variables, config.Module.Variables)
+	inputs := make(terraform.InputValues, len(variables))
+	var diags tfdiags.Diagnostics
+	for name, variable := range variables {
+		value, valueDiags := variable.ParseVariableValue(configs.VariableParseLiteral)
+		diags = diags.Append(valueDiags)
+		inputs[name] = value
+	}
+	return inputs, diags
 }

internal/command/test_test.go

@@ -374,6 +374,61 @@ func TestTest_ModuleDependencies(t *testing.T) {
 	}
 }
 
+func TestTest_CatchesErrorsBeforeDestroy(t *testing.T) {
+	td := t.TempDir()
+	testCopyDir(t, testFixturePath(path.Join("test", "invalid_default_state")), td)
+	defer testChdir(t, td)()
+
+	provider := testing_command.NewProvider(nil)
+	view, done := testView(t)
+
+	c := &TestCommand{
+		Meta: Meta{
+			testingOverrides: metaOverridesForProvider(provider.Provider),
+			View:             view,
+		},
+	}
+
+	code := c.Run([]string{"-no-color"})
+	output := done(t)
+
+	if code != 1 {
+		t.Errorf("expected status code 0 but got %d", code)
+	}
+
+	expectedOut := `main.tftest.hcl... fail
+  run "test"... skip
+
+Failure! 0 passed, 0 failed, 1 skipped.
+`
+
+	expectedErr := `
+Error: No value for required variable
+
+  on main.tf line 2:
+   2: variable "input" {
+
+The root module input variable "input" is not set, and has no default value.
+Use a -var or -var-file command line argument to provide a value for this
+variable.
+`
+
+	actualOut := output.Stdout()
+	actualErr := output.Stderr()
+
+	if diff := cmp.Diff(actualOut, expectedOut); len(diff) > 0 {
+		t.Errorf("std out didn't match expected:\nexpected:\n%s\nactual:\n%s\ndiff:\n%s", expectedOut, actualOut, diff)
+	}
+
+	if diff := cmp.Diff(actualErr, expectedErr); len(diff) > 0 {
+		t.Errorf("std err didn't match expected:\nexpected:\n%s\nactual:\n%s\ndiff:\n%s", expectedErr, actualErr, diff)
+	}
+
+	if provider.ResourceCount() > 0 {
+		t.Errorf("should have deleted all resources on completion but left %v", provider.ResourceString())
+	}
+}
+
 func TestTest_Verbose(t *testing.T) {
 	td := t.TempDir()
 	testCopyDir(t, testFixturePath(path.Join("test", "plan_then_apply")), td)

internal/command/testdata/test/invalid_default_state/main.tf

@@ -0,0 +1,8 @@
+
+variable "input" {
+  type = string
+}
+
+resource "test_resource" "resource" {
+    value = var.input
+}

internal/command/testdata/test/invalid_default_state/main.tftest.hcl

@@ -0,0 +1,10 @@
+run "test" {
+    variables {
+        input = "Hello, world!"
+    }
+
+    assert {
+        condition = test_resource.resource.value == "Hello, world!"
+        error_message = "wrong condition"
+    }
+}