diff --git a/.changes/v1.13/BUG FIXES-20250704-182248.yaml b/.changes/v1.13/BUG FIXES-20250704-182248.yaml new file mode 100644 index 0000000000..bf922073f2 --- /dev/null +++ b/.changes/v1.13/BUG FIXES-20250704-182248.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: Test run Parallelism of 1 should not result in deadlock +time: 2025-07-04T18:22:48.934287+02:00 +custom: + Issue: "37292" diff --git a/internal/command/arguments/test.go b/internal/command/arguments/test.go index 3a11394489..ecf100506a 100644 --- a/internal/command/arguments/test.go +++ b/internal/command/arguments/test.go @@ -22,6 +22,10 @@ type Test struct { // during the plan or apply command within a single test run. OperationParallelism int + // RunParallelism is the limit Terraform places on parallel test runs. This + // is the number of test runs that can be executed in parallel within a file. + RunParallelism int + // TestDirectory allows the user to override the directory that the test // command will use to discover test files, defaults to "tests". Regardless // of the value here, test files within the configuration directory will @@ -60,6 +64,7 @@ func ParseTest(args []string) (*Test, tfdiags.Diagnostics) { cmdFlags.StringVar(&test.JUnitXMLFile, "junit-xml", "", "junit-xml") cmdFlags.BoolVar(&test.Verbose, "verbose", false, "verbose") cmdFlags.IntVar(&test.OperationParallelism, "parallelism", DefaultParallelism, "parallelism") + cmdFlags.IntVar(&test.RunParallelism, "run-parallelism", DefaultParallelism, "run-parallelism") // TODO: Finalise the name of this flag. cmdFlags.StringVar(&test.CloudRunSource, "cloud-run", "", "cloud-run") diff --git a/internal/command/arguments/test_test.go b/internal/command/arguments/test_test.go index dc3849ff93..b4f5addf0e 100644 --- a/internal/command/arguments/test_test.go +++ b/internal/command/arguments/test_test.go @@ -77,6 +77,7 @@ func TestParseTest(t *testing.T) { ViewType: ViewHuman, Vars: &Vars{}, OperationParallelism: 10, + RunParallelism: 10, }, wantDiags: nil, }, @@ -88,6 +89,7 @@ func TestParseTest(t *testing.T) { ViewType: ViewHuman, Vars: &Vars{}, OperationParallelism: 10, + RunParallelism: 10, }, wantDiags: nil, }, @@ -99,6 +101,7 @@ func TestParseTest(t *testing.T) { ViewType: ViewJSON, Vars: &Vars{}, OperationParallelism: 10, + RunParallelism: 10, }, wantDiags: nil, }, @@ -110,6 +113,7 @@ func TestParseTest(t *testing.T) { ViewType: ViewHuman, Vars: &Vars{}, OperationParallelism: 10, + RunParallelism: 10, }, wantDiags: nil, }, @@ -122,6 +126,7 @@ func TestParseTest(t *testing.T) { Verbose: true, Vars: &Vars{}, OperationParallelism: 10, + RunParallelism: 10, }, }, "with-parallelism-set": { @@ -132,6 +137,7 @@ func TestParseTest(t *testing.T) { ViewType: ViewHuman, Vars: &Vars{}, OperationParallelism: 5, + RunParallelism: 10, }, wantDiags: nil, }, @@ -143,9 +149,11 @@ func TestParseTest(t *testing.T) { ViewType: ViewHuman, Vars: &Vars{}, OperationParallelism: 10, + RunParallelism: 10, }, wantDiags: nil, }, + "cloud-with-parallelism-0": { args: []string{"-parallelism=0", "-cloud-run=foobar"}, want: &Test{ @@ -155,6 +163,31 @@ func TestParseTest(t *testing.T) { ViewType: ViewHuman, Vars: &Vars{}, OperationParallelism: 0, + RunParallelism: 10, + }, + wantDiags: nil, + }, + "with-run-parallelism-set": { + args: []string{"-run-parallelism=10"}, + want: &Test{ + Filter: nil, + TestDirectory: "tests", + ViewType: ViewHuman, + Vars: &Vars{}, + OperationParallelism: 10, + RunParallelism: 10, + }, + wantDiags: nil, + }, + "with-run-parallelism-0": { + args: []string{"-run-parallelism=0"}, + want: &Test{ + Filter: nil, + TestDirectory: "tests", + ViewType: ViewHuman, + Vars: &Vars{}, + OperationParallelism: 10, + RunParallelism: 0, }, wantDiags: nil, }, @@ -166,6 +199,7 @@ func TestParseTest(t *testing.T) { ViewType: ViewHuman, Vars: &Vars{}, OperationParallelism: 10, + RunParallelism: 10, }, wantDiags: tfdiags.Diagnostics{ tfdiags.Sourceless( @@ -185,6 +219,7 @@ func TestParseTest(t *testing.T) { ViewType: ViewHuman, Vars: &Vars{}, OperationParallelism: 10, + RunParallelism: 10, }, wantDiags: tfdiags.Diagnostics{ tfdiags.Sourceless( diff --git a/internal/command/test.go b/internal/command/test.go index 42ff7157fa..5466560514 100644 --- a/internal/command/test.go +++ b/internal/command/test.go @@ -227,6 +227,7 @@ func (c *TestCommand) Run(rawArgs []string) int { CancelledCtx: cancelCtx, Filter: args.Filter, Verbose: args.Verbose, + Concurrency: args.RunParallelism, } // JUnit output is only compatible with local test execution diff --git a/internal/command/test_test.go b/internal/command/test_test.go index f96caf4bfc..6b2356d474 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -62,6 +62,11 @@ func TestTest_Runs(t *testing.T) { expectedOut: []string{"1 passed, 0 failed."}, code: 0, }, + "simple_pass_count": { + expectedOut: []string{"1 passed, 0 failed."}, + args: []string{"-run-parallelism", "1"}, + code: 0, + }, "simple_pass_nested_alternate": { args: []string{"-test-directory", "other"}, expectedOut: []string{"1 passed, 0 failed."}, diff --git a/internal/command/testdata/test/simple_pass_count/main.tf b/internal/command/testdata/test/simple_pass_count/main.tf new file mode 100644 index 0000000000..6ade4a4a07 --- /dev/null +++ b/internal/command/testdata/test/simple_pass_count/main.tf @@ -0,0 +1,4 @@ +resource "test_resource" "foo" { + count = 3 + value = "bar" +} diff --git a/internal/command/testdata/test/simple_pass_count/main.tftest.hcl b/internal/command/testdata/test/simple_pass_count/main.tftest.hcl new file mode 100644 index 0000000000..8ecf8e16c9 --- /dev/null +++ b/internal/command/testdata/test/simple_pass_count/main.tftest.hcl @@ -0,0 +1,6 @@ +run "validate_test_resource" { + assert { + condition = test_resource.foo[0].value == "bar" + error_message = "invalid value" + } +} diff --git a/internal/moduletest/graph/test_graph_builder.go b/internal/moduletest/graph/test_graph_builder.go index 1881e2dae2..425d59f008 100644 --- a/internal/moduletest/graph/test_graph_builder.go +++ b/internal/moduletest/graph/test_graph_builder.go @@ -129,8 +129,12 @@ func Walk(g *terraform.Graph, ctx *EvalContext) tfdiags.Diagnostics { log.Printf("[TRACE] vertex %q: visit complete", dag.VertexName(v)) }() - ctx.evalSem.Acquire() - defer ctx.evalSem.Release() + // expandable nodes are not executed, but they are walked and + // their children are executed, so they need not acquire the semaphore themselves. + if _, ok := v.(Subgrapher); !ok { + ctx.evalSem.Acquire() + defer ctx.evalSem.Release() + } if executable, ok := v.(GraphNodeExecutable); ok { executable.Execute(ctx) diff --git a/internal/moduletest/graph/transform_state_cleanup.go b/internal/moduletest/graph/transform_state_cleanup.go index ced8e2903b..a08286a8db 100644 --- a/internal/moduletest/graph/transform_state_cleanup.go +++ b/internal/moduletest/graph/transform_state_cleanup.go @@ -12,7 +12,14 @@ import ( "github.com/hashicorp/terraform/internal/terraform" ) -var _ GraphNodeExecutable = &TeardownSubgraph{} +var ( + _ GraphNodeExecutable = &TeardownSubgraph{} + _ Subgrapher = &TeardownSubgraph{} +) + +type Subgrapher interface { + isSubGrapher() +} // TeardownSubgraph is a subgraph for cleaning up the state of // resources defined in the state files created by the test runs. @@ -54,6 +61,8 @@ func (b *TeardownSubgraph) Execute(ctx *EvalContext) { b.opts.File.AppendDiagnostics(diags) } +func (b *TeardownSubgraph) isSubGrapher() {} + // TestStateCleanupTransformer is a GraphTransformer that adds a cleanup node // for each state that is created by the test runs. type TestStateCleanupTransformer struct {