terraform test: fix crash when using nested modules from test run blocks (#33589)

internal/command/test_test.go

@@ -615,3 +615,58 @@ variable can be declared with a variable "not_real" {} block.
 		t.Errorf("should have deleted all resources on completion but left %v", provider.ResourceString())
 	}
 }
+
+func TestTest_NestedSetupModules(t *testing.T) {
+	td := t.TempDir()
+	testCopyDir(t, testFixturePath(path.Join("test", "with_nested_setup_modules")), td)
+	defer testChdir(t, td)()
+
+	provider := testing_command.NewProvider(nil)
+
+	providerSource, close := newMockProviderSource(t, map[string][]string{
+		"test": {"1.0.0"},
+	})
+	defer close()
+
+	streams, done := terminal.StreamsForTesting(t)
+	view := views.NewView(streams)
+	ui := new(cli.MockUi)
+
+	meta := Meta{
+		testingOverrides: metaOverridesForProvider(provider.Provider),
+		Ui:               ui,
+		View:             view,
+		Streams:          streams,
+		ProviderSource:   providerSource,
+	}
+
+	init := &InitCommand{
+		Meta: meta,
+	}
+
+	if code := init.Run(nil); code != 0 {
+		t.Fatalf("expected status code 0 but got %d: %s", code, ui.ErrorWriter)
+	}
+
+	command := &TestCommand{
+		Meta: meta,
+	}
+
+	code := command.Run(nil)
+	output := done(t)
+
+	printedOutput := false
+
+	if code != 0 {
+		printedOutput = true
+		t.Errorf("expected status code 0 but got %d: %s", code, output.All())
+	}
+
+	if provider.ResourceCount() > 0 {
+		if !printedOutput {
+			t.Errorf("should have deleted all resources on completion but left %s\n\n%s", provider.ResourceString(), output.All())
+		} else {
+			t.Errorf("should have deleted all resources on completion but left %s", provider.ResourceString())
+		}
+	}
+}

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

@@ -0,0 +1,2 @@
+
+resource "test_resource" "resource" {}

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

@@ -0,0 +1,14 @@
+variables {
+  value = "Hello, world!"
+}
+
+run "load_module" {
+  module {
+    source = "./setup"
+  }
+
+  assert {
+    condition     = output.value == "Hello, world!"
+    error_message = "invalid value"
+  }
+}

internal/command/testdata/test/with_nested_setup_modules/setup/main.tf

@@ -0,0 +1,14 @@
+
+variable "value" {
+  type = string
+}
+
+module "child" {
+  source = "./other"
+
+  value = var.value
+}
+
+output "value" {
+  value = module.child.value
+}

internal/command/testdata/test/with_nested_setup_modules/setup/other/main.tf

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

internal/configs/config_build.go

@@ -93,9 +93,18 @@ func buildTestModules(root *Config, walker ModuleWalker) hcl.Diagnostics {
 				// 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
+
+				// First, update the main module for this test run to behave as
+				// if it is the root module.
 				cfg.Parent = nil
-				cfg.Root = cfg
+
+				// Then we need to update the paths for this config and all
+				// children, so they think they are all relative to the root
+				// module we just created.
+				rebaseChildModule(cfg, cfg)
+
+				// Finally, link the new config back into our test run so
+				// it can be retrieved later.
 				run.ConfigUnderTest = cfg
 			}
 		}
@@ -194,6 +203,27 @@ func loadModule(root *Config, req *ModuleRequest, walker ModuleWalker) (*Config,
 	return cfg, diags
 }
 
+// rebaseChildModule updates cfg to make it act as if root is the base of the
+// module tree.
+//
+// This is used for modules loaded directly from test files. In order to load
+// them properly, and reuse the code for loading modules from normal
+// configuration files, we pretend they are children of the main configuration
+// object. Later, when it comes time for them to execute they will act as if
+// they are the root module directly.
+//
+// This function updates cfg so that it treats the provided root as the actual
+// root of this module tree. It then recurses into all the child modules and
+// does the same for them.
+func rebaseChildModule(cfg *Config, root *Config) {
+	for _, child := range cfg.Children {
+		rebaseChildModule(child, root)
+	}
+
+	cfg.Path = cfg.Path[len(root.Path):]
+	cfg.Root = root
+}
+
 // 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.

internal/configs/config_build_test.go

@@ -6,6 +6,7 @@ package configs
 import (
 	"fmt"
 	"io/ioutil"
+	"path"
 	"path/filepath"
 	"reflect"
 	"sort"
@@ -283,6 +284,86 @@ func TestBuildConfigInvalidModules(t *testing.T) {
 	}
 }
 
+func TestBuildConfig_WithNestedTestModules(t *testing.T) {
+	parser := NewParser(nil)
+	mod, diags := parser.LoadConfigDirWithTests("testdata/valid-modules/with-tests-nested-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) {
+
+			// Bit of a hack to get the test working, but we know all the source
+			// addresses in this test are locals, so we can just treat them as
+			// paths in the filesystem.
+
+			addr := req.SourceAddr.String()
+			current := req.Parent
+			for current.SourceAddr != nil {
+				addr = path.Join(current.SourceAddr.String(), addr)
+				current = current.Parent
+			}
+			sourcePath := filepath.Join("testdata/valid-modules/with-tests-nested-module", addr)
+
+			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.hcl"]
+	if len(test.Runs) != 1 {
+		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")
+	}
+
+	// We should also have loaded a single child underneath the config under
+	// test, and it should have valid paths.
+
+	child := run.ConfigUnderTest.Children["child"]
+
+	if child.Parent != run.ConfigUnderTest {
+		t.Errorf("child should point back to root")
+	}
+
+	if len(child.Path) != 1 || child.Path[0] != "child" {
+		t.Errorf("child should have rebased against virtual root")
+	}
+
+	if child.Root != run.ConfigUnderTest {
+		t.Errorf("child root should be main config under test")
+	}
+}
+
 func TestBuildConfig_WithTestModule(t *testing.T) {
 	parser := NewParser(nil)
 	mod, diags := parser.LoadConfigDirWithTests("testdata/valid-modules/with-tests-module", "tests")

internal/configs/testdata/valid-modules/with-tests-nested-module/main.tf

@@ -0,0 +1,2 @@
+
+resource "test_resource" "resource" {}

internal/configs/testdata/valid-modules/with-tests-nested-module/main.tftest.hcl

@@ -0,0 +1,14 @@
+variables {
+  value = "Hello, world!"
+}
+
+run "load_module" {
+  module {
+    source = "./setup"
+  }
+
+  assert {
+    condition     = output.value == "Hello, world!"
+    error_message = "invalid value"
+  }
+}

internal/configs/testdata/valid-modules/with-tests-nested-module/setup/main.tf

@@ -0,0 +1,14 @@
+
+variable "value" {
+  type = string
+}
+
+module "child" {
+  source = "./other"
+
+  value = var.value
+}
+
+output "value" {
+  value = module.child.value
+}

internal/configs/testdata/valid-modules/with-tests-nested-module/setup/other/main.tf

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