diff --git a/internal/command/init.go b/internal/command/init.go index f8eaab66cd..aabd669983 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -158,7 +158,7 @@ func (c *InitCommand) Run(args []string) int { } // Load just the root module to begin backend and module initialization - rootModEarly, earlyConfDiags := c.loadSingleModule(path) + rootModEarly, earlyConfDiags := c.loadSingleModuleWithTests(path, "tests") // There may be parsing errors in config loading but these will be shown later _after_ // checking for core version requirement errors. Not meeting the version requirement should @@ -329,7 +329,16 @@ func (c *InitCommand) Run(args []string) int { } func (c *InitCommand) getModules(path string, earlyRoot *configs.Module, upgrade bool) (output bool, abort bool, diags tfdiags.Diagnostics) { - if len(earlyRoot.ModuleCalls) == 0 { + testModules := false // We can also have modules buried in test files. + for _, file := range earlyRoot.Tests { + for _, run := range file.Runs { + if run.Module != nil { + testModules = true + } + } + } + + if len(earlyRoot.ModuleCalls) == 0 && !testModules { // Nothing to do return false, false, nil } diff --git a/internal/command/init_test.go b/internal/command/init_test.go index e81db3e205..5ada9765cc 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -2711,6 +2711,72 @@ func TestInit_invalidSyntaxInvalidBackend(t *testing.T) { } } +func TestInit_tests(t *testing.T) { + // Create a temporary working directory that is empty + td := t.TempDir() + testCopyDir(t, testFixturePath("init-with-tests"), td) + defer testChdir(t, td)() + + provider := applyFixtureProvider() // We just want the types from this provider. + + providerSource, close := newMockProviderSource(t, map[string][]string{ + "hashicorp/test": {"1.0.0"}, + }) + defer close() + + ui := new(cli.MockUi) + view, _ := testView(t) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(provider), + Ui: ui, + View: view, + ProviderSource: providerSource, + }, + } + + args := []string{} + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } +} + +func TestInit_testsWithModule(t *testing.T) { + // Create a temporary working directory that is empty + td := t.TempDir() + testCopyDir(t, testFixturePath("init-with-tests-with-module"), td) + defer testChdir(t, td)() + + provider := applyFixtureProvider() // We just want the types from this provider. + + providerSource, close := newMockProviderSource(t, map[string][]string{ + "hashicorp/test": {"1.0.0"}, + }) + defer close() + + ui := new(cli.MockUi) + view, _ := testView(t) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(provider), + Ui: ui, + View: view, + ProviderSource: providerSource, + }, + } + + args := []string{} + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + + // Check output + output := ui.OutputWriter.String() + if !strings.Contains(output, "test.main.setup in setup") { + t.Fatalf("doesn't look like we installed the test module': %s", output) + } +} + // newMockProviderSource is a helper to succinctly construct a mock provider // source that contains a set of packages matching the given provider versions // that are available for installation (from temporary local files). diff --git a/internal/command/meta_config.go b/internal/command/meta_config.go index 5a86d84cd0..0b91eba29a 100644 --- a/internal/command/meta_config.go +++ b/internal/command/meta_config.go @@ -12,6 +12,9 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" + "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configload" "github.com/hashicorp/terraform/internal/configs/configschema" @@ -19,8 +22,6 @@ import ( "github.com/hashicorp/terraform/internal/registry" "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" - "github.com/zclconf/go-cty/cty/convert" ) // normalizePath normalizes a given path so that it is, if possible, relative @@ -73,6 +74,23 @@ func (m *Meta) loadSingleModule(dir string) (*configs.Module, tfdiags.Diagnostic return module, diags } +// loadSingleModuleWithTests matches loadSingleModule except it also loads any +// tests for the target module. +func (m *Meta) loadSingleModuleWithTests(dir string, testDir string) (*configs.Module, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics + dir = m.normalizePath(dir) + + loader, err := m.initConfigLoader() + if err != nil { + diags = diags.Append(err) + return nil, diags + } + + module, hclDiags := loader.Parser().LoadConfigDirWithTests(dir, testDir) + diags = diags.Append(hclDiags) + return module, diags +} + // dirIsConfigPath checks if the given path is a directory that contains at // least one Terraform configuration file (.tf or .tf.json), returning true // if so. diff --git a/internal/command/test.go b/internal/command/test.go index cf4381a0af..4e48ef3896 100644 --- a/internal/command/test.go +++ b/internal/command/test.go @@ -142,60 +142,43 @@ func (c *TestCommand) ExecuteTestSuite(suite *moduletest.Suite, config *configs. } func (c *TestCommand) ExecuteTestFile(ctx *terraform.Context, file *moduletest.File, config *configs.Config, view views.Test) { - var diags tfdiags.Diagnostics - globalVariableValues, diags := c.CollectDefaultVariables(file.Config.Variables, config) - if diags.HasErrors() { - file.Status = file.Status.Merge(moduletest.Error) - view.File(file) - view.Diagnostics(nil, file, diags) - return - } - - state := states.NewState() - defer func() { - - // Whatever happens, at the end of this test we don't want to leave - // active resources behind. So we'll do a destroy action against the - // state in a deferred function. - - plan, planDiags := ctx.Plan(config, state, &terraform.PlanOpts{ - Mode: plans.DestroyMode, - SetVariables: globalVariableValues, - }) - if planDiags.HasErrors() { - // This is bad, we need to tell the user that we couldn't clean up - // and they need to go and manually delete some resources. - view.DestroySummary(planDiags, file, state) - return - } - view.Diagnostics(nil, file, planDiags) // Print out any warnings from the destroy plan. - - finalState, applyDiags := ctx.Apply(plan, config) - view.DestroySummary(applyDiags, file, finalState) - }() + mgr := new(TestStateManager) + mgr.c = c + mgr.State = states.NewState() + defer mgr.cleanupStates(ctx, view, file, config) file.Status = file.Status.Merge(moduletest.Pass) for _, run := range file.Runs { if file.Status == moduletest.Error { + // If the overall test file has errored, we don't keep trying to + // execute tests. Instead, we mark all remaining run blocks as + // skipped. run.Status = moduletest.Skip continue } - state = c.ExecuteTestRun(ctx, run, state, config, globalVariableValues) + if run.Config.ConfigUnderTest != nil { + // Then we want to execute a different module under a kind of + // sandbox. + state := c.ExecuteTestRun(ctx, run, states.NewState(), run.Config.ConfigUnderTest, file.Config.Variables) + mgr.States = append(mgr.States, &TestModuleState{ + State: state, + Run: run, + }) + } else { + mgr.State = c.ExecuteTestRun(ctx, run, mgr.State, config, file.Config.Variables) + } file.Status = file.Status.Merge(run.Status) } view.File(file) - view.Diagnostics(nil, file, diags) - for _, run := range file.Runs { view.Run(run, file) } } -func (c *TestCommand) ExecuteTestRun(ctx *terraform.Context, run *moduletest.Run, state *states.State, config *configs.Config, defaults terraform.InputValues) *states.State { - +func (c *TestCommand) ExecuteTestRun(ctx *terraform.Context, run *moduletest.Run, state *states.State, config *configs.Config, globals map[string]hcl.Expression) *states.State { var targets []addrs.Targetable for _, target := range run.Config.Options.Target { addr, diags := addrs.ParseTarget(target) @@ -230,7 +213,7 @@ func (c *TestCommand) ExecuteTestRun(ctx *terraform.Context, run *moduletest.Run replaces = append(replaces, addr) } - variables, diags := c.OverrideDefaultVariables(run.Config.Variables, config, defaults) + variables, diags := c.GetInputValues(run.Config.Variables, globals, config) run.Diagnostics = run.Diagnostics.Append(diags) if diags.HasErrors() { run.Status = moduletest.Error @@ -287,9 +270,27 @@ func (c *TestCommand) ExecuteTestRun(ctx *terraform.Context, run *moduletest.Run return state } -func (c *TestCommand) CollectDefaultVariables(exprs map[string]hcl.Expression, config *configs.Config) (terraform.InputValues, tfdiags.Diagnostics) { +func (c *TestCommand) GetInputValues(locals map[string]hcl.Expression, globals map[string]hcl.Expression, config *configs.Config) (terraform.InputValues, tfdiags.Diagnostics) { + variables := make(map[string]hcl.Expression) + for name := range config.Module.Variables { + if expr, exists := locals[name]; exists { + // Local variables take precedence. + variables[name] = expr + continue + } + + if expr, exists := globals[name]; exists { + // If it's not set locally, it maybe set globally. + variables[name] = expr + continue + } + + // If it's not set at all that might be okay if the variable is optional + // so we'll just not add anything to the map. + } + unparsed := make(map[string]backend.UnparsedVariableValue) - for key, value := range exprs { + for key, value := range variables { unparsed[key] = unparsedVariableValueExpression{ expr: value, sourceType: terraform.ValueFromConfig, @@ -298,33 +299,83 @@ func (c *TestCommand) CollectDefaultVariables(exprs map[string]hcl.Expression, c return backend.ParseVariableValues(unparsed, config.Module.Variables) } -func (c *TestCommand) OverrideDefaultVariables(exprs map[string]hcl.Expression, config *configs.Config, existing terraform.InputValues) (terraform.InputValues, tfdiags.Diagnostics) { - if len(exprs) == 0 { - return existing, nil +func (c *TestCommand) cleanupState(ctx *terraform.Context, view views.Test, run *moduletest.Run, file *moduletest.File, config *configs.Config, state *states.State) { + if state.Empty() { + // Nothing to do. + return } - decls := make(map[string]*configs.Variable) - unparsed := make(map[string]backend.UnparsedVariableValue) - for name, variable := range exprs { - - if config, ok := config.Module.Variables[name]; ok { - decls[name] = config - } - - unparsed[name] = unparsedVariableValueExpression{ - expr: variable, - sourceType: terraform.ValueFromConfig, - } + var locals map[string]hcl.Expression + if run != nil { + locals = run.Config.Variables } - overrides, diags := backend.ParseVariableValues(unparsed, decls) - values := make(terraform.InputValues) - for name, value := range existing { - if override, ok := overrides[name]; ok { - values[name] = override - continue - } - values[name] = value + variables, variableDiags := c.GetInputValues(locals, file.Config.Variables, config) + if variableDiags.HasErrors() { + // This shouldn't really trigger, as we will have created something + // using these variables at an earlier stage so for them to have a + // problem now would be strange. But just to be safe we'll handle this. + view.DestroySummary(variableDiags, run, file, state) + return + } + view.Diagnostics(nil, file, variableDiags) + + plan, planDiags := ctx.Plan(config, state, &terraform.PlanOpts{ + Mode: plans.DestroyMode, + SetVariables: variables, + }) + if planDiags.HasErrors() { + // This is bad, we need to tell the user that we couldn't clean up + // and they need to go and manually delete some resources. + view.DestroySummary(planDiags, run, file, state) + return + } + view.Diagnostics(nil, file, planDiags) // Print out any warnings from the destroy plan. + + finalState, applyDiags := ctx.Apply(plan, config) + view.DestroySummary(applyDiags, run, file, finalState) +} + +// TestStateManager is a helper struct to maintain the various state objects +// that a test file has to keep track of. +type TestStateManager struct { + c *TestCommand + + // State is the main state of the module under test during a single test + // file execution. This state will be updated by every run block without + // a modifier module block within the test file. At the end of the test + // file's execution everything in this state should be executed. + State *states.State + + // States contains the states of every run block within a test file that + // executed using an alternative module. Any resources created by these + // run blocks also need to be tidied up, but only after the main state file + // has been handled. + States []*TestModuleState +} + +// TestModuleState holds the config and the state for a given run block that +// executed with a custom module. +type TestModuleState struct { + // State is the state after the module executed. + State *states.State + + // File is the config for the file containing the Run. + File *moduletest.File + + // Run is the config for the given run block, that contains the config + // under test and the variable values. + Run *moduletest.Run +} + +func (manager *TestStateManager) cleanupStates(ctx *terraform.Context, view views.Test, file *moduletest.File, config *configs.Config) { + // First, we'll clean up the main state. + manager.c.cleanupState(ctx, view, nil, file, config, manager.State) + + // Then we'll clean up the additional states for custom modules in reverse + // order. + for ix := len(manager.States); ix > 0; ix-- { + state := manager.States[ix-1] + manager.c.cleanupState(ctx, view, state.Run, file, state.Run.Config.ConfigUnderTest, state.State) } - return values, diags } diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 3972ccdc62..10a3ed0488 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -197,10 +197,6 @@ func TestTest_ProviderAlias(t *testing.T) { } func TestTest_ModuleDependencies(t *testing.T) { - // TODO(liamcervante): Enable this test once we have added support for - // module customisation into the testing framework. - t.Skip() - td := t.TempDir() testCopyDir(t, testFixturePath(path.Join("test", "with_setup_module")), td) defer testChdir(t, td)() diff --git a/internal/command/testdata/init-with-tests-with-module/main.tf b/internal/command/testdata/init-with-tests-with-module/main.tf new file mode 100644 index 0000000000..2b976525ac --- /dev/null +++ b/internal/command/testdata/init-with-tests-with-module/main.tf @@ -0,0 +1,3 @@ +resource "test_instance" "foo" { + ami = "bar" +} diff --git a/internal/command/testdata/init-with-tests-with-module/main.tftest b/internal/command/testdata/init-with-tests-with-module/main.tftest new file mode 100644 index 0000000000..8b0776e1a7 --- /dev/null +++ b/internal/command/testdata/init-with-tests-with-module/main.tftest @@ -0,0 +1,12 @@ +run "setup" { + module { + source = "./setup" + } +} + +run "test" { + assert { + condition = test_instance.foo.ami == "bar" + error_message = "incorrect value" + } +} diff --git a/internal/command/testdata/init-with-tests-with-module/setup/main.tf b/internal/command/testdata/init-with-tests-with-module/setup/main.tf new file mode 100644 index 0000000000..f1017765c5 --- /dev/null +++ b/internal/command/testdata/init-with-tests-with-module/setup/main.tf @@ -0,0 +1,3 @@ +resource "test_instance" "baz" { + ami = "baz" +} diff --git a/internal/command/testdata/init-with-tests/main.tf b/internal/command/testdata/init-with-tests/main.tf new file mode 100644 index 0000000000..2b976525ac --- /dev/null +++ b/internal/command/testdata/init-with-tests/main.tf @@ -0,0 +1,3 @@ +resource "test_instance" "foo" { + ami = "bar" +} diff --git a/internal/command/testdata/init-with-tests/main.tftest b/internal/command/testdata/init-with-tests/main.tftest new file mode 100644 index 0000000000..b68725876b --- /dev/null +++ b/internal/command/testdata/init-with-tests/main.tftest @@ -0,0 +1,6 @@ +run "test" { + assert { + condition = test_instance.foo.ami == "bar" + error_message = "incorrect value" + } +} diff --git a/internal/command/views/test.go b/internal/command/views/test.go index 1420087ad2..c0697eabc9 100644 --- a/internal/command/views/test.go +++ b/internal/command/views/test.go @@ -34,7 +34,7 @@ type Test interface { // DestroySummary prints out the summary of the destroy step of each test // file. If everything goes well, this should be empty. - DestroySummary(diags tfdiags.Diagnostics, file *moduletest.File, state *states.State) + DestroySummary(diags tfdiags.Diagnostics, run *moduletest.Run, file *moduletest.File, state *states.State) // Diagnostics prints out the provided diagnostics. Diagnostics(run *moduletest.Run, file *moduletest.File, diags tfdiags.Diagnostics) @@ -112,14 +112,19 @@ func (t *TestHuman) Run(run *moduletest.Run, file *moduletest.File) { t.Diagnostics(run, file, run.Diagnostics) } -func (t *TestHuman) DestroySummary(diags tfdiags.Diagnostics, file *moduletest.File, state *states.State) { - if diags.HasErrors() { - t.view.streams.Eprintf("Terraform encountered an error destroying resources created while executing %s.\n", file.Name) +func (t *TestHuman) DestroySummary(diags tfdiags.Diagnostics, run *moduletest.Run, file *moduletest.File, state *states.State) { + identifier := file.Name + if run != nil { + identifier = fmt.Sprintf("%s/%s", identifier, run.Name) } - t.Diagnostics(nil, file, diags) + + if diags.HasErrors() { + t.view.streams.Eprintf("Terraform encountered an error destroying resources created while executing %s.\n", identifier) + } + t.Diagnostics(run, file, diags) if state.HasManagedResourceInstanceObjects() { - t.view.streams.Eprintf("\nTerraform left the following resources in state after executing %s, they need to be cleaned up manually:\n", file.Name) + t.view.streams.Eprintf("\nTerraform left the following resources in state after executing %s, they need to be cleaned up manually:\n", identifier) for _, resource := range state.AllResourceInstanceObjectAddrs() { if resource.DeposedKey != states.NotDeposed { t.view.streams.Eprintf(" - %s (%s)\n", resource.Instance, resource.DeposedKey) @@ -239,7 +244,7 @@ func (t TestJSON) Run(run *moduletest.Run, file *moduletest.File) { t.Diagnostics(run, file, run.Diagnostics) } -func (t TestJSON) DestroySummary(diags tfdiags.Diagnostics, file *moduletest.File, state *states.State) { +func (t TestJSON) DestroySummary(diags tfdiags.Diagnostics, run *moduletest.Run, file *moduletest.File, state *states.State) { if state.HasManagedResourceInstanceObjects() { cleanup := json.TestFileCleanup{} for _, resource := range state.AllResourceInstanceObjectAddrs() { @@ -249,14 +254,24 @@ func (t TestJSON) DestroySummary(diags tfdiags.Diagnostics, file *moduletest.Fil }) } - t.view.log.Error( - fmt.Sprintf("Terraform left some resources in state after executing %s, they need to be cleaned up manually.", file.Name), - "type", json.MessageTestCleanup, - json.MessageTestCleanup, cleanup, - "@testfile", file.Name) + if run != nil { + t.view.log.Error( + fmt.Sprintf("Terraform left some resources in state after executing %s/%s, they need to be cleaned up manually.", file.Name, run.Name), + "type", json.MessageTestCleanup, + json.MessageTestCleanup, cleanup, + "@testfile", file.Name, + "@testrun", run.Name) + } else { + t.view.log.Error( + fmt.Sprintf("Terraform left some resources in state after executing %s, they need to be cleaned up manually.", file.Name), + "type", json.MessageTestCleanup, + json.MessageTestCleanup, cleanup, + "@testfile", file.Name) + } + } - t.Diagnostics(nil, file, diags) + t.Diagnostics(run, file, diags) } func (t TestJSON) Diagnostics(run *moduletest.Run, file *moduletest.File, diags tfdiags.Diagnostics) { diff --git a/internal/command/views/test_test.go b/internal/command/views/test_test.go index 6c22d01631..7b4dc702fb 100644 --- a/internal/command/views/test_test.go +++ b/internal/command/views/test_test.go @@ -555,6 +555,7 @@ something bad happened during this test func TestTestHuman_DestroySummary(t *testing.T) { tcs := map[string]struct { diags tfdiags.Diagnostics + run *moduletest.Run file *moduletest.File state *states.State stdout string @@ -603,6 +604,20 @@ some thing not very bad happened again Error: first error +this time it is very bad +`, + }, + "error_from_run": { + diags: tfdiags.Diagnostics{ + tfdiags.Sourceless(tfdiags.Error, "first error", "this time it is very bad"), + }, + run: &moduletest.Run{Name: "run_block"}, + file: &moduletest.File{Name: "main.tftest"}, + state: states.NewState(), + stderr: `Terraform encountered an error destroying resources created while executing main.tftest/run_block. + +Error: first error + this time it is very bad `, }, @@ -746,7 +761,7 @@ Terraform left the following resources in state after executing main.tftest, the streams, done := terminal.StreamsForTesting(t) view := NewTest(arguments.ViewHuman, NewView(streams)) - view.DestroySummary(tc.diags, tc.file, tc.state) + view.DestroySummary(tc.diags, tc.run, tc.file, tc.state) output := done(t) actual, expected := output.Stdout(), tc.stdout @@ -1355,6 +1370,7 @@ func TestTestJSON_Conclusion(t *testing.T) { func TestTestJSON_DestroySummary(t *testing.T) { tcs := map[string]struct { file *moduletest.File + run *moduletest.Run state *states.State diags tfdiags.Diagnostics want []map[string]interface{} @@ -1440,6 +1456,42 @@ func TestTestJSON_DestroySummary(t *testing.T) { }, }, }, + "state_from_run": { + file: &moduletest.File{Name: "main.tftest"}, + run: &moduletest.Run{Name: "run_block"}, + state: states.BuildState(func(state *states.SyncState) { + state.SetResourceInstanceCurrent( + addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "test", + Name: "foo", + }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance), + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + }, + addrs.AbsProviderConfig{ + Module: addrs.RootModule, + Provider: addrs.NewDefaultProvider("test"), + }) + }), + want: []map[string]interface{}{ + { + "@level": "error", + "@message": "Terraform left some resources in state after executing main.tftest/run_block, they need to be cleaned up manually.", + "@module": "terraform.ui", + "@testfile": "main.tftest", + "@testrun": "run_block", + "test_cleanup": map[string]interface{}{ + "failed_resources": []interface{}{ + map[string]interface{}{ + "instance": "test.foo", + }, + }, + }, + "type": "test_cleanup", + }, + }, + }, "state_only_warnings": { diags: tfdiags.Diagnostics{ tfdiags.Sourceless(tfdiags.Warning, "first warning", "something not very bad happened"), @@ -1651,7 +1703,7 @@ func TestTestJSON_DestroySummary(t *testing.T) { streams, done := terminal.StreamsForTesting(t) view := NewTest(arguments.ViewJSON, NewView(streams)) - view.DestroySummary(tc.diags, tc.file, tc.state) + view.DestroySummary(tc.diags, tc.run, tc.file, tc.state) testJSONViewOutputEquals(t, done(t).All(), tc.want) }) } diff --git a/internal/configs/config_build.go b/internal/configs/config_build.go index 78a0cf9fa1..4536fed898 100644 --- a/internal/configs/config_build.go +++ b/internal/configs/config_build.go @@ -5,10 +5,13 @@ package configs import ( "fmt" + "path" "sort" + "strings" version "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/addrs" ) @@ -26,6 +29,7 @@ func BuildConfig(root *Module, walker ModuleWalker) (*Config, hcl.Diagnostics) { } cfg.Root = cfg // Root module is self-referential. cfg.Children, diags = buildChildModules(cfg, walker) + diags = append(diags, buildTestModules(cfg, walker)...) // Skip provider resolution if there are any errors, since the provider // configurations themselves may not be valid. @@ -40,6 +44,64 @@ func BuildConfig(root *Module, walker ModuleWalker) (*Config, hcl.Diagnostics) { return cfg, diags } +func buildTestModules(root *Config, walker ModuleWalker) hcl.Diagnostics { + var diags hcl.Diagnostics + + for name, file := range root.Module.Tests { + for _, run := range file.Runs { + if run.Module == nil { + continue + } + + // We want to make sure the path for the testing modules are unique + // so we create a dedicated path for them. + // + // Some examples: + // - file: main.tftest, run: setup - test.main.setup + // - file: tests/main.tftest, run: setup - test.tests.main.setup + + dir := path.Dir(name) + base := path.Base(name) + + path := addrs.Module{} + path = append(path, "test") + if dir != "." { + path = append(path, strings.Split(dir, "/")...) + } + path = append(path, strings.TrimSuffix(base, ".tftest"), run.Name) + + req := ModuleRequest{ + Name: run.Name, + Path: path, + SourceAddr: run.Module.Source, + SourceAddrRange: run.Module.SourceDeclRange, + VersionConstraint: run.Module.Version, + Parent: root, + CallRange: run.Module.DeclRange, + } + + cfg, modDiags := loadModule(root, &req, walker) + diags = append(diags, modDiags...) + + if cfg != nil { + // To get the loader to work, we need to set a bunch of values + // (like the name, path, and parent) as if the module was being + // loaded as a child of the root config. + // + // In actuality, when this is executed it will be as if the + // module was the root. So, we'll post-process some things to + // get it to behave as expected later. + cfg.Path = addrs.RootModule + cfg.Parent = nil + cfg.Root = cfg + run.ConfigUnderTest = cfg + } + } + } + + return diags +} + func buildChildModules(parent *Config, walker ModuleWalker) (map[string]*Config, hcl.Diagnostics) { var diags hcl.Diagnostics ret := map[string]*Config{} @@ -69,54 +131,67 @@ func buildChildModules(parent *Config, walker ModuleWalker) (map[string]*Config, Parent: parent, CallRange: call.DeclRange, } - - mod, ver, modDiags := walker.LoadModule(&req) + child, modDiags := loadModule(parent.Root, &req, walker) diags = append(diags, modDiags...) - if mod == nil { - // nil can be returned if the source address was invalid and so - // nothing could be loaded whatsoever. LoadModule should've - // returned at least one error diagnostic in that case. + if child == nil { + // This means an error occurred, there should be diagnostics within + // modDiags for this. continue } - child := &Config{ - Parent: parent, - Root: parent.Root, - Path: path, - Module: mod, - CallRange: call.DeclRange, - SourceAddr: call.SourceAddr, - SourceAddrRange: call.SourceAddrRange, - Version: ver, - } - - child.Children, modDiags = buildChildModules(child, walker) - diags = append(diags, modDiags...) - - if mod.Backend != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagWarning, - Summary: "Backend configuration ignored", - Detail: "Any selected backend applies to the entire configuration, so Terraform expects provider configurations only in the root module.\n\nThis is a warning rather than an error because it's sometimes convenient to temporarily call a root module as a child module for testing purposes, but this backend configuration block will have no effect.", - Subject: mod.Backend.DeclRange.Ptr(), - }) - } - - if len(mod.Import) > 0 { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Invalid import configuration", - Detail: fmt.Sprintf("An import block was detected in %q. Import blocks are only allowed in the root module.", child.Path), - Subject: mod.Import[0].DeclRange.Ptr(), - }) - } - ret[call.Name] = child } return ret, diags } +func loadModule(root *Config, req *ModuleRequest, walker ModuleWalker) (*Config, hcl.Diagnostics) { + var diags hcl.Diagnostics + + mod, ver, modDiags := walker.LoadModule(req) + diags = append(diags, modDiags...) + if mod == nil { + // nil can be returned if the source address was invalid and so + // nothing could be loaded whatsoever. LoadModule should've + // returned at least one error diagnostic in that case. + return nil, diags + } + + cfg := &Config{ + Parent: req.Parent, + Root: root, + Path: req.Path, + Module: mod, + CallRange: req.CallRange, + SourceAddr: req.SourceAddr, + SourceAddrRange: req.SourceAddrRange, + Version: ver, + } + + cfg.Children, modDiags = buildChildModules(cfg, walker) + diags = append(diags, modDiags...) + + if mod.Backend != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Backend configuration ignored", + Detail: "Any selected backend applies to the entire configuration, so Terraform expects provider configurations only in the root module.\n\nThis is a warning rather than an error because it's sometimes convenient to temporarily call a root module as a child module for testing purposes, but this backend configuration block will have no effect.", + Subject: mod.Backend.DeclRange.Ptr(), + }) + } + + if len(mod.Import) > 0 { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid import configuration", + Detail: fmt.Sprintf("An import block was detected in %q. Import blocks are only allowed in the root module.", cfg.Path), + Subject: mod.Import[0].DeclRange.Ptr(), + }) + } + + return cfg, diags +} + // A ModuleWalker knows how to find and load a child module given details about // the module to be loaded and a reference to its partially-loaded parent // Config. diff --git a/internal/configs/config_build_test.go b/internal/configs/config_build_test.go index cee7c095e6..9ed83df5af 100644 --- a/internal/configs/config_build_test.go +++ b/internal/configs/config_build_test.go @@ -282,3 +282,59 @@ func TestBuildConfigInvalidModules(t *testing.T) { }) } } + +func TestBuildConfig_WithTestModule(t *testing.T) { + parser := NewParser(nil) + mod, diags := parser.LoadConfigDirWithTests("testdata/valid-modules/with-tests-module", "tests") + assertNoDiagnostics(t, diags) + if mod == nil { + t.Fatal("got nil root module; want non-nil") + } + + cfg, diags := BuildConfig(mod, ModuleWalkerFunc( + func(req *ModuleRequest) (*Module, *version.Version, hcl.Diagnostics) { + // For the sake of this test we're going to just treat our + // SourceAddr as a path relative to our fixture directory. + // A "real" implementation of ModuleWalker should accept the + // various different source address syntaxes Terraform supports. + sourcePath := filepath.Join("testdata/valid-modules/with-tests-module", req.SourceAddr.String()) + + mod, diags := parser.LoadConfigDir(sourcePath) + version, _ := version.NewVersion("1.0.0") + return mod, version, diags + }, + )) + assertNoDiagnostics(t, diags) + if cfg == nil { + t.Fatal("got nil config; want non-nil") + } + + // We should have loaded our test case, and one of the test runs should + // have loaded an alternate module. + + if len(cfg.Module.Tests) != 1 { + t.Fatalf("expected exactly one test case but found %d", len(cfg.Module.Tests)) + } + + test := cfg.Module.Tests["main.tftest"] + if len(test.Runs) != 2 { + t.Fatalf("expected two test runs but found %d", len(test.Runs)) + } + + run := test.Runs[0] + if run.ConfigUnderTest == nil { + t.Fatalf("the first test run should have loaded config but did not") + } + + if run.ConfigUnderTest.Parent != nil { + t.Errorf("config under test should not have a parent") + } + + if run.ConfigUnderTest.Root != run.ConfigUnderTest { + t.Errorf("config under test root should be itself") + } + + if len(run.ConfigUnderTest.Path) > 0 { + t.Errorf("config under test path should be the root module") + } +} diff --git a/internal/configs/test_file.go b/internal/configs/test_file.go index 244b7dadfe..a6578d7738 100644 --- a/internal/configs/test_file.go +++ b/internal/configs/test_file.go @@ -5,6 +5,9 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/gohcl" + + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/getmodules" ) // TestCommand represents the Terraform a given run block will execute, plan @@ -78,6 +81,23 @@ type TestRun struct { // checked by this run block. CheckRules []*CheckRule + // Module defines an address of another module that should be loaded and + // executed as part of this run block instead of the module under test. + // + // In the initial version of the testing framework we will only support + // loading alternate modules from local directories or the registry. + Module *TestRunModuleCall + + // ConfigUnderTest describes the configuration this run block should execute + // against. + // + // In typical cases, this will be null and the config under test is the + // configuration within the directory the terraform test command is + // executing within. However, when Module is set the config under test is + // whichever config is defined by Module. This field is then set during the + // configuration load process and should be used when the test is executed. + ConfigUnderTest *Config + // ExpectFailures should be a list of checkable objects that are expected // to report a failure from their custom conditions as part of this test // run. @@ -88,6 +108,19 @@ type TestRun struct { DeclRange hcl.Range } +// TestRunModuleCall specifies which module should be executed by a given run +// block. +type TestRunModuleCall struct { + // Source is the source of the module to test. + Source addrs.ModuleSource + + // Version is the version of the module to load from the registry. + Version VersionConstraint + + DeclRange hcl.Range + SourceDeclRange hcl.Range +} + // TestRunOptions contains the plan options for a given run block. type TestRunOptions struct { // Mode is the planning mode to run in. One of ['normal', 'refresh-only']. @@ -200,7 +233,21 @@ func decodeTestRunBlock(block *hcl.Block) (*TestRun, hcl.Diagnostics) { for _, v := range vars { r.Variables[v.Name] = v.Expr } + case "module": + if r.Module != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Multiple \"module\" blocks", + Detail: fmt.Sprintf("This run block already has a module block defined at %s.", r.Module.DeclRange), + Subject: block.DefRange.Ptr(), + }) + } + module, moduleDiags := decodeTestRunModuleBlock(block) + diags = append(diags, moduleDiags...) + if !moduleDiags.HasErrors() { + r.Module = module + } } } @@ -247,6 +294,110 @@ func decodeTestRunBlock(block *hcl.Block) (*TestRun, hcl.Diagnostics) { return &r, diags } +func decodeTestRunModuleBlock(block *hcl.Block) (*TestRunModuleCall, hcl.Diagnostics) { + var diags hcl.Diagnostics + + content, contentDiags := block.Body.Content(testRunModuleBlockSchema) + diags = append(diags, contentDiags...) + + module := TestRunModuleCall{ + DeclRange: block.DefRange, + } + + haveVersionArg := false + if attr, exists := content.Attributes["version"]; exists { + var versionDiags hcl.Diagnostics + module.Version, versionDiags = decodeVersionConstraint(attr) + diags = append(diags, versionDiags...) + haveVersionArg = true + } + + if attr, exists := content.Attributes["source"]; exists { + module.SourceDeclRange = attr.Range + + var raw string + rawDiags := gohcl.DecodeExpression(attr.Expr, nil, &raw) + diags = append(diags, rawDiags...) + if !rawDiags.HasErrors() { + var err error + if haveVersionArg { + module.Source, err = addrs.ParseModuleSourceRegistry(raw) + } else { + module.Source, err = addrs.ParseModuleSource(raw) + } + if err != nil { + // NOTE: We leave mc.SourceAddr as nil for any situation where the + // source attribute is invalid, so any code which tries to carefully + // use the partial result of a failed config decode must be + // resilient to that. + module.Source = nil + + // NOTE: In practice it's actually very unlikely to end up here, + // because our source address parser can turn just about any string + // into some sort of remote package address, and so for most errors + // we'll detect them only during module installation. There are + // still a _few_ purely-syntax errors we can catch at parsing time, + // though, mostly related to remote package sub-paths and local + // paths. + switch err := err.(type) { + case *getmodules.MaybeRelativePathErr: + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid module source address", + Detail: fmt.Sprintf( + "Terraform failed to determine your intended installation method for remote module package %q.\n\nIf you intended this as a path relative to the current module, use \"./%s\" instead. The \"./\" prefix indicates that the address is a relative filesystem path.", + err.Addr, err.Addr, + ), + Subject: module.SourceDeclRange.Ptr(), + }) + default: + if haveVersionArg { + // In this case we'll include some extra context that + // we assumed a registry source address due to the + // version argument. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid registry module source address", + Detail: fmt.Sprintf("Failed to parse module registry address: %s.\n\nTerraform assumed that you intended a module registry source address because you also set the argument \"version\", which applies only to registry modules.", err), + Subject: module.SourceDeclRange.Ptr(), + }) + } else { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid module source address", + Detail: fmt.Sprintf("Failed to parse module source address: %s.", err), + Subject: module.SourceDeclRange.Ptr(), + }) + } + } + } + + switch module.Source.(type) { + case addrs.ModuleSourceRemote: + // We only support local or registry modules when loading + // modules directly from alternate sources during a test + // execution. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid module source address", + Detail: "Only local or registry module sources are currently supported from within test run blocks.", + Subject: module.SourceDeclRange.Ptr(), + }) + } + } + } else { + // Must have a source attribute. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Missing \"source\" attribute for module block", + Detail: "You must specify a source attribute when executing alternate modules during test executions.", + Subject: module.DeclRange.Ptr(), + }) + } + + return &module, diags +} + func decodeTestRunOptionsBlock(block *hcl.Block) (*TestRunOptions, hcl.Diagnostics) { var diags hcl.Diagnostics @@ -334,6 +485,9 @@ var testRunBlockSchema = &hcl.BodySchema{ { Type: "variables", }, + { + Type: "module", + }, }, } @@ -345,3 +499,10 @@ var testRunOptionsBlockSchema = &hcl.BodySchema{ {Name: "target"}, }, } + +var testRunModuleBlockSchema = &hcl.BodySchema{ + Attributes: []hcl.AttributeSchema{ + {Name: "source"}, + {Name: "version"}, + }, +} diff --git a/internal/configs/testdata/valid-modules/with-tests-module/main.tf b/internal/configs/testdata/valid-modules/with-tests-module/main.tf new file mode 100644 index 0000000000..3b60dc01cc --- /dev/null +++ b/internal/configs/testdata/valid-modules/with-tests-module/main.tf @@ -0,0 +1,12 @@ + +variable "managed_id" { + type = string +} + +data "test_data_source" "managed_data" { + id = var.managed_id +} + +resource "test_resource" "created" { + value = data.test_data_source.managed_data.value +} diff --git a/internal/configs/testdata/valid-modules/with-tests-module/main.tftest b/internal/configs/testdata/valid-modules/with-tests-module/main.tftest new file mode 100644 index 0000000000..1f2a6a94aa --- /dev/null +++ b/internal/configs/testdata/valid-modules/with-tests-module/main.tftest @@ -0,0 +1,21 @@ +variables { + managed_id = "B853C121" +} + +run "setup" { + module { + source = "./setup" + } + + variables { + value = "Hello, world!" + id = "B853C121" + } +} + +run "test" { + assert { + condition = test_resource.created.value == "Hello, world!" + error_message = "bad value" + } +} diff --git a/internal/configs/testdata/valid-modules/with-tests-module/setup/main.tf b/internal/configs/testdata/valid-modules/with-tests-module/setup/main.tf new file mode 100644 index 0000000000..49056bbea7 --- /dev/null +++ b/internal/configs/testdata/valid-modules/with-tests-module/setup/main.tf @@ -0,0 +1,13 @@ +variable "value" { + type = string +} + +variable "id" { + type = string +} + +resource "test_resource" "managed" { + provider = setup + id = var.id + value = var.value +} diff --git a/internal/initwd/module_install.go b/internal/initwd/module_install.go index a53008f629..3c50c98093 100644 --- a/internal/initwd/module_install.go +++ b/internal/initwd/module_install.go @@ -89,7 +89,7 @@ func (i *ModuleInstaller) InstallModules(ctx context.Context, rootDir string, up log.Printf("[TRACE] ModuleInstaller: installing child modules for %s into %s", rootDir, i.modsDir) var diags tfdiags.Diagnostics - rootMod, mDiags := i.loader.Parser().LoadConfigDir(rootDir) + rootMod, mDiags := i.loader.Parser().LoadConfigDirWithTests(rootDir, "tests") if rootMod == nil { // We drop the diagnostics here because we only want to report module // loading errors after checking the core version constraints, which we diff --git a/internal/initwd/module_install_test.go b/internal/initwd/module_install_test.go index 460838930a..c8235fdd38 100644 --- a/internal/initwd/module_install_test.go +++ b/internal/initwd/module_install_test.go @@ -19,6 +19,7 @@ import ( "github.com/google/go-cmp/cmp" version "github.com/hashicorp/go-version" svchost "github.com/hashicorp/terraform-svchost" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configload" @@ -703,6 +704,159 @@ func TestLoaderInstallModules_goGetter(t *testing.T) { } +func TestModuleInstaller_fromTests(t *testing.T) { + fixtureDir := filepath.Clean("testdata/local-module-from-test") + dir, done := tempChdir(t, fixtureDir) + defer done() + + hooks := &testInstallHooks{} + + modulesDir := filepath.Join(dir, ".terraform/modules") + loader, close := configload.NewLoaderForTests(t) + defer close() + inst := NewModuleInstaller(modulesDir, loader, nil) + _, diags := inst.InstallModules(context.Background(), ".", false, hooks) + assertNoDiagnostics(t, diags) + + wantCalls := []testInstallHookCall{ + { + Name: "Install", + ModuleAddr: "test.tests.main.setup", + PackageAddr: "", + LocalPath: "setup", + }, + } + + if assertResultDeepEqual(t, hooks.Calls, wantCalls) { + return + } + + loader, err := configload.NewLoader(&configload.Config{ + ModulesDir: modulesDir, + }) + if err != nil { + t.Fatal(err) + } + + // Make sure the configuration is loadable now. + // (This ensures that correct information is recorded in the manifest.) + config, loadDiags := loader.LoadConfigWithTests(".", "tests") + assertNoDiagnostics(t, tfdiags.Diagnostics{}.Append(loadDiags)) + + if config.Module.Tests["tests/main.tftest"].Runs[0].ConfigUnderTest == nil { + t.Fatalf("should have loaded config into the relevant run block but did not") + } +} + +func TestLoadInstallModules_registryFromTest(t *testing.T) { + if os.Getenv("TF_ACC") == "" { + t.Skip("this test accesses registry.terraform.io and github.com; set TF_ACC=1 to run it") + } + + fixtureDir := filepath.Clean("testdata/registry-module-from-test") + tmpDir, done := tempChdir(t, fixtureDir) + // the module installer runs filepath.EvalSymlinks() on the destination + // directory before copying files, and the resultant directory is what is + // returned by the install hooks. Without this, tests could fail on machines + // where the default temp dir was a symlink. + dir, err := filepath.EvalSymlinks(tmpDir) + if err != nil { + t.Error(err) + } + + defer done() + + hooks := &testInstallHooks{} + modulesDir := filepath.Join(dir, ".terraform/modules") + + loader, close := configload.NewLoaderForTests(t) + defer close() + inst := NewModuleInstaller(modulesDir, loader, registry.NewClient(nil, nil)) + _, diags := inst.InstallModules(context.Background(), dir, false, hooks) + assertNoDiagnostics(t, diags) + + v := version.Must(version.NewVersion("0.0.1")) + wantCalls := []testInstallHookCall{ + // the configuration builder visits each level of calls in lexicographical + // order by name, so the following list is kept in the same order. + + // setup access acctest directly. + { + Name: "Download", + ModuleAddr: "test.main.setup", + PackageAddr: "registry.terraform.io/hashicorp/module-installer-acctest/aws", // intentionally excludes the subdir because we're downloading the whole package here + Version: v, + }, + { + Name: "Install", + ModuleAddr: "test.main.setup", + Version: v, + // NOTE: This local path and the other paths derived from it below + // can vary depending on how the registry is implemented. At the + // time of writing this test, registry.terraform.io returns + // git repository source addresses and so this path refers to the + // root of the git clone, but historically the registry referred + // to GitHub-provided tar archives which meant that there was an + // extra level of subdirectory here for the typical directory + // nesting in tar archives, which would've been reflected as + // an extra segment on this path. If this test fails due to an + // additional path segment in future, then a change to the upstream + // registry might be the root cause. + LocalPath: filepath.Join(dir, ".terraform/modules/test.main.setup"), + }, + + // main.tftest.setup.child_a + // (no download because it's a relative path inside acctest_child_a) + { + Name: "Install", + ModuleAddr: "test.main.setup.child_a", + LocalPath: filepath.Join(dir, ".terraform/modules/test.main.setup/modules/child_a"), + }, + + // main.tftest.setup.child_a.child_b + // (no download because it's a relative path inside main.tftest.setup.child_a) + { + Name: "Install", + ModuleAddr: "test.main.setup.child_a.child_b", + LocalPath: filepath.Join(dir, ".terraform/modules/test.main.setup/modules/child_b"), + }, + } + + if diff := cmp.Diff(wantCalls, hooks.Calls); diff != "" { + t.Fatalf("wrong installer calls\n%s", diff) + } + + //check that the registry reponses were cached + packageAddr := addrs.ModuleRegistryPackage{ + Host: svchost.Hostname("registry.terraform.io"), + Namespace: "hashicorp", + Name: "module-installer-acctest", + TargetSystem: "aws", + } + if _, ok := inst.registryPackageVersions[packageAddr]; !ok { + t.Errorf("module versions cache was not populated\ngot: %s\nwant: key hashicorp/module-installer-acctest/aws", spew.Sdump(inst.registryPackageVersions)) + } + if _, ok := inst.registryPackageSources[moduleVersion{module: packageAddr, version: "0.0.1"}]; !ok { + t.Errorf("module download url cache was not populated\ngot: %s", spew.Sdump(inst.registryPackageSources)) + } + + loader, err = configload.NewLoader(&configload.Config{ + ModulesDir: modulesDir, + }) + if err != nil { + t.Fatal(err) + } + + // Make sure the configuration is loadable now. + // (This ensures that correct information is recorded in the manifest.) + config, loadDiags := loader.LoadConfigWithTests(".", "tests") + assertNoDiagnostics(t, tfdiags.Diagnostics{}.Append(loadDiags)) + + if config.Module.Tests["main.tftest"].Runs[0].ConfigUnderTest == nil { + t.Fatalf("should have loaded config into the relevant run block but did not") + } +} + type testInstallHooks struct { Calls []testInstallHookCall } diff --git a/internal/initwd/testdata/local-module-from-test/main.tf b/internal/initwd/testdata/local-module-from-test/main.tf new file mode 100644 index 0000000000..4263e1f121 --- /dev/null +++ b/internal/initwd/testdata/local-module-from-test/main.tf @@ -0,0 +1,2 @@ +# Keep this empty, we just want to make sure the test file loads the setup +# module. \ No newline at end of file diff --git a/internal/initwd/testdata/local-module-from-test/setup/main.tf b/internal/initwd/testdata/local-module-from-test/setup/main.tf new file mode 100644 index 0000000000..6eac7e720a --- /dev/null +++ b/internal/initwd/testdata/local-module-from-test/setup/main.tf @@ -0,0 +1,4 @@ +variable "v" { + description = "in setup module" + default = "" +} diff --git a/internal/initwd/testdata/local-module-from-test/tests/main.tftest b/internal/initwd/testdata/local-module-from-test/tests/main.tftest new file mode 100644 index 0000000000..e9a479f24f --- /dev/null +++ b/internal/initwd/testdata/local-module-from-test/tests/main.tftest @@ -0,0 +1,5 @@ +run "setup" { + module { + source = "./setup" + } +} diff --git a/internal/initwd/testdata/registry-module-from-test/main.tf b/internal/initwd/testdata/registry-module-from-test/main.tf new file mode 100644 index 0000000000..69d0720bc0 --- /dev/null +++ b/internal/initwd/testdata/registry-module-from-test/main.tf @@ -0,0 +1,2 @@ +# Deliberately empty, we just want to make sure the module is loaded from the +# tests. \ No newline at end of file diff --git a/internal/initwd/testdata/registry-module-from-test/main.tftest b/internal/initwd/testdata/registry-module-from-test/main.tftest new file mode 100644 index 0000000000..da19708133 --- /dev/null +++ b/internal/initwd/testdata/registry-module-from-test/main.tftest @@ -0,0 +1,8 @@ +run "setup" { + # We have a dedicated repo for this test module. + # See ../registry-modules/root.tf for more info. + module { + source = "hashicorp/module-installer-acctest/aws" + version = "0.0.1" + } +}