diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 7b22698453c5..c5ada820c2f6 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -305,9 +305,6 @@ func TestTest_Runs(t *testing.T) { }, } for name, tc := range tcs { - if name != "shared_state_parallel" { - continue - } t.Run(name, func(t *testing.T) { if tc.skip { t.Skip() @@ -506,24 +503,31 @@ func TestTest_Parallel(t *testing.T) { // Split the log into lines lines := strings.Split(output, "\n") - // Find the positions of "test_d" and "test_c" - var testDIndex, testCIndex int + // Find the positions of "test_d", "test_c", "test_setup" in the log output + var testDIndex, testCIndex, testSetupIndex int for i, line := range lines { - if strings.Contains(line, "run \"test_d\"") { + if strings.Contains(line, "run \"setup\"") { + testSetupIndex = i + } else if strings.Contains(line, "run \"test_d\"") { testDIndex = i } else if strings.Contains(line, "run \"test_c\"") { testCIndex = i } } + if testDIndex == 0 || testCIndex == 0 || testSetupIndex == 0 { + t.Fatalf("test_d, test_c, or test_setup not found in the log output") + } // Ensure "test_d" appears before "test_c", because test_d has no dependencies, // and would therefore run in parallel to much earlier tests which test_c depends on. - if testDIndex == 0 || testCIndex == 0 { - t.Fatalf("Could not find both test_d and test_c in the output") - } if testDIndex > testCIndex { t.Errorf("test_d appears after test_c in the log output") } + + // Ensure "test_d" appears after "test_setup", because they have the same state key + if testDIndex < testSetupIndex { + t.Errorf("test_d appears before test_setup in the log output") + } } func TestTest_InterruptSkipsRemaining(t *testing.T) { diff --git a/internal/command/testdata/test/shared_state_parallel/parallel.tftest.hcl b/internal/command/testdata/test/shared_state_parallel/parallel.tftest.hcl index 55621a62e012..89ad20c00ad2 100644 --- a/internal/command/testdata/test/shared_state_parallel/parallel.tftest.hcl +++ b/internal/command/testdata/test/shared_state_parallel/parallel.tftest.hcl @@ -9,6 +9,7 @@ variables { run "setup" { parallel = true + state_key = "test_d" module { source = "./setup" } @@ -81,6 +82,7 @@ run "test_c" { // Does not depend on previous run, and has different state key, so would run in parallel // NotDepends: true // DiffStateKey: true +// However, it has a state key that is the same as a previous run, so it should wait for that run. run "test_d" { parallel = true state_key = "test_d" @@ -93,3 +95,19 @@ run "test_d" { error_message = "double bad" } } + +// Does not depend on previous run, and has different state key, so would run in parallel +// NotDepends: true +// DiffStateKey: true +# run "test_d" { +# parallel = true +# state_key = "test_d" +# variables { +# input = "foo" +# } + +# assert { +# condition = output.value == var.foo +# error_message = "double bad" +# } +# } diff --git a/internal/terraformtest/node_test_run.go b/internal/terraformtest/node_test_run.go index db605324cdc0..5d218a754020 100644 --- a/internal/terraformtest/node_test_run.go +++ b/internal/terraformtest/node_test_run.go @@ -6,14 +6,12 @@ package terraformtest import ( "fmt" - "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/moduletest" ) type NodeTestRun struct { - file *moduletest.File - run *moduletest.Run - config *configs.Config + file *moduletest.File + run *moduletest.Run } func (n *NodeTestRun) Run() *moduletest.Run { diff --git a/internal/terraformtest/transform_provider.go b/internal/terraformtest/transform_provider.go deleted file mode 100644 index d08ec0e16ba0..000000000000 --- a/internal/terraformtest/transform_provider.go +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: BUSL-1.1 - -package terraformtest - -import ( - "github.com/hashicorp/terraform/internal/backend/backendrun" - "github.com/hashicorp/terraform/internal/configs" - "github.com/hashicorp/terraform/internal/moduletest" - "github.com/hashicorp/terraform/internal/terraform" -) - -// ProviderTransformer is a GraphTransformer that adds all the test runs, -// and the variables defined in each run block, to the graph. -type ProviderTransformer struct { - File *moduletest.File - config *configs.Config - globalVars map[string]backendrun.UnparsedVariableValue -} - -func (t *ProviderTransformer) Transform(g *terraform.Graph) error { - - return nil -} - -type nodeProvider struct { -} - -// func (n *nodeProvider) Execute(ctx *hcltest.ExecContext, g *terraform.Graph) (tfdiags.Diagnostics, error) { -// runner.Suite.configLock.Lock() -// defer runner.Suite.configLock.Unlock() -// if _, exists := runner.Suite.configProviders[key]; exists { -// // Then we've processed this key before, so skip it. -// return -// } - -// providers := make(map[string]bool) - -// // First, let's look at the required providers first. -// for _, provider := range config.Module.ProviderRequirements.RequiredProviders { -// providers[provider.Name] = true -// for _, alias := range provider.Aliases { -// providers[alias.StringCompact()] = true -// } -// } - -// // Second, we look at the defined provider configs. -// for _, provider := range config.Module.ProviderConfigs { -// providers[provider.Addr().StringCompact()] = true -// } - -// // Third, we look at the resources and data sources. -// for _, resource := range config.Module.ManagedResources { -// if resource.ProviderConfigRef != nil { -// providers[resource.ProviderConfigRef.String()] = true -// continue -// } -// providers[resource.Provider.Type] = true -// } -// for _, datasource := range config.Module.DataResources { -// if datasource.ProviderConfigRef != nil { -// providers[datasource.ProviderConfigRef.String()] = true -// continue -// } -// providers[datasource.Provider.Type] = true -// } - -// // Finally, we look at any module calls to see if any providers are used -// // in there. -// for _, module := range config.Module.ModuleCalls { -// for _, provider := range module.Providers { -// providers[provider.InParent.String()] = true -// } -// } - -// runner.Suite.configProviders[key] = providers -// } diff --git a/internal/terraformtest/transform_test_run.go b/internal/terraformtest/transform_test_run.go index 748e93aee476..c82a511a8505 100644 --- a/internal/terraformtest/transform_test_run.go +++ b/internal/terraformtest/transform_test_run.go @@ -26,27 +26,45 @@ type TestRunTransformer struct { } func (t *TestRunTransformer) Transform(g *terraform.Graph) error { - var prev *NodeTestRun var errs []error - runsSoFar := make(map[string]*NodeTestRun) - for _, run := range t.File.Runs { - // If we're testing a specific configuration, we need to use that - config := t.config - if run.Config.ConfigUnderTest != nil { - config = run.Config.ConfigUnderTest - } - node := &NodeTestRun{run: run, file: t.File, config: config} - g.Add(node) + // Create and add nodes for each run + nodes, err := t.createNodes(g) + if err != nil { + return err + } - // parallelized sequential runs are only connected if they have the same state key or if they depend on each other - if prev != nil && prev.run.GetStateKey() == run.GetStateKey() && prev.run.Config.Parallel && run.Config.Parallel { - g.Connect(dag.BasicEdge(node, prev)) - } - prev = node + // Connect nodes based on dependencies + if err := t.connectDependencies(g, nodes); err != nil { + errs = append(errs, err) + } - // Connect the run to all the other runs that it depends on - refs, err := getRefs(run) + // Connect nodes with the same state key sequentially + if err := t.connectStateKeyRuns(g, nodes); err != nil { + errs = append(errs, err) + } + + return errors.Join(errs...) +} + +func (t *TestRunTransformer) createNodes(g *terraform.Graph) ([]*NodeTestRun, error) { + var nodes []*NodeTestRun + for _, run := range t.File.Runs { + node := &NodeTestRun{run: run, file: t.File} + g.Add(node) + nodes = append(nodes, node) + } + return nodes, nil +} + +func (t *TestRunTransformer) connectDependencies(g *terraform.Graph, nodes []*NodeTestRun) error { + var errs []error + nodeMap := make(map[string]*NodeTestRun) + for _, node := range nodes { + nodeMap[node.run.Name] = node + } + for _, node := range nodes { + refs, err := getRefs(node.run) if err != nil { return err } @@ -59,19 +77,31 @@ func (t *TestRunTransformer) Transform(g *terraform.Graph) error { if runName == "" { continue } - dependency, ok := runsSoFar[runName] + dependency, ok := nodeMap[runName] if !ok { - errs = append(errs, fmt.Errorf("dependency `run.%s` not found for run %q", runName, run.Name)) + errs = append(errs, fmt.Errorf("dependency `run.%s` not found for run %q", runName, node.run.Name)) continue } g.Connect(dag.BasicEdge(node, dependency)) } - runsSoFar[run.Name] = node } - return errors.Join(errs...) } +func (t *TestRunTransformer) connectStateKeyRuns(g *terraform.Graph, nodes []*NodeTestRun) error { + stateRuns := make(map[string][]*NodeTestRun) + for _, node := range nodes { + key := node.run.GetStateKey() + stateRuns[key] = append(stateRuns[key], node) + } + for _, runs := range stateRuns { + for i := 1; i < len(runs); i++ { + g.Connect(dag.BasicEdge(runs[i], runs[i-1])) + } + } + return nil +} + func getRefs(run *moduletest.Run) ([]*addrs.Reference, error) { refs, refDiags := run.GetReferences() if refDiags.HasErrors() {