From 204a9f844658ec907bb424aeb8b3e2861845c2e6 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 17 Mar 2026 20:59:49 +0100 Subject: [PATCH 01/12] Run acceptance tests in phases Add a non-inheritable Phase setting to test.toml so acceptance tests can wait for earlier phases before running, and mark the template and permissions tests that must run after phase 0. --- acceptance/acceptance_test.go | 106 ++++++++++++++++++ .../resources/permissions/out.test.toml | 1 + .../bundle/resources/permissions/test.toml | 1 + .../default-python/classic/out.test.toml | 1 + .../default-python/classic/test.toml | 1 + .../serverless-customcatalog/out.test.toml | 1 + .../serverless-customcatalog/test.toml | 2 + acceptance/internal/config.go | 20 ++++ acceptance/internal/config_test.go | 61 ++++++++++ acceptance/internal/materialized_config.go | 7 ++ .../internal/materialized_config_test.go | 43 +++++++ acceptance/phase_test.go | 91 +++++++++++++++ 12 files changed, 335 insertions(+) create mode 100644 acceptance/internal/materialized_config_test.go create mode 100644 acceptance/phase_test.go diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 695027f0a2..13b34c1642 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -20,6 +20,7 @@ import ( "sort" "strconv" "strings" + "sync" "testing" "time" "unicode/utf8" @@ -121,6 +122,18 @@ var Ignored = map[string]bool{ userReplacementsFilename: true, } +type testPhase struct { + Phase int + Dirs []string +} + +type phaseScheduler struct { + mu sync.Mutex + remaining map[int]int + next map[int]int + gates map[int]chan struct{} +} + func TestAccept(t *testing.T) { testAccept(t, InprocessMode, "") } @@ -287,6 +300,10 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { require.NotEmpty(t, testDirs, "singleTest=%#v did not match any tests\n%#v", singleTest, testDirs) } + testPhases := collectTestPhases(t, testDirs) + testDirs = flattenTestPhases(testPhases) + scheduler := newPhaseScheduler(testPhases) + skippedDirs := 0 totalDirs := 0 selectedDirs := 0 @@ -300,6 +317,7 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { selectedDirs += 1 config, configPath := internal.LoadConfig(t, dir) + defer scheduler.Done(config.Phase) skipReason := getSkipReason(&config, configPath) // Generate materialized config for this test. @@ -328,6 +346,8 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { t.Parallel() } + scheduler.Wait(config.Phase) + // Build extra vars for exclusion matching (config state as env vars) var extraVars []string if cloudEnv != "" { @@ -405,6 +425,92 @@ func getTests(t *testing.T) []string { return testDirs } +func collectTestPhases(t *testing.T, testDirs []string) []testPhase { + phaseDirs := make(map[int][]string) + + for _, dir := range testDirs { + config, _ := internal.LoadConfig(t, dir) + phaseDirs[config.Phase] = append(phaseDirs[config.Phase], dir) + } + + phases := make([]int, 0, len(phaseDirs)) + for phase := range phaseDirs { + phases = append(phases, phase) + } + sort.Ints(phases) + + result := make([]testPhase, 0, len(phases)) + for _, phase := range phases { + dirs := phaseDirs[phase] + sort.Strings(dirs) + result = append(result, testPhase{ + Phase: phase, + Dirs: dirs, + }) + } + + return result +} + +func flattenTestPhases(testPhases []testPhase) []string { + totalDirs := 0 + for _, phase := range testPhases { + totalDirs += len(phase.Dirs) + } + + result := make([]string, 0, totalDirs) + for _, phase := range testPhases { + result = append(result, phase.Dirs...) + } + + return result +} + +func newPhaseScheduler(testPhases []testPhase) *phaseScheduler { + remaining := make(map[int]int, len(testPhases)) + next := make(map[int]int, len(testPhases)) + gates := make(map[int]chan struct{}, len(testPhases)) + + for i, phase := range testPhases { + remaining[phase.Phase] = len(phase.Dirs) + gates[phase.Phase] = make(chan struct{}) + if i+1 < len(testPhases) { + next[phase.Phase] = testPhases[i+1].Phase + } + } + + if len(testPhases) > 0 { + close(gates[testPhases[0].Phase]) + } + + return &phaseScheduler{ + remaining: remaining, + next: next, + gates: gates, + } +} + +func (s *phaseScheduler) Wait(phase int) { + <-s.gates[phase] +} + +func (s *phaseScheduler) Done(phase int) { + s.mu.Lock() + defer s.mu.Unlock() + + s.remaining[phase]-- + if s.remaining[phase] != 0 { + return + } + + next, ok := s.next[phase] + if !ok { + return + } + + close(s.gates[next]) +} + // Return a reason to skip the test. Empty string means "don't skip". func getSkipReason(config *internal.TestConfig, configPath string) string { // Apply default first, so that it's visible in out.test.toml diff --git a/acceptance/bundle/resources/permissions/out.test.toml b/acceptance/bundle/resources/permissions/out.test.toml index d560f1de04..4cfe03e9f9 100644 --- a/acceptance/bundle/resources/permissions/out.test.toml +++ b/acceptance/bundle/resources/permissions/out.test.toml @@ -1,5 +1,6 @@ Local = true Cloud = false +Phase = 1 [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/resources/permissions/test.toml b/acceptance/bundle/resources/permissions/test.toml index 4a1a3bb8f5..7cb7c1def5 100644 --- a/acceptance/bundle/resources/permissions/test.toml +++ b/acceptance/bundle/resources/permissions/test.toml @@ -1,2 +1,3 @@ +Phase = 1 RecordRequests = true Ignore = ['.databricks'] diff --git a/acceptance/bundle/templates/default-python/classic/out.test.toml b/acceptance/bundle/templates/default-python/classic/out.test.toml index c820fbee96..3c8bb15e9f 100644 --- a/acceptance/bundle/templates/default-python/classic/out.test.toml +++ b/acceptance/bundle/templates/default-python/classic/out.test.toml @@ -1,5 +1,6 @@ Local = true Cloud = false +Phase = 1 [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/templates/default-python/classic/test.toml b/acceptance/bundle/templates/default-python/classic/test.toml index 0b1b996a83..68f298aca3 100644 --- a/acceptance/bundle/templates/default-python/classic/test.toml +++ b/acceptance/bundle/templates/default-python/classic/test.toml @@ -1,2 +1,3 @@ +Phase = 1 RecordRequests = true EnvMatrix.READPLAN = ["", "1"] diff --git a/acceptance/bundle/templates/default-python/serverless-customcatalog/out.test.toml b/acceptance/bundle/templates/default-python/serverless-customcatalog/out.test.toml index d560f1de04..4cfe03e9f9 100644 --- a/acceptance/bundle/templates/default-python/serverless-customcatalog/out.test.toml +++ b/acceptance/bundle/templates/default-python/serverless-customcatalog/out.test.toml @@ -1,5 +1,6 @@ Local = true Cloud = false +Phase = 1 [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/templates/default-python/serverless-customcatalog/test.toml b/acceptance/bundle/templates/default-python/serverless-customcatalog/test.toml index 4029057bea..a9b8e0859e 100644 --- a/acceptance/bundle/templates/default-python/serverless-customcatalog/test.toml +++ b/acceptance/bundle/templates/default-python/serverless-customcatalog/test.toml @@ -1,3 +1,5 @@ +Phase = 1 + [[Server]] Pattern = "GET /api/2.1/unity-catalog/current-metastore-assignment" Response.Body = '{"default_catalog_name": "customcatalog"}' diff --git a/acceptance/internal/config.go b/acceptance/internal/config.go index 376047c9ca..783c340fb9 100644 --- a/acceptance/internal/config.go +++ b/acceptance/internal/config.go @@ -24,6 +24,10 @@ type TestConfig struct { // Place to describe what's wrong with this test. Does not affect how the test is run. Badness *string + // Execution phase for this test. Tests run in ascending phase order. + // Phase is not inherited from parent test.toml files. Default is 0. + Phase int + // Which OSes the test is enabled on. Each string is compared against runtime.GOOS. // If absent, default to true. GOOS map[string]bool @@ -194,9 +198,19 @@ func LoadConfig(t *testing.T, dir string) (TestConfig, string) { } result := DoLoadConfig(t, configs[0]) + leafConfigPath := filepath.Join(dir, configFilename) + leafConfig := TestConfig{} + hasLeafConfig := configs[0] == leafConfigPath + if hasLeafConfig { + leafConfig = result + } for _, cfgName := range configs[1:] { cfg := DoLoadConfig(t, cfgName) + if cfgName == leafConfigPath { + leafConfig = cfg + hasLeafConfig = true + } err := mergo.Merge( &result, cfg, @@ -210,6 +224,12 @@ func LoadConfig(t *testing.T, dir string) (TestConfig, string) { } } + if hasLeafConfig { + result.Phase = leafConfig.Phase + } else { + result.Phase = 0 + } + // Always ignore .cache directory (used by local cache) result.Ignore = append(result.Ignore, ".cache") result.CompiledIgnoreObject = ignore.CompileIgnoreLines(result.Ignore...) diff --git a/acceptance/internal/config_test.go b/acceptance/internal/config_test.go index c2701337f5..d21c4bcb6c 100644 --- a/acceptance/internal/config_test.go +++ b/acceptance/internal/config_test.go @@ -1,9 +1,12 @@ package internal import ( + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestExpandEnvMatrix(t *testing.T) { @@ -197,3 +200,61 @@ func TestExpandEnvMatrix(t *testing.T) { }) } } + +func TestLoadConfigPhaseIsNotInherited(t *testing.T) { + tests := []struct { + name string + files map[string]string + dir string + wantPhase int + wantConfig string + }{ + { + name: "missing leaf config defaults to zero", + files: map[string]string{ + "test.toml": "Phase = 3\n", + }, + dir: "suite/test", + wantPhase: 0, + wantConfig: "test.toml", + }, + { + name: "leaf config without phase resets inherited value", + files: map[string]string{ + "test.toml": "Phase = 3\n", + "suite/test/test.toml": "Local = true\n", + }, + dir: "suite/test", + wantPhase: 0, + wantConfig: "test.toml, suite/test/test.toml", + }, + { + name: "leaf phase wins", + files: map[string]string{ + "test.toml": "Phase = 3\n", + "suite/test/test.toml": "Local = true\nPhase = 1\n", + }, + dir: "suite/test", + wantPhase: 1, + wantConfig: "test.toml, suite/test/test.toml", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := t.TempDir() + t.Chdir(root) + + for path, contents := range tt.files { + absPath := filepath.Join(root, filepath.FromSlash(path)) + require.NoError(t, os.MkdirAll(filepath.Dir(absPath), 0o755)) + require.NoError(t, os.WriteFile(absPath, []byte(contents), 0o644)) + } + + config, configPath := LoadConfig(t, tt.dir) + + assert.Equal(t, tt.wantPhase, config.Phase) + assert.Equal(t, tt.wantConfig, configPath) + }) + } +} diff --git a/acceptance/internal/materialized_config.go b/acceptance/internal/materialized_config.go index 9ab0dc18cb..a1f30c841c 100644 --- a/acceptance/internal/materialized_config.go +++ b/acceptance/internal/materialized_config.go @@ -18,12 +18,18 @@ type MaterializedConfig struct { RequiresCluster *bool `toml:"RequiresCluster,omitempty"` RequiresWarehouse *bool `toml:"RequiresWarehouse,omitempty"` RunsOnDbr *bool `toml:"RunsOnDbr,omitempty"` + Phase *int `toml:"Phase,omitempty"` EnvMatrix map[string][]string `toml:"EnvMatrix,omitempty"` } // GenerateMaterializedConfig creates a TOML representation of the configuration fields // that determine where and how a test is executed func GenerateMaterializedConfig(config TestConfig) (string, error) { + var phase *int + if config.Phase != 0 { + phase = &config.Phase + } + materialized := MaterializedConfig{ GOOS: config.GOOS, CloudEnvs: config.CloudEnvs, @@ -34,6 +40,7 @@ func GenerateMaterializedConfig(config TestConfig) (string, error) { RequiresCluster: config.RequiresCluster, RequiresWarehouse: config.RequiresWarehouse, RunsOnDbr: config.RunsOnDbr, + Phase: phase, EnvMatrix: config.EnvMatrix, } diff --git a/acceptance/internal/materialized_config_test.go b/acceptance/internal/materialized_config_test.go new file mode 100644 index 0000000000..eb35eb4710 --- /dev/null +++ b/acceptance/internal/materialized_config_test.go @@ -0,0 +1,43 @@ +package internal + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGenerateMaterializedConfigIncludesPhase(t *testing.T) { + local := true + cloud := false + + config := TestConfig{ + Local: localPtr(local), + Cloud: localPtr(cloud), + Phase: 1, + } + + out, err := GenerateMaterializedConfig(config) + require.NoError(t, err) + + assert.Equal(t, "Local = true\nCloud = false\nPhase = 1\n", out) +} + +func TestGenerateMaterializedConfigOmitsDefaultPhase(t *testing.T) { + local := true + cloud := false + + config := TestConfig{ + Local: localPtr(local), + Cloud: localPtr(cloud), + } + + out, err := GenerateMaterializedConfig(config) + require.NoError(t, err) + + assert.Equal(t, "Local = true\nCloud = false\n", out) +} + +func localPtr[T any](v T) *T { + return &v +} diff --git a/acceptance/phase_test.go b/acceptance/phase_test.go new file mode 100644 index 0000000000..48aadc8c01 --- /dev/null +++ b/acceptance/phase_test.go @@ -0,0 +1,91 @@ +package acceptance_test + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCollectTestPhases(t *testing.T) { + root := t.TempDir() + t.Chdir(root) + + files := map[string]string{ + "test.toml": "Phase = 9\n", + "alpha/test.toml": "Local = true\n", + "beta/test.toml": "Phase = 1\n", + "gamma/test.toml": "Phase = 2\n", + "zeta/test.toml": "Local = true\nPhase = 1\n", + "nested/test.toml": "Phase = 4\n", + } + + for path, contents := range files { + absPath := filepath.Join(root, filepath.FromSlash(path)) + require.NoError(t, os.MkdirAll(filepath.Dir(absPath), 0o755)) + require.NoError(t, os.WriteFile(absPath, []byte(contents), 0o644)) + } + require.NoError(t, os.MkdirAll(filepath.Join(root, "nested", "leaf"), 0o755)) + + phases := collectTestPhases(t, []string{"gamma", "beta", "alpha", "zeta", "nested/leaf"}) + + assert.Equal(t, []testPhase{ + { + Phase: 0, + Dirs: []string{"alpha", "nested/leaf"}, + }, + { + Phase: 1, + Dirs: []string{"beta", "zeta"}, + }, + { + Phase: 2, + Dirs: []string{"gamma"}, + }, + }, phases) + assert.Equal(t, []string{"alpha", "nested/leaf", "beta", "zeta", "gamma"}, flattenTestPhases(phases)) +} + +func TestPhaseSchedulerWaitsForPreviousPhase(t *testing.T) { + scheduler := newPhaseScheduler([]testPhase{ + { + Phase: 0, + Dirs: []string{"a", "b"}, + }, + { + Phase: 1, + Dirs: []string{"c"}, + }, + }) + + released := make(chan struct{}) + go func() { + scheduler.Wait(1) + close(released) + }() + + select { + case <-released: + t.Fatal("phase 1 should stay blocked until phase 0 completes") + case <-time.After(20 * time.Millisecond): + } + + scheduler.Done(0) + + select { + case <-released: + t.Fatal("phase 1 should stay blocked until all phase 0 tests complete") + case <-time.After(20 * time.Millisecond): + } + + scheduler.Done(0) + + select { + case <-released: + case <-time.After(time.Second): + t.Fatal("phase 1 was not released") + } +} From 709f983408da32862295f0634e34d4b87fc8aece Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 17 Mar 2026 21:30:08 +0100 Subject: [PATCH 02/12] Refactor acceptance phase execution Collect runnable acceptance tests up front so phase 0 runs immediately and phase 1 only starts after all phase 0 tests complete. --- acceptance/acceptance_test.go | 247 +++++++++++++++++----------------- acceptance/phase_test.go | 98 +++++++------- 2 files changed, 175 insertions(+), 170 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 13b34c1642..bb2cbb068d 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -122,16 +122,23 @@ var Ignored = map[string]bool{ userReplacementsFilename: true, } -type testPhase struct { - Phase int - Dirs []string +type runnableTest struct { + dir string + config internal.TestConfig + skipReason string + coverDir string + repls testdiff.ReplacementsContext + envFilters []string + runParallel bool + phaseGate <-chan struct{} + phaseDone func() } -type phaseScheduler struct { +type phaseSemaphore struct { mu sync.Mutex - remaining map[int]int - next map[int]int - gates map[int]chan struct{} + remaining int + sealed bool + gate chan struct{} } func TestAccept(t *testing.T) { @@ -300,80 +307,65 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { require.NotEmpty(t, testDirs, "singleTest=%#v did not match any tests\n%#v", singleTest, testDirs) } - testPhases := collectTestPhases(t, testDirs) - testDirs = flattenTestPhases(testPhases) - scheduler := newPhaseScheduler(testPhases) - skippedDirs := 0 totalDirs := 0 selectedDirs := 0 envFilters := getEnvFilters(t) + phase0Semaphore := newPhaseSemaphore() + var phase1Tests []runnableTest for _, dir := range testDirs { totalDirs += 1 - t.Run(dir, func(t *testing.T) { - selectedDirs += 1 - - config, configPath := internal.LoadConfig(t, dir) - defer scheduler.Done(config.Phase) - skipReason := getSkipReason(&config, configPath) - - // Generate materialized config for this test. - // We do this before skipping the test, so the configs are generated for all tests. - materializedConfig, err := internal.GenerateMaterializedConfig(config) - require.NoError(t, err) - testutil.WriteFile(t, filepath.Join(dir, internal.MaterializedConfigFile), materializedConfig) - - // If only regenerating out.test.toml, skip the actual test execution - if OnlyOutTestToml { - t.Skip("Skipping test execution (only regenerating out.test.toml)") - } - - if skipReason != "" { - skippedDirs += 1 - t.Skip(skipReason) - } + config, configPath := internal.LoadConfig(t, dir) + err := validateTestPhase(config.Phase) + if err != nil { + t.Fatalf("Invalid config %s: %s", configPath, err) + } - runParallel := !inprocessMode + // Generate materialized config for this test. + // We do this before skipping the test, so the configs are generated for all tests. + materializedConfig, err := internal.GenerateMaterializedConfig(config) + require.NoError(t, err) + testutil.WriteFile(t, filepath.Join(dir, internal.MaterializedConfigFile), materializedConfig) - if benchmarkMode && strings.Contains(dir, "benchmark") { - runParallel = false - } + runParallel := !inprocessMode + if benchmarkMode && strings.Contains(dir, "benchmark") { + runParallel = false + } + skipReason := getSkipReason(&config, configPath) + + runnable := runnableTest{ + dir: dir, + config: config, + skipReason: skipReason, + coverDir: coverDir, + repls: repls.Clone(), + envFilters: envFilters, + runParallel: runParallel, + } - if runParallel { - t.Parallel() - } + selectedDirs += 1 + if runnable.skipReason != "" { + skippedDirs += 1 + } - scheduler.Wait(config.Phase) + if config.Phase == 0 { + phase0Semaphore.Add() + runnable.phaseDone = phase0Semaphore.Done + t.Run(dir, runnable.run) + continue + } - // Build extra vars for exclusion matching (config state as env vars) - var extraVars []string - if cloudEnv != "" { - extraVars = append(extraVars, "CONFIG_Cloud=true") - } + phase1Tests = append(phase1Tests, runnable) + } - expanded := internal.ExpandEnvMatrix(config.EnvMatrix, config.EnvMatrixExclude, extraVars) + phase1Gate := phase0Semaphore.Seal() - if len(expanded) == 1 { - // env vars aren't part of the test case name, so log them for debugging - if len(expanded[0]) > 0 { - t.Logf("Running test with env %v", expanded[0]) - } - runTest(t, dir, 0, coverDir, repls.Clone(), config, expanded[0], envFilters) - } else { - for ind, envset := range expanded { - envname := strings.Join(envset, "/") - t.Run(envname, func(t *testing.T) { - if runParallel { - t.Parallel() - } - runTest(t, dir, ind, coverDir, repls.Clone(), config, envset, envFilters) - }) - } - } - }) + for _, runnable := range phase1Tests { + runnable.phaseGate = phase1Gate + t.Run(runnable.dir, runnable.run) } t.Logf("Summary (dirs): %d/%d/%d run/selected/total, %d skipped", selectedDirs-skippedDirs, selectedDirs, totalDirs, skippedDirs) @@ -425,90 +417,99 @@ func getTests(t *testing.T) []string { return testDirs } -func collectTestPhases(t *testing.T, testDirs []string) []testPhase { - phaseDirs := make(map[int][]string) - - for _, dir := range testDirs { - config, _ := internal.LoadConfig(t, dir) - phaseDirs[config.Phase] = append(phaseDirs[config.Phase], dir) +func newPhaseSemaphore() *phaseSemaphore { + return &phaseSemaphore{ + gate: make(chan struct{}), } +} - phases := make([]int, 0, len(phaseDirs)) - for phase := range phaseDirs { - phases = append(phases, phase) - } - sort.Ints(phases) +func (s *phaseSemaphore) Add() { + s.mu.Lock() + defer s.mu.Unlock() - result := make([]testPhase, 0, len(phases)) - for _, phase := range phases { - dirs := phaseDirs[phase] - sort.Strings(dirs) - result = append(result, testPhase{ - Phase: phase, - Dirs: dirs, - }) + s.remaining++ +} + +func (s *phaseSemaphore) Done() { + s.mu.Lock() + defer s.mu.Unlock() + + s.remaining-- + if !s.sealed || s.remaining != 0 { + return } - return result + close(s.gate) } -func flattenTestPhases(testPhases []testPhase) []string { - totalDirs := 0 - for _, phase := range testPhases { - totalDirs += len(phase.Dirs) - } +func (s *phaseSemaphore) Seal() <-chan struct{} { + s.mu.Lock() + defer s.mu.Unlock() - result := make([]string, 0, totalDirs) - for _, phase := range testPhases { - result = append(result, phase.Dirs...) + s.sealed = true + if s.remaining == 0 { + close(s.gate) } - return result + return s.gate } -func newPhaseScheduler(testPhases []testPhase) *phaseScheduler { - remaining := make(map[int]int, len(testPhases)) - next := make(map[int]int, len(testPhases)) - gates := make(map[int]chan struct{}, len(testPhases)) +func validateTestPhase(phase int) error { + if phase == 0 || phase == 1 { + return nil + } + + return fmt.Errorf("Phase must be 0 or 1, got %d", phase) +} - for i, phase := range testPhases { - remaining[phase.Phase] = len(phase.Dirs) - gates[phase.Phase] = make(chan struct{}) - if i+1 < len(testPhases) { - next[phase.Phase] = testPhases[i+1].Phase - } +func (r runnableTest) run(t *testing.T) { + if r.phaseDone != nil { + defer r.phaseDone() } - if len(testPhases) > 0 { - close(gates[testPhases[0].Phase]) + // If only regenerating out.test.toml, skip the actual test execution + if OnlyOutTestToml { + t.Skip("Skipping test execution (only regenerating out.test.toml)") } - return &phaseScheduler{ - remaining: remaining, - next: next, - gates: gates, + if r.skipReason != "" { + t.Skip(r.skipReason) } -} -func (s *phaseScheduler) Wait(phase int) { - <-s.gates[phase] -} + if r.runParallel { + t.Parallel() + } -func (s *phaseScheduler) Done(phase int) { - s.mu.Lock() - defer s.mu.Unlock() + if r.phaseGate != nil { + <-r.phaseGate + } - s.remaining[phase]-- - if s.remaining[phase] != 0 { - return + // Build extra vars for exclusion matching (config state as env vars) + var extraVars []string + if os.Getenv("CLOUD_ENV") != "" { + extraVars = append(extraVars, "CONFIG_Cloud=true") } - next, ok := s.next[phase] - if !ok { + expanded := internal.ExpandEnvMatrix(r.config.EnvMatrix, r.config.EnvMatrixExclude, extraVars) + + if len(expanded) == 1 { + // env vars aren't part of the test case name, so log them for debugging + if len(expanded[0]) > 0 { + t.Logf("Running test with env %v", expanded[0]) + } + runTest(t, r.dir, 0, r.coverDir, r.repls.Clone(), r.config, expanded[0], r.envFilters) return } - close(s.gates[next]) + for ind, envset := range expanded { + envname := strings.Join(envset, "/") + t.Run(envname, func(t *testing.T) { + if r.runParallel { + t.Parallel() + } + runTest(t, r.dir, ind, r.coverDir, r.repls.Clone(), r.config, envset, r.envFilters) + }) + } } // Return a reason to skip the test. Empty string means "don't skip". diff --git a/acceptance/phase_test.go b/acceptance/phase_test.go index 48aadc8c01..e6faab46fb 100644 --- a/acceptance/phase_test.go +++ b/acceptance/phase_test.go @@ -1,8 +1,6 @@ package acceptance_test import ( - "os" - "path/filepath" "testing" "time" @@ -10,60 +8,54 @@ import ( "github.com/stretchr/testify/require" ) -func TestCollectTestPhases(t *testing.T) { - root := t.TempDir() - t.Chdir(root) - - files := map[string]string{ - "test.toml": "Phase = 9\n", - "alpha/test.toml": "Local = true\n", - "beta/test.toml": "Phase = 1\n", - "gamma/test.toml": "Phase = 2\n", - "zeta/test.toml": "Local = true\nPhase = 1\n", - "nested/test.toml": "Phase = 4\n", - } - - for path, contents := range files { - absPath := filepath.Join(root, filepath.FromSlash(path)) - require.NoError(t, os.MkdirAll(filepath.Dir(absPath), 0o755)) - require.NoError(t, os.WriteFile(absPath, []byte(contents), 0o644)) - } - require.NoError(t, os.MkdirAll(filepath.Join(root, "nested", "leaf"), 0o755)) - - phases := collectTestPhases(t, []string{"gamma", "beta", "alpha", "zeta", "nested/leaf"}) - - assert.Equal(t, []testPhase{ +func TestValidateTestPhase(t *testing.T) { + tests := []struct { + name string + phase int + wantErr string + }{ { - Phase: 0, - Dirs: []string{"alpha", "nested/leaf"}, + name: "phase zero", + phase: 0, }, { - Phase: 1, - Dirs: []string{"beta", "zeta"}, + name: "phase one", + phase: 1, }, { - Phase: 2, - Dirs: []string{"gamma"}, + name: "negative phase", + phase: -1, + wantErr: "Phase must be 0 or 1, got -1", }, - }, phases) - assert.Equal(t, []string{"alpha", "nested/leaf", "beta", "zeta", "gamma"}, flattenTestPhases(phases)) -} - -func TestPhaseSchedulerWaitsForPreviousPhase(t *testing.T) { - scheduler := newPhaseScheduler([]testPhase{ { - Phase: 0, - Dirs: []string{"a", "b"}, + name: "phase two", + phase: 2, + wantErr: "Phase must be 0 or 1, got 2", }, - { - Phase: 1, - Dirs: []string{"c"}, - }, - }) + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateTestPhase(tt.phase) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + + require.EqualError(t, err, tt.wantErr) + }) + } +} + +func TestPhaseSemaphoreWaitsForAllPhaseZeroTests(t *testing.T) { + semaphore := newPhaseSemaphore() + semaphore.Add() + semaphore.Add() + gate := semaphore.Seal() released := make(chan struct{}) go func() { - scheduler.Wait(1) + <-gate close(released) }() @@ -73,7 +65,7 @@ func TestPhaseSchedulerWaitsForPreviousPhase(t *testing.T) { case <-time.After(20 * time.Millisecond): } - scheduler.Done(0) + semaphore.Done() select { case <-released: @@ -81,7 +73,7 @@ func TestPhaseSchedulerWaitsForPreviousPhase(t *testing.T) { case <-time.After(20 * time.Millisecond): } - scheduler.Done(0) + semaphore.Done() select { case <-released: @@ -89,3 +81,15 @@ func TestPhaseSchedulerWaitsForPreviousPhase(t *testing.T) { t.Fatal("phase 1 was not released") } } + +func TestPhaseSemaphoreWithoutPhaseZeroTestsIsOpen(t *testing.T) { + gate := newPhaseSemaphore().Seal() + + select { + case <-gate: + default: + t.Fatal("phase 1 should be released when there are no phase 0 tests") + } + + assert.NotNil(t, gate) +} From 46540cc7dbf289e8222983f88c73bdd048c1f59e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 18 Mar 2026 14:14:49 +0100 Subject: [PATCH 03/12] fixes --- acceptance/acceptance_test.go | 13 ++--- acceptance/phase_test.go | 95 ----------------------------------- 2 files changed, 5 insertions(+), 103 deletions(-) delete mode 100644 acceptance/phase_test.go diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index bb2cbb068d..2a77c1be78 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -131,7 +131,6 @@ type runnableTest struct { envFilters []string runParallel bool phaseGate <-chan struct{} - phaseDone func() } type phaseSemaphore struct { @@ -352,9 +351,11 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { } if config.Phase == 0 { - phase0Semaphore.Add() - runnable.phaseDone = phase0Semaphore.Done - t.Run(dir, runnable.run) + t.Run(dir, func(t *testing.T) { + phase0Semaphore.Add() + defer phase0Semaphore.Done() + runnable.run(t) + }) continue } @@ -463,10 +464,6 @@ func validateTestPhase(phase int) error { } func (r runnableTest) run(t *testing.T) { - if r.phaseDone != nil { - defer r.phaseDone() - } - // If only regenerating out.test.toml, skip the actual test execution if OnlyOutTestToml { t.Skip("Skipping test execution (only regenerating out.test.toml)") diff --git a/acceptance/phase_test.go b/acceptance/phase_test.go deleted file mode 100644 index e6faab46fb..0000000000 --- a/acceptance/phase_test.go +++ /dev/null @@ -1,95 +0,0 @@ -package acceptance_test - -import ( - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestValidateTestPhase(t *testing.T) { - tests := []struct { - name string - phase int - wantErr string - }{ - { - name: "phase zero", - phase: 0, - }, - { - name: "phase one", - phase: 1, - }, - { - name: "negative phase", - phase: -1, - wantErr: "Phase must be 0 or 1, got -1", - }, - { - name: "phase two", - phase: 2, - wantErr: "Phase must be 0 or 1, got 2", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := validateTestPhase(tt.phase) - if tt.wantErr == "" { - require.NoError(t, err) - return - } - - require.EqualError(t, err, tt.wantErr) - }) - } -} - -func TestPhaseSemaphoreWaitsForAllPhaseZeroTests(t *testing.T) { - semaphore := newPhaseSemaphore() - semaphore.Add() - semaphore.Add() - gate := semaphore.Seal() - - released := make(chan struct{}) - go func() { - <-gate - close(released) - }() - - select { - case <-released: - t.Fatal("phase 1 should stay blocked until phase 0 completes") - case <-time.After(20 * time.Millisecond): - } - - semaphore.Done() - - select { - case <-released: - t.Fatal("phase 1 should stay blocked until all phase 0 tests complete") - case <-time.After(20 * time.Millisecond): - } - - semaphore.Done() - - select { - case <-released: - case <-time.After(time.Second): - t.Fatal("phase 1 was not released") - } -} - -func TestPhaseSemaphoreWithoutPhaseZeroTestsIsOpen(t *testing.T) { - gate := newPhaseSemaphore().Seal() - - select { - case <-gate: - default: - t.Fatal("phase 1 should be released when there are no phase 0 tests") - } - - assert.NotNil(t, gate) -} From de5a12b9cab384e9ae4eb6208687f5d35ee8dc79 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 18 Mar 2026 15:20:15 +0100 Subject: [PATCH 04/12] Simplify phase execution: use WaitGroup+channel instead of custom semaphore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove runnableTest struct and phaseSemaphore in favor of sync.WaitGroup and a plain channel (phase1Gate) - Keep all test logic inside t.Run callbacks (original structure) - Phase=0 tests call phase0wg.Add(1) before t.Parallel() and t.Cleanup(phase0wg.Done) after all subtests complete; phase=1 tests block on <-phase1Gate - Use t.Cleanup (not defer) so the gate opens only after all EnvMatrix subtests finish writing their output files - Move CloudSlow→Cloud mutation out of getSkipReason to call site, before GenerateMaterializedConfig, so out.test.toml captures the implied value Co-Authored-By: Claude Sonnet 4.6 --- acceptance/acceptance_test.go | 233 ++++++++++++---------------------- 1 file changed, 82 insertions(+), 151 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 2a77c1be78..d5ee8cc9de 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -122,24 +122,6 @@ var Ignored = map[string]bool{ userReplacementsFilename: true, } -type runnableTest struct { - dir string - config internal.TestConfig - skipReason string - coverDir string - repls testdiff.ReplacementsContext - envFilters []string - runParallel bool - phaseGate <-chan struct{} -} - -type phaseSemaphore struct { - mu sync.Mutex - remaining int - sealed bool - gate chan struct{} -} - func TestAccept(t *testing.T) { testAccept(t, InprocessMode, "") } @@ -311,64 +293,101 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { selectedDirs := 0 envFilters := getEnvFilters(t) - phase0Semaphore := newPhaseSemaphore() - var phase1Tests []runnableTest + + var phase0wg sync.WaitGroup + phase1Gate := make(chan struct{}) for _, dir := range testDirs { totalDirs += 1 - config, configPath := internal.LoadConfig(t, dir) - err := validateTestPhase(config.Phase) - if err != nil { - t.Fatalf("Invalid config %s: %s", configPath, err) - } + t.Run(dir, func(t *testing.T) { + selectedDirs += 1 - // Generate materialized config for this test. - // We do this before skipping the test, so the configs are generated for all tests. - materializedConfig, err := internal.GenerateMaterializedConfig(config) - require.NoError(t, err) - testutil.WriteFile(t, filepath.Join(dir, internal.MaterializedConfigFile), materializedConfig) + config, configPath := internal.LoadConfig(t, dir) + err := validateTestPhase(config.Phase) + if err != nil { + t.Fatalf("Invalid config %s: %s", configPath, err) + } - runParallel := !inprocessMode - if benchmarkMode && strings.Contains(dir, "benchmark") { - runParallel = false - } - skipReason := getSkipReason(&config, configPath) - - runnable := runnableTest{ - dir: dir, - config: config, - skipReason: skipReason, - coverDir: coverDir, - repls: repls.Clone(), - envFilters: envFilters, - runParallel: runParallel, - } + // Apply default: CloudSlow implies Cloud. Do this before generating + // the materialized config so the implication is visible in out.test.toml. + if isTruePtr(config.CloudSlow) { + config.Cloud = config.CloudSlow + } - selectedDirs += 1 - if runnable.skipReason != "" { - skippedDirs += 1 - } + // Generate materialized config for this test. + // We do this before skipping the test, so the configs are generated for all tests. + materializedConfig, err := internal.GenerateMaterializedConfig(config) + require.NoError(t, err) + testutil.WriteFile(t, filepath.Join(dir, internal.MaterializedConfigFile), materializedConfig) - if config.Phase == 0 { - t.Run(dir, func(t *testing.T) { - phase0Semaphore.Add() - defer phase0Semaphore.Done() - runnable.run(t) - }) - continue - } + // If only regenerating out.test.toml, skip the actual test execution + if OnlyOutTestToml { + t.Skip("Skipping test execution (only regenerating out.test.toml)") + } - phase1Tests = append(phase1Tests, runnable) - } + skipReason := getSkipReason(&config, configPath) + if skipReason != "" { + skippedDirs += 1 + t.Skip(skipReason) + } + + runParallel := !inprocessMode + if benchmarkMode && strings.Contains(dir, "benchmark") { + runParallel = false + } + + // t.Run blocks until t.Parallel() is called, so Add must happen before t.Parallel(). + // This ensures all phase0 adds are visible before the wait goroutine starts. + if config.Phase == 0 { + phase0wg.Add(1) + t.Cleanup(phase0wg.Done) + } + + if runParallel { + t.Parallel() + } + + if config.Phase != 0 { + t.Logf("Waiting for Phase=%d to start", config.Phase) + <-phase1Gate + t.Logf("Continue with Phase=%d", config.Phase) + } + + // Build extra vars for exclusion matching (config state as env vars) + var extraVars []string + if cloudEnv != "" { + extraVars = append(extraVars, "CONFIG_Cloud=true") + } - phase1Gate := phase0Semaphore.Seal() + expanded := internal.ExpandEnvMatrix(config.EnvMatrix, config.EnvMatrixExclude, extraVars) - for _, runnable := range phase1Tests { - runnable.phaseGate = phase1Gate - t.Run(runnable.dir, runnable.run) + if len(expanded) == 1 { + // env vars aren't part of the test case name, so log them for debugging + if len(expanded[0]) > 0 { + t.Logf("Running test with env %v", expanded[0]) + } + runTest(t, dir, 0, coverDir, repls.Clone(), config, expanded[0], envFilters) + return + } + + for ind, envset := range expanded { + envname := strings.Join(envset, "/") + t.Run(envname, func(t *testing.T) { + if runParallel { + t.Parallel() + } + runTest(t, dir, ind, coverDir, repls.Clone(), config, envset, envFilters) + }) + } + }) } + go func() { + phase0wg.Wait() + close(phase1Gate) + }() + t.Logf("Summary (dirs): %d/%d/%d run/selected/total, %d skipped", selectedDirs-skippedDirs, selectedDirs, totalDirs, skippedDirs) return selectedDirs - skippedDirs @@ -418,43 +437,6 @@ func getTests(t *testing.T) []string { return testDirs } -func newPhaseSemaphore() *phaseSemaphore { - return &phaseSemaphore{ - gate: make(chan struct{}), - } -} - -func (s *phaseSemaphore) Add() { - s.mu.Lock() - defer s.mu.Unlock() - - s.remaining++ -} - -func (s *phaseSemaphore) Done() { - s.mu.Lock() - defer s.mu.Unlock() - - s.remaining-- - if !s.sealed || s.remaining != 0 { - return - } - - close(s.gate) -} - -func (s *phaseSemaphore) Seal() <-chan struct{} { - s.mu.Lock() - defer s.mu.Unlock() - - s.sealed = true - if s.remaining == 0 { - close(s.gate) - } - - return s.gate -} - func validateTestPhase(phase int) error { if phase == 0 || phase == 1 { return nil @@ -463,59 +445,8 @@ func validateTestPhase(phase int) error { return fmt.Errorf("Phase must be 0 or 1, got %d", phase) } -func (r runnableTest) run(t *testing.T) { - // If only regenerating out.test.toml, skip the actual test execution - if OnlyOutTestToml { - t.Skip("Skipping test execution (only regenerating out.test.toml)") - } - - if r.skipReason != "" { - t.Skip(r.skipReason) - } - - if r.runParallel { - t.Parallel() - } - - if r.phaseGate != nil { - <-r.phaseGate - } - - // Build extra vars for exclusion matching (config state as env vars) - var extraVars []string - if os.Getenv("CLOUD_ENV") != "" { - extraVars = append(extraVars, "CONFIG_Cloud=true") - } - - expanded := internal.ExpandEnvMatrix(r.config.EnvMatrix, r.config.EnvMatrixExclude, extraVars) - - if len(expanded) == 1 { - // env vars aren't part of the test case name, so log them for debugging - if len(expanded[0]) > 0 { - t.Logf("Running test with env %v", expanded[0]) - } - runTest(t, r.dir, 0, r.coverDir, r.repls.Clone(), r.config, expanded[0], r.envFilters) - return - } - - for ind, envset := range expanded { - envname := strings.Join(envset, "/") - t.Run(envname, func(t *testing.T) { - if r.runParallel { - t.Parallel() - } - runTest(t, r.dir, ind, r.coverDir, r.repls.Clone(), r.config, envset, r.envFilters) - }) - } -} - // Return a reason to skip the test. Empty string means "don't skip". func getSkipReason(config *internal.TestConfig, configPath string) string { - // Apply default first, so that it's visible in out.test.toml - if isTruePtr(config.CloudSlow) { - config.Cloud = config.CloudSlow - } - if os.Getenv("DATABRICKS_TEST_SKIPLOCAL") != "" && isTruePtr(config.Local) { return "Disabled via DATABRICKS_TEST_SKIPLOCAL environment variable in " + configPath } From a50af7874088b41c5f7f708990f7a583b25214c9 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 18 Mar 2026 15:26:20 +0100 Subject: [PATCH 05/12] simplify --- acceptance/acceptance_test.go | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index d5ee8cc9de..9c74d3dd1b 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -344,12 +344,9 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { t.Cleanup(phase0wg.Done) } - if runParallel { - t.Parallel() - } - if config.Phase != 0 { t.Logf("Waiting for Phase=%d to start", config.Phase) + t.Parallel() <-phase1Gate t.Logf("Continue with Phase=%d", config.Phase) } @@ -362,15 +359,6 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { expanded := internal.ExpandEnvMatrix(config.EnvMatrix, config.EnvMatrixExclude, extraVars) - if len(expanded) == 1 { - // env vars aren't part of the test case name, so log them for debugging - if len(expanded[0]) > 0 { - t.Logf("Running test with env %v", expanded[0]) - } - runTest(t, dir, 0, coverDir, repls.Clone(), config, expanded[0], envFilters) - return - } - for ind, envset := range expanded { envname := strings.Join(envset, "/") t.Run(envname, func(t *testing.T) { From 97972d43031932422239954000755a9eb6ba7cb7 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 18 Mar 2026 17:21:34 +0100 Subject: [PATCH 06/12] update --- Makefile | 2 -- acceptance/acceptance_test.go | 7 ++++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index e138b08bce..5714483a36 100644 --- a/Makefile +++ b/Makefile @@ -98,8 +98,6 @@ test-slow-acc: .PHONY: test-update test-update: -go test ./acceptance -run '^TestAccept$$' -update -timeout=${LOCAL_TIMEOUT} - @# at the moment second pass is required because some tests show diff against output of another test for easier review - -go test ./acceptance -run '^TestAccept$$' -update -timeout=${LOCAL_TIMEOUT} # Updates acceptance test output for template tests only .PHONY: test-update-templates diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 9c74d3dd1b..b3a2326989 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -344,11 +344,12 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { t.Cleanup(phase0wg.Done) } - if config.Phase != 0 { - t.Logf("Waiting for Phase=%d to start", config.Phase) + if runParallel { t.Parallel() + } + + if config.Phase != 0 { <-phase1Gate - t.Logf("Continue with Phase=%d", config.Phase) } // Build extra vars for exclusion matching (config state as env vars) From b1dce52e04f80d243c0b564dae5813a3d2ed309f Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Mar 2026 15:47:11 +0100 Subject: [PATCH 07/12] update test.toml --- acceptance/bundle/user_agent/out.test.toml | 1 + acceptance/bundle/user_agent/test.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/acceptance/bundle/user_agent/out.test.toml b/acceptance/bundle/user_agent/out.test.toml index d560f1de04..4cfe03e9f9 100644 --- a/acceptance/bundle/user_agent/out.test.toml +++ b/acceptance/bundle/user_agent/out.test.toml @@ -1,5 +1,6 @@ Local = true Cloud = false +Phase = 1 [EnvMatrix] DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/user_agent/test.toml b/acceptance/bundle/user_agent/test.toml index 9295fedc55..a9f876b6b7 100644 --- a/acceptance/bundle/user_agent/test.toml +++ b/acceptance/bundle/user_agent/test.toml @@ -1,3 +1,4 @@ +Phase = 1 RecordRequests = true Local = true IncludeRequestHeaders = ["User-Agent"] From 6a73f0bc32142cd1517bf306eaa94a1b1ca52574 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Mar 2026 16:19:35 +0100 Subject: [PATCH 08/12] inherit tag --- acceptance/internal/config.go | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/acceptance/internal/config.go b/acceptance/internal/config.go index 783c340fb9..5c2f980f70 100644 --- a/acceptance/internal/config.go +++ b/acceptance/internal/config.go @@ -26,7 +26,7 @@ type TestConfig struct { // Execution phase for this test. Tests run in ascending phase order. // Phase is not inherited from parent test.toml files. Default is 0. - Phase int + Phase int `inherit:"false"` // Which OSes the test is enabled on. Each string is compared against runtime.GOOS. // If absent, default to true. @@ -224,11 +224,7 @@ func LoadConfig(t *testing.T, dir string) (TestConfig, string) { } } - if hasLeafConfig { - result.Phase = leafConfig.Phase - } else { - result.Phase = 0 - } + restoreNonInheritable(&result, leafConfig, hasLeafConfig) // Always ignore .cache directory (used by local cache) result.Ignore = append(result.Ignore, ".cache") @@ -276,6 +272,24 @@ func DoLoadConfig(t *testing.T, path string) TestConfig { return config } +// restoreNonInheritable resets fields tagged with `inherit:"false"` to their leaf config values. +// If there is no leaf config, those fields are reset to their zero value. +func restoreNonInheritable(result *TestConfig, leafConfig TestConfig, hasLeafConfig bool) { + typ := reflect.TypeFor[TestConfig]() + val := reflect.ValueOf(result).Elem() + leafVal := reflect.ValueOf(leafConfig) + for i := range typ.NumField() { + field := typ.Field(i) + if field.Tag.Get("inherit") == "false" { + if hasLeafConfig { + val.Field(i).Set(leafVal.Field(i)) + } else { + val.Field(i).SetZero() + } + } + } +} + // mapTransformer is a mergo transformer that merges two maps // by overriding values in the destination map with values from the source map. // From afc047a07d430c0e148d8ff973a59ecf56b849e1 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Mar 2026 16:26:23 +0100 Subject: [PATCH 09/12] clean up materialized_config_test.go --- .../internal/materialized_config_test.go | 43 ------------------- 1 file changed, 43 deletions(-) delete mode 100644 acceptance/internal/materialized_config_test.go diff --git a/acceptance/internal/materialized_config_test.go b/acceptance/internal/materialized_config_test.go deleted file mode 100644 index eb35eb4710..0000000000 --- a/acceptance/internal/materialized_config_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package internal - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestGenerateMaterializedConfigIncludesPhase(t *testing.T) { - local := true - cloud := false - - config := TestConfig{ - Local: localPtr(local), - Cloud: localPtr(cloud), - Phase: 1, - } - - out, err := GenerateMaterializedConfig(config) - require.NoError(t, err) - - assert.Equal(t, "Local = true\nCloud = false\nPhase = 1\n", out) -} - -func TestGenerateMaterializedConfigOmitsDefaultPhase(t *testing.T) { - local := true - cloud := false - - config := TestConfig{ - Local: localPtr(local), - Cloud: localPtr(cloud), - } - - out, err := GenerateMaterializedConfig(config) - require.NoError(t, err) - - assert.Equal(t, "Local = true\nCloud = false\n", out) -} - -func localPtr[T any](v T) *T { - return &v -} From 498fc69d83985ee45ce22946be291b0549e09cba Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Mar 2026 16:39:06 +0100 Subject: [PATCH 10/12] Use filepath.ToSlash for config path description to ensure forward slashes on Windows Co-Authored-By: Claude Opus 4.6 --- acceptance/internal/config.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/acceptance/internal/config.go b/acceptance/internal/config.go index 5c2f980f70..2f57cf270d 100644 --- a/acceptance/internal/config.go +++ b/acceptance/internal/config.go @@ -231,9 +231,10 @@ func LoadConfig(t *testing.T, dir string) (TestConfig, string) { result.CompiledIgnoreObject = ignore.CompileIgnoreLines(result.Ignore...) // Validate incompatible configuration combinations - validateConfig(t, result, strings.Join(configs, ", ")) + configDesc := filepath.ToSlash(strings.Join(configs, ", ")) + validateConfig(t, result, configDesc) - return result, strings.Join(configs, ", ") + return result, configDesc } // validateConfig checks for incompatible configuration combinations. From 3f9f164b6154a4dbfb95d4b2263e58e425367e2b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 19 Mar 2026 17:36:15 +0100 Subject: [PATCH 11/12] update the comment --- acceptance/internal/config.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/acceptance/internal/config.go b/acceptance/internal/config.go index 2f57cf270d..23981e7093 100644 --- a/acceptance/internal/config.go +++ b/acceptance/internal/config.go @@ -24,8 +24,10 @@ type TestConfig struct { // Place to describe what's wrong with this test. Does not affect how the test is run. Badness *string - // Execution phase for this test. Tests run in ascending phase order. - // Phase is not inherited from parent test.toml files. Default is 0. + // Execution phase for this test. Phase 1 tests run after all phase 0 tests complete, + // which is useful when a test depends on the output of another tests. + // Some tests run diff against sister test output to highlight the differences. + // Some tests summarize output of many child tests. Phase int `inherit:"false"` // Which OSes the test is enabled on. Each string is compared against runtime.GOOS. From a60dcfbf8091a2a7704b32446740f44edaad04d0 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 20 Mar 2026 12:45:13 +0100 Subject: [PATCH 12/12] Only enable phase ordering in update mode Phase 1 tests read output files from phase 0 tests via $TESTDIR, which points to the source directory. In normal runs those files are already committed and stable, so the phase gate just adds unnecessary serialization. Only activate the WaitGroup/channel gate when -update is set. Co-authored-by: Isaac --- acceptance/acceptance_test.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index b3a2326989..e293388dc6 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -294,8 +294,15 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { envFilters := getEnvFilters(t) + // Phases are only needed in update mode, where phase 0 tests regenerate + // output files that phase 1 tests read via $TESTDIR. In normal runs, + // those files are already committed and stable. + usePhases := testdiff.OverwriteMode var phase0wg sync.WaitGroup phase1Gate := make(chan struct{}) + if !usePhases { + close(phase1Gate) + } for _, dir := range testDirs { totalDirs += 1 @@ -339,7 +346,7 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { // t.Run blocks until t.Parallel() is called, so Add must happen before t.Parallel(). // This ensures all phase0 adds are visible before the wait goroutine starts. - if config.Phase == 0 { + if usePhases && config.Phase == 0 { phase0wg.Add(1) t.Cleanup(phase0wg.Done) } @@ -372,10 +379,12 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { }) } - go func() { - phase0wg.Wait() - close(phase1Gate) - }() + if usePhases { + go func() { + phase0wg.Wait() + close(phase1Gate) + }() + } t.Logf("Summary (dirs): %d/%d/%d run/selected/total, %d skipped", selectedDirs-skippedDirs, selectedDirs, totalDirs, skippedDirs)