From 080ddead6e3180cd4c136715ba63cab2fc02751e Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Wed, 26 Jul 2023 10:24:25 +0200 Subject: [PATCH] testing framework: perform a plan before starting any tests (#33561) --- internal/command/test.go | 59 ++++++++++++++++--- internal/command/test_test.go | 55 +++++++++++++++++ .../test/invalid_default_state/main.tf | 8 +++ .../invalid_default_state/main.tftest.hcl | 10 ++++ 4 files changed, 123 insertions(+), 9 deletions(-) create mode 100644 internal/command/testdata/test/invalid_default_state/main.tf create mode 100644 internal/command/testdata/test/invalid_default_state/main.tftest.hcl diff --git a/internal/command/test.go b/internal/command/test.go index 5f55f59e41..96fb2253a8 100644 --- a/internal/command/test.go +++ b/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 } diff --git a/internal/command/test_test.go b/internal/command/test_test.go index d988dd8cee..fdf621183a 100644 --- a/internal/command/test_test.go +++ b/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) diff --git a/internal/command/testdata/test/invalid_default_state/main.tf b/internal/command/testdata/test/invalid_default_state/main.tf new file mode 100644 index 0000000000..5cbdbbedcf --- /dev/null +++ b/internal/command/testdata/test/invalid_default_state/main.tf @@ -0,0 +1,8 @@ + +variable "input" { + type = string +} + +resource "test_resource" "resource" { + value = var.input +} diff --git a/internal/command/testdata/test/invalid_default_state/main.tftest.hcl b/internal/command/testdata/test/invalid_default_state/main.tftest.hcl new file mode 100644 index 0000000000..a196d5ce7a --- /dev/null +++ b/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" + } +}