From 82422e0d8ae4f9b1d89995f6307097dbe551b563 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 9 Jul 2026 15:28:33 +0200 Subject: [PATCH 1/6] acc: extract changed-test detection into internal.ChangedTests Move the "which acceptance test dirs did this branch's diff affect?" logic out of the skiplocal_test.go helper and into acceptance/internal, so both the existing DATABRICKS_TEST_SKIPLOCAL=withchanged path and future callers can share one detection function. - internal.ChangedTests(baseRef, headRef, testDirs) runs the git diff and returns a map of test dir -> ChangedTest{Added, VariantFilters}. An empty headRef diffs against the working tree (preserving withchanged's local-dev behavior of re-enabling uncommitted edits); a non-empty headRef diffs between two refs, independent of the checkout, for CI and replaying historical PRs. - internal.ParseChangedTests is split out as a pure function over git diff text, unit-tested with literal diff strings (added dir, rename, nested dir, invariant config INPUT_CONFIG= expansion, files outside acceptance/). - selectChangedLocalTests keeps only the skiplocal policy (added-first ordering and the maxChangedLocalTests cap) and delegates detection to ChangedTests. No behavior change: the withchanged path produces the same selection as before. Co-authored-by: Isaac --- acceptance/internal/changed_tests.go | 129 ++++++++++++++++++++++ acceptance/internal/changed_tests_test.go | 116 +++++++++++++++++++ acceptance/skiplocal_test.go | 102 +++-------------- 3 files changed, 261 insertions(+), 86 deletions(-) create mode 100644 acceptance/internal/changed_tests.go create mode 100644 acceptance/internal/changed_tests_test.go diff --git a/acceptance/internal/changed_tests.go b/acceptance/internal/changed_tests.go new file mode 100644 index 00000000000..af5dcf35608 --- /dev/null +++ b/acceptance/internal/changed_tests.go @@ -0,0 +1,129 @@ +package internal + +import ( + "fmt" + "os/exec" + "path/filepath" + "strings" +) + +const ( + invariantConfigsPrefix = "acceptance/bundle/invariant/configs/" + invariantDirPrefix = "bundle/invariant/" +) + +// ChangedTest describes how one acceptance test dir was affected by a branch diff. +type ChangedTest struct { + // Added is true when the dir is brand new (its "script" file was added on the branch). + Added bool + + // VariantFilters restricts which env-matrix variants of the dir are relevant. + // nil means all variants; a non-nil slice (e.g. "INPUT_CONFIG=job.yml.tmpl") + // restricts to matching variants. Only invariant-config changes set this. + VariantFilters []string +} + +// ChangedTests returns the acceptance test dirs affected by the diff against the +// merge base with baseRef, mapped to how each was affected. testDirs is the set +// of known test dirs (relative to acceptance/). +// +// When headRef is empty, the diff is against the working tree: this covers +// committed, staged, and unstaged changes alike, which enables the local dev +// workflow of "touch a config, run the test, see it re-enabled" without +// committing (the same reason lintdiff.py uses --merge-base). Untracked files +// (not yet git-added) are not visible to git diff and will not be re-enabled +// until staged or committed. +// +// When headRef is non-empty, the diff is between the two refs (baseRef and +// headRef), independent of what is checked out. This is what CI and tests that +// replay a historical PR want: only committed changes on headRef, no dependence +// on the working tree. The three-dot form baseRef...headRef would only cover +// committed changes too, but --merge-base keeps the semantics identical to the +// working-tree case. +func ChangedTests(baseRef, headRef string, testDirs map[string]bool) (map[string]ChangedTest, error) { + args := []string{"diff", "--name-status", "--merge-base", "-M", baseRef} + if headRef != "" { + args = append(args, headRef) + } + out, err := exec.Command("git", args...).Output() + if err != nil { + return nil, fmt.Errorf("git diff against %s failed: %w", baseRef, err) + } + return ParseChangedTests(string(out), testDirs), nil +} + +// ParseChangedTests turns the output of `git diff --name-status -M` into the set +// of affected acceptance test dirs. It is split from ChangedTests so the mapping +// logic can be unit-tested with literal diff strings, without a git repo. +// +// Each diff line is "\t" (or "\t\t" for renames); +// the last field is the current path. A changed invariant config +// (acceptance/bundle/invariant/configs/*.yml.tmpl) maps to all invariant subdirs +// with an INPUT_CONFIG= filter, so touching job.yml.tmpl re-enables all subdirs +// but only for their job.yml.tmpl variants. A direct change to a subdir takes +// precedence and re-enables all of its variants. +func ParseChangedTests(diffOutput string, testDirs map[string]bool) map[string]ChangedTest { + result := map[string]ChangedTest{} + + for line := range strings.SplitSeq(strings.TrimSpace(diffOutput), "\n") { + fields := strings.Split(line, "\t") + if len(fields) < 2 { + continue + } + status := fields[0] + path := fields[len(fields)-1] + + // A changed invariant config re-enables all invariant subdirs with an + // INPUT_CONFIG filter, unless a subdir was already unlocked by a non-config change. + if strings.HasPrefix(path, invariantConfigsPrefix) { + configName := path[len(invariantConfigsPrefix):] + // Strip -init.sh / -cleanup.sh suffixes to get the base config name. + if i := strings.Index(configName, "-"); i > 0 && strings.HasSuffix(configName, ".sh") { + configName = configName[:i] + } + if strings.HasSuffix(configName, ".yml.tmpl") { + for dir := range testDirs { + if strings.HasPrefix(dir, invariantDirPrefix) { + existing, ok := result[dir] + if !ok || existing.VariantFilters != nil { + existing.VariantFilters = append(existing.VariantFilters, "INPUT_CONFIG="+configName) + result[dir] = existing + } + } + } + } + continue + } + + dir := testDirForFile(path, testDirs) + if dir == "" { + continue + } + // nil VariantFilters = all variants; overrides any prior config-scoped filter. + // A script file with status A means the test dir is brand new. Renames (R) + // land here as the destination path but are not "added". + result[dir] = ChangedTest{ + Added: status == "A" && strings.HasSuffix(path, "/script"), + } + } + + return result +} + +// testDirForFile maps a repo-relative changed file (e.g. acceptance/bundle/foo/script) +// to its owning test dir relative to acceptance/ (e.g. bundle/foo), or "" if the file +// is outside acceptance/ or not under any known test dir. +func testDirForFile(repoRelPath string, testDirs map[string]bool) string { + parts := strings.Split(filepath.ToSlash(repoRelPath), "/") + if len(parts) < 2 || parts[0] != "acceptance" { + return "" + } + // Longest ancestor first so nested tests map to the innermost test dir. + for depth := len(parts); depth > 1; depth-- { + candidate := strings.Join(parts[1:depth], "/") + if testDirs[candidate] { + return candidate + } + } + return "" +} diff --git a/acceptance/internal/changed_tests_test.go b/acceptance/internal/changed_tests_test.go new file mode 100644 index 00000000000..3e2d3a5af51 --- /dev/null +++ b/acceptance/internal/changed_tests_test.go @@ -0,0 +1,116 @@ +package internal + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// testDirsFixture is the set of known acceptance test dirs used across the +// parser tests. Values mirror real acceptance/ layout: some plain bundle tests +// and several bundle/invariant subdirs that share the configs/ directory. +func testDirsFixture() map[string]bool { + return map[string]bool{ + "bundle/resources": true, + "bundle/resources/jobs": true, + "bundle/invariant/no_drift": true, + "bundle/invariant/migrate": true, + "bundle/invariant/continue_293": true, + "cmd/version": true, + } +} + +func TestParseChangedTests(t *testing.T) { + tests := []struct { + name string + diff string + want map[string]ChangedTest + }{ + { + name: "empty diff", + diff: "", + want: map[string]ChangedTest{}, + }, + { + name: "modified file maps to owning test dir", + diff: "M\tacceptance/bundle/resources/script", + want: map[string]ChangedTest{ + "bundle/resources": {Added: false, VariantFilters: nil}, + }, + }, + { + name: "added script marks the dir as added", + diff: "A\tacceptance/bundle/resources/jobs/script", + want: map[string]ChangedTest{ + "bundle/resources/jobs": {Added: true, VariantFilters: nil}, + }, + }, + { + name: "nested file maps to innermost test dir", + diff: "M\tacceptance/bundle/resources/jobs/output.txt", + want: map[string]ChangedTest{ + "bundle/resources/jobs": {Added: false, VariantFilters: nil}, + }, + }, + { + name: "file outside acceptance is ignored", + diff: "M\tlibs/dyn/value.go", + want: map[string]ChangedTest{}, + }, + { + name: "file under acceptance but not in any test dir is ignored", + diff: "M\tacceptance/README.md", + want: map[string]ChangedTest{}, + }, + { + name: "rename destination is used and is not treated as added", + diff: "R092\tacceptance/bundle/resources/old\tacceptance/bundle/resources/script", + want: map[string]ChangedTest{ + "bundle/resources": {Added: false, VariantFilters: nil}, + }, + }, + { + name: "changed invariant config re-enables all invariant subdirs with INPUT_CONFIG filter", + diff: "M\tacceptance/bundle/invariant/configs/job.yml.tmpl", + want: map[string]ChangedTest{ + "bundle/invariant/no_drift": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + "bundle/invariant/migrate": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + }, + }, + { + name: "invariant config init.sh strips suffix to base config name", + diff: "M\tacceptance/bundle/invariant/configs/job.yml.tmpl-init.sh", + want: map[string]ChangedTest{ + "bundle/invariant/no_drift": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + "bundle/invariant/migrate": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + }, + }, + { + name: "direct change to an invariant subdir overrides config-scoped filter with all-variants", + diff: "M\tacceptance/bundle/invariant/configs/job.yml.tmpl\nM\tacceptance/bundle/invariant/no_drift/script", + want: map[string]ChangedTest{ + "bundle/invariant/no_drift": {Added: false, VariantFilters: nil}, + "bundle/invariant/migrate": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + }, + }, + { + name: "two changed invariant configs accumulate filters", + diff: "M\tacceptance/bundle/invariant/configs/job.yml.tmpl\nM\tacceptance/bundle/invariant/configs/pipeline.yml.tmpl", + want: map[string]ChangedTest{ + "bundle/invariant/no_drift": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl", "INPUT_CONFIG=pipeline.yml.tmpl"}}, + "bundle/invariant/migrate": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl", "INPUT_CONFIG=pipeline.yml.tmpl"}}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl", "INPUT_CONFIG=pipeline.yml.tmpl"}}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ParseChangedTests(tt.diff, testDirsFixture()) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/acceptance/skiplocal_test.go b/acceptance/skiplocal_test.go index c72f7c39a3f..90bfbe8decd 100644 --- a/acceptance/skiplocal_test.go +++ b/acceptance/skiplocal_test.go @@ -1,10 +1,9 @@ package acceptance_test import ( - "os/exec" - "path/filepath" "slices" - "strings" + + "github.com/databricks/cli/acceptance/internal" ) // DATABRICKS_TEST_SKIPLOCAL controls skipping of Local acceptance tests. @@ -21,98 +20,29 @@ const ( // maxChangedLocalTests caps how many changed tests SkipLocalWithChanged re-enables, // keeping the cloud run bounded. Added tests are preferred over modified ones. maxChangedLocalTests = 50 - - invariantConfigsPrefix = "acceptance/bundle/invariant/configs/" - invariantDirPrefix = "bundle/invariant/" ) -// testDirForFile maps a repo-relative changed file (e.g. acceptance/bundle/foo/script) -// to its owning test dir relative to acceptance/ (e.g. bundle/foo), or "" if the file -// is outside acceptance/ or not under any known test dir. -func testDirForFile(repoRelPath string, testDirs map[string]bool) string { - parts := strings.Split(filepath.ToSlash(repoRelPath), "/") - if len(parts) < 2 || parts[0] != "acceptance" { - return "" - } - // Longest ancestor first so nested tests map to the innermost test dir. - for depth := len(parts); depth > 1; depth-- { - candidate := strings.Join(parts[1:depth], "/") - if testDirs[candidate] { - return candidate - } - } - return "" -} - // selectChangedLocalTests returns a map of test dir → extra env filters for // re-enabling under SkipLocalWithChanged. A nil filter slice means all variants // of that dir run; a non-nil slice restricts to variants matching those filters // (applied by the caller via checkEnvFilters in the variant loop). // Added dirs come before modified ones; the total is capped at maxChangedLocalTests. // -// A changed invariant config (acceptance/bundle/invariant/configs/*.yml.tmpl) -// maps to all invariant subdirs with an INPUT_CONFIG= filter, so touching -// job.yml.tmpl re-enables all subdirs but only for their job.yml.tmpl variants. -// -// --merge-base diffs the working tree against the merge base of HEAD and -// origin/main. This covers committed, staged, and unstaged changes alike — -// the working tree reflects all three. Untracked files (not yet git-added) -// are not visible to git diff and will not be re-enabled until staged or -// committed. The three-dot form origin/main...HEAD only covers committed -// changes and misses unstaged edits, which breaks the "touch a config, run -// the test" local dev workflow (same reason lintdiff.py uses --merge-base). +// Detection (which dirs the branch's diff against origin/main affects, and how) +// lives in internal.ChangedTests; this wrapper applies the skiplocal-specific +// ordering and cap. On error (e.g. origin/main unreachable) it returns nil, so a +// local run without origin/main degrades to skipping all Local tests. func selectChangedLocalTests(testDirs map[string]bool) map[string][]string { - out, _ := exec.Command("git", "diff", "--name-status", "--merge-base", "-M", "origin/main").Output() - diff := strings.TrimSpace(string(out)) - - // result accumulates dirs with their filters; added tracks brand-new dirs. - // nil filter slice = all variants run; non-nil = restricted to those filters. - result := map[string][]string{} - added := map[string]bool{} - - for line := range strings.SplitSeq(diff, "\n") { - fields := strings.Split(line, "\t") - if len(fields) < 2 { - continue - } - status := fields[0] - path := fields[len(fields)-1] - - // A changed invariant config re-enables all invariant subdirs with an - // INPUT_CONFIG filter, unless a subdir was already unlocked by a non-config change. - if strings.HasPrefix(path, invariantConfigsPrefix) { - configName := path[len(invariantConfigsPrefix):] - // Strip -init.sh / -cleanup.sh suffixes to get the base config name. - if i := strings.Index(configName, "-"); i > 0 && strings.HasSuffix(configName, ".sh") { - configName = configName[:i] - } - if strings.HasSuffix(configName, ".yml.tmpl") { - for dir := range testDirs { - if strings.HasPrefix(dir, invariantDirPrefix) { - if existing, ok := result[dir]; !ok || existing != nil { - result[dir] = append(result[dir], "INPUT_CONFIG="+configName) - } - } - } - } - continue - } - - dir := testDirForFile(path, testDirs) - if dir == "" { - continue - } - result[dir] = nil // nil = all variants; overrides any prior config-scoped filter - // A script file with status A means the test dir is brand new. - // Renames (R) land here as the destination path but are not "added". - if status == "A" && strings.HasSuffix(path, "/script") { - added[dir] = true - } + // Empty headRef: diff against the working tree so uncommitted local edits + // re-enable their tests without needing a commit. + changed, err := internal.ChangedTests("origin/main", "", testDirs) + if err != nil { + return nil } var addedDirs, modifiedDirs []string - for dir := range result { - if added[dir] { + for dir, ct := range changed { + if ct.Added { addedDirs = append(addedDirs, dir) } else { modifiedDirs = append(modifiedDirs, dir) @@ -126,9 +56,9 @@ func selectChangedLocalTests(testDirs map[string]bool) map[string][]string { selected = selected[:maxChangedLocalTests] } - out2 := make(map[string][]string, len(selected)) + out := make(map[string][]string, len(selected)) for _, dir := range selected { - out2[dir] = result[dir] + out[dir] = changed[dir].VariantFilters } - return out2 + return out } From 6f03ea440d532f2c50da39dadf6c77694fe7c82f Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 9 Jul 2026 15:48:26 +0200 Subject: [PATCH 2/6] acc: ignore deleted invariant configs in ChangedTests A deleted invariant config (D status) previously added an INPUT_CONFIG= filter for a variant that no longer exists, so the invariant subdirs were re-enabled for a config that can never match. Skip deletions in the invariant-config branch. Also add deletion coverage to the parser tests: a deleted file in a still-present test dir re-enables that dir; a deleted whole test dir (script gone) is ignored; a deleted config is ignored; and a deleted config alongside a modified one keeps only the modified config's filter. Co-authored-by: Isaac --- acceptance/internal/changed_tests.go | 5 +++++ acceptance/internal/changed_tests_test.go | 26 +++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/acceptance/internal/changed_tests.go b/acceptance/internal/changed_tests.go index af5dcf35608..108d6895ca6 100644 --- a/acceptance/internal/changed_tests.go +++ b/acceptance/internal/changed_tests.go @@ -76,6 +76,11 @@ func ParseChangedTests(diffOutput string, testDirs map[string]bool) map[string]C // A changed invariant config re-enables all invariant subdirs with an // INPUT_CONFIG filter, unless a subdir was already unlocked by a non-config change. if strings.HasPrefix(path, invariantConfigsPrefix) { + // A deleted config has no variant left to run, so ignore it. Without + // this, a deletion would add an INPUT_CONFIG= filter matching nothing. + if status == "D" { + continue + } configName := path[len(invariantConfigsPrefix):] // Strip -init.sh / -cleanup.sh suffixes to get the base config name. if i := strings.Index(configName, "-"); i > 0 && strings.HasSuffix(configName, ".sh") { diff --git a/acceptance/internal/changed_tests_test.go b/acceptance/internal/changed_tests_test.go index 3e2d3a5af51..4c2ea248327 100644 --- a/acceptance/internal/changed_tests_test.go +++ b/acceptance/internal/changed_tests_test.go @@ -69,6 +69,32 @@ func TestParseChangedTests(t *testing.T) { "bundle/resources": {Added: false, VariantFilters: nil}, }, }, + { + name: "deleted file in a still-existing test dir re-enables that dir", + diff: "D\tacceptance/bundle/resources/output.txt", + want: map[string]ChangedTest{ + "bundle/resources": {Added: false, VariantFilters: nil}, + }, + }, + { + name: "deleted script of a removed test dir is ignored (dir no longer in testDirs)", + diff: "D\tacceptance/bundle/removed_test/script", + want: map[string]ChangedTest{}, + }, + { + name: "deleted invariant config is ignored", + diff: "D\tacceptance/bundle/invariant/configs/job.yml.tmpl", + want: map[string]ChangedTest{}, + }, + { + name: "deleted config alongside a modified config keeps only the modified filter", + diff: "M\tacceptance/bundle/invariant/configs/job.yml.tmpl\nD\tacceptance/bundle/invariant/configs/job_with_permissions.yml.tmpl", + want: map[string]ChangedTest{ + "bundle/invariant/no_drift": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + "bundle/invariant/migrate": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + }, + }, { name: "changed invariant config re-enables all invariant subdirs with INPUT_CONFIG filter", diff: "M\tacceptance/bundle/invariant/configs/job.yml.tmpl", From 743b6b09ce0b3b8b768f505fbb5cf04c2a56b068 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 9 Jul 2026 16:04:09 +0200 Subject: [PATCH 3/6] acc: keep ChangedTest.Added sticky across diff lines ParseChangedTests overwrote the whole ChangedTest for every changed file in a dir, so a later diff line (e.g. test.toml) reset Added to false after an added script had set it true. Since git diff lines for a dir arrive in arbitrary order, a brand-new dir could end up Added=false, breaking the added-first ordering that selectChangedLocalTests relies on under maxChangedLocalTests. Preserve the existing entry when a direct change re-enables all variants: clear VariantFilters but OR in Added. This restores parity with the pre-refactor algorithm's separate added map. Add tests covering both diff-line orders and the added-subdir-overrides-config-filter interaction. Co-authored-by: Isaac --- acceptance/internal/changed_tests.go | 16 +++++++++++----- acceptance/internal/changed_tests_test.go | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/acceptance/internal/changed_tests.go b/acceptance/internal/changed_tests.go index 108d6895ca6..b664b5fdad4 100644 --- a/acceptance/internal/changed_tests.go +++ b/acceptance/internal/changed_tests.go @@ -104,12 +104,18 @@ func ParseChangedTests(diffOutput string, testDirs map[string]bool) map[string]C if dir == "" { continue } - // nil VariantFilters = all variants; overrides any prior config-scoped filter. - // A script file with status A means the test dir is brand new. Renames (R) - // land here as the destination path but are not "added". - result[dir] = ChangedTest{ - Added: status == "A" && strings.HasSuffix(path, "/script"), + // A direct change re-enables all variants, so clear any config-scoped + // filter (nil VariantFilters = all variants). Added is sticky: once any + // line marks the dir new it stays new, since diff lines for the same dir + // arrive in arbitrary order. A script file with status A means the test + // dir is brand new; renames (R) land here as the destination path but are + // not "added". + ct := result[dir] + ct.VariantFilters = nil + if status == "A" && strings.HasSuffix(path, "/script") { + ct.Added = true } + result[dir] = ct } return result diff --git a/acceptance/internal/changed_tests_test.go b/acceptance/internal/changed_tests_test.go index 4c2ea248327..d2630855802 100644 --- a/acceptance/internal/changed_tests_test.go +++ b/acceptance/internal/changed_tests_test.go @@ -45,6 +45,20 @@ func TestParseChangedTests(t *testing.T) { "bundle/resources/jobs": {Added: true, VariantFilters: nil}, }, }, + { + name: "added dir stays added when another file in it is also changed", + diff: "A\tacceptance/bundle/resources/jobs/script\nM\tacceptance/bundle/resources/jobs/test.toml", + want: map[string]ChangedTest{ + "bundle/resources/jobs": {Added: true, VariantFilters: nil}, + }, + }, + { + name: "added dir stays added regardless of diff line order", + diff: "M\tacceptance/bundle/resources/jobs/test.toml\nA\tacceptance/bundle/resources/jobs/script", + want: map[string]ChangedTest{ + "bundle/resources/jobs": {Added: true, VariantFilters: nil}, + }, + }, { name: "nested file maps to innermost test dir", diff: "M\tacceptance/bundle/resources/jobs/output.txt", @@ -122,6 +136,15 @@ func TestParseChangedTests(t *testing.T) { "bundle/invariant/continue_293": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, }, }, + { + name: "added invariant subdir overrides config-scoped filter and stays added", + diff: "M\tacceptance/bundle/invariant/configs/job.yml.tmpl\nA\tacceptance/bundle/invariant/no_drift/script", + want: map[string]ChangedTest{ + "bundle/invariant/no_drift": {Added: true, VariantFilters: nil}, + "bundle/invariant/migrate": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + }, + }, { name: "two changed invariant configs accumulate filters", diff: "M\tacceptance/bundle/invariant/configs/job.yml.tmpl\nM\tacceptance/bundle/invariant/configs/pipeline.yml.tmpl", From dd8ba49143771478472dd1a61034ce4702a400c3 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 9 Jul 2026 16:47:06 +0200 Subject: [PATCH 4/6] acc: detect cross-dir test dependencies in ChangedTests The mapper only saw a changed file's own innermost test dir, so it missed changes to files that affect tests in other dirs: - A parent script.prepare/script.cleanup/test.toml is concatenated or inherited into every descendant test, so it now re-enables that whole subtree (works even when its own dir is also a test dir with nested test dirs under it). - A helper under acceptance/bin/ is on PATH for every test, so a change there re-enables the whole suite. - Any other changed file that no test dir owns (a sourced _script, a fixture read via $TESTDIR/..) re-enables every test dir in its subtree. This over-selects but errs toward running more tests, never skipping a real one. A stray file at the acceptance root maps to nothing and is ignored. Verified against real commits: a root script.prepare change (49ea8ae30) now re-enables every test dir; earlier resource/config PRs are unchanged. Co-authored-by: Isaac --- acceptance/internal/changed_tests.go | 73 +++++++++++++++++++++ acceptance/internal/changed_tests_test.go | 78 +++++++++++++++++++++++ 2 files changed, 151 insertions(+) diff --git a/acceptance/internal/changed_tests.go b/acceptance/internal/changed_tests.go index b664b5fdad4..097b30a0f3a 100644 --- a/acceptance/internal/changed_tests.go +++ b/acceptance/internal/changed_tests.go @@ -10,6 +10,11 @@ import ( const ( invariantConfigsPrefix = "acceptance/bundle/invariant/configs/" invariantDirPrefix = "bundle/invariant/" + + acceptancePrefix = "acceptance/" + // binPrefix holds helper scripts placed on PATH for every test (print_requests.py, + // contains.py, ...), so a change to any of them can affect any test. + binPrefix = "acceptance/bin/" ) // ChangedTest describes how one acceptance test dir was affected by a branch diff. @@ -100,8 +105,33 @@ func ParseChangedTests(diffOutput string, testDirs map[string]bool) map[string]C continue } + // bin/ holds helper scripts placed on PATH for every test, so a change to + // any of them can affect any test: re-enable the whole suite. + if strings.HasPrefix(path, binPrefix) { + markSubtree("", testDirs, result) + continue + } + + // A shared file (script.prepare/script.cleanup/test.toml) is concatenated + // or inherited into every test at or below its dir, so it re-enables that + // whole subtree. This runs before the single-dir mapping because such a + // file can live inside a test dir that also has nested test dirs, which + // the single-dir mapping would miss. + if sharedTestFile(path) { + markSubtree(containingDir(path), testDirs, result) + continue + } + dir := testDirForFile(path, testDirs) if dir == "" { + // The file is not owned by any test dir. It is a shared input in a + // non-test dir (a sourced _script, a fixture read via $TESTDIR/..), + // so conservatively re-enable every test dir in its subtree. A stray + // file at the acceptance root (no subtree dir) affects nothing + // identifiable and is ignored. + if base := containingDir(path); base != "" { + markSubtree(base, testDirs, result) + } continue } // A direct change re-enables all variants, so clear any config-scoped @@ -121,6 +151,49 @@ func ParseChangedTests(diffOutput string, testDirs map[string]bool) map[string]C return result } +// sharedTestFilenames are files that, when they live in an ancestor dir, affect +// every test below them: script.prepare / script.cleanup are concatenated into +// each descendant's script (see readMergedScriptContents in acceptance_test.go) +// and test.toml config is inherited by descendants. +var sharedTestFilenames = map[string]bool{ + "script.prepare": true, + "script.cleanup": true, + "test.toml": true, +} + +// sharedTestFile reports whether path is a shared file whose change propagates to +// every test in its subtree. +func sharedTestFile(path string) bool { + return strings.HasPrefix(path, acceptancePrefix) && sharedTestFilenames[filepath.Base(path)] +} + +// containingDir returns the dir of an acceptance/ path relative to acceptance/ +// ("" for a file directly under acceptance/), or "" if path is outside acceptance/. +func containingDir(path string) string { + if !strings.HasPrefix(path, acceptancePrefix) { + return "" + } + dir := filepath.ToSlash(filepath.Dir(path[len(acceptancePrefix):])) + if dir == "." { + return "" + } + return dir +} + +// markSubtree re-enables (with all variants) every test dir equal to or nested +// under base. An empty base matches every test dir. It preserves any existing +// Added flag and only clears VariantFilters, since a subtree-wide change affects +// all variants. +func markSubtree(base string, testDirs map[string]bool, result map[string]ChangedTest) { + for dir := range testDirs { + if base == "" || dir == base || strings.HasPrefix(dir, base+"/") { + ct := result[dir] + ct.VariantFilters = nil + result[dir] = ct + } + } +} + // testDirForFile maps a repo-relative changed file (e.g. acceptance/bundle/foo/script) // to its owning test dir relative to acceptance/ (e.g. bundle/foo), or "" if the file // is outside acceptance/ or not under any known test dir. diff --git a/acceptance/internal/changed_tests_test.go b/acceptance/internal/changed_tests_test.go index d2630855802..930806865e4 100644 --- a/acceptance/internal/changed_tests_test.go +++ b/acceptance/internal/changed_tests_test.go @@ -90,6 +90,84 @@ func TestParseChangedTests(t *testing.T) { "bundle/resources": {Added: false, VariantFilters: nil}, }, }, + { + name: "parent script.prepare re-enables all descendant test dirs", + diff: "M\tacceptance/bundle/resources/script.prepare", + want: map[string]ChangedTest{ + "bundle/resources": {Added: false, VariantFilters: nil}, + "bundle/resources/jobs": {Added: false, VariantFilters: nil}, + }, + }, + { + name: "parent script.cleanup re-enables all descendant test dirs", + diff: "M\tacceptance/bundle/resources/script.cleanup", + want: map[string]ChangedTest{ + "bundle/resources": {Added: false, VariantFilters: nil}, + "bundle/resources/jobs": {Added: false, VariantFilters: nil}, + }, + }, + { + name: "parent test.toml re-enables all descendant test dirs", + diff: "M\tacceptance/bundle/invariant/test.toml", + want: map[string]ChangedTest{ + "bundle/invariant/no_drift": {Added: false, VariantFilters: nil}, + "bundle/invariant/migrate": {Added: false, VariantFilters: nil}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: nil}, + }, + }, + { + name: "root script.prepare re-enables every test dir", + diff: "M\tacceptance/script.prepare", + want: map[string]ChangedTest{ + "bundle/resources": {Added: false, VariantFilters: nil}, + "bundle/resources/jobs": {Added: false, VariantFilters: nil}, + "bundle/invariant/no_drift": {Added: false, VariantFilters: nil}, + "bundle/invariant/migrate": {Added: false, VariantFilters: nil}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: nil}, + "cmd/version": {Added: false, VariantFilters: nil}, + }, + }, + { + name: "test.toml directly in a test dir re-enables only that dir", + diff: "M\tacceptance/bundle/resources/jobs/test.toml", + want: map[string]ChangedTest{ + "bundle/resources/jobs": {Added: false, VariantFilters: nil}, + }, + }, + { + name: "parent shared file preserves an added descendant", + diff: "A\tacceptance/bundle/resources/jobs/script\nM\tacceptance/bundle/resources/test.toml", + want: map[string]ChangedTest{ + "bundle/resources": {Added: false, VariantFilters: nil}, + "bundle/resources/jobs": {Added: true, VariantFilters: nil}, + }, + }, + { + name: "a bin helper re-enables every test dir", + diff: "M\tacceptance/bin/print_requests.py", + want: map[string]ChangedTest{ + "bundle/resources": {Added: false, VariantFilters: nil}, + "bundle/resources/jobs": {Added: false, VariantFilters: nil}, + "bundle/invariant/no_drift": {Added: false, VariantFilters: nil}, + "bundle/invariant/migrate": {Added: false, VariantFilters: nil}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: nil}, + "cmd/version": {Added: false, VariantFilters: nil}, + }, + }, + { + name: "a shared fixture in a non-test dir re-enables its subtree", + diff: "M\tacceptance/bundle/invariant/_script", + want: map[string]ChangedTest{ + "bundle/invariant/no_drift": {Added: false, VariantFilters: nil}, + "bundle/invariant/migrate": {Added: false, VariantFilters: nil}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: nil}, + }, + }, + { + name: "a stray file at the acceptance root is ignored", + diff: "M\tacceptance/README.md", + want: map[string]ChangedTest{}, + }, { name: "deleted script of a removed test dir is ignored (dir no longer in testDirs)", diff: "D\tacceptance/bundle/removed_test/script", From b06a4ccdc8757c68c9601ab177ff947d3f522235 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 20 Jul 2026 11:27:34 +0200 Subject: [PATCH 5/6] acc: handle renames and deleted config helpers in ChangedTests Two edge cases surfaced in review, both from the parser looking at only the new path of each diff record and treating deletes too coarsely: - A rename affects tests at both paths, but only the new path was classified. Moving an inherited test.toml/script.prepare out of a parent dir left its old descendants unmarked. Expand each rename record into a deletion of the old path plus a modification of the new, so one classifier covers deletes, renames, and edits uniformly (the new path stays "M", not "A", so a moved script does not mark its dir brand-new). - A deleted invariant config helper (job.yml.tmpl-init.sh) was ignored along with the config template itself, but the template still exists and its variant changed. Only skip a deletion when the deleted path is the .yml.tmpl template. Extract the per-path logic into classifyPath so the rename expansion reuses it. Co-authored-by: Isaac --- acceptance/internal/changed_tests.go | 154 ++++++++++++---------- acceptance/internal/changed_tests_test.go | 26 ++++ 2 files changed, 110 insertions(+), 70 deletions(-) diff --git a/acceptance/internal/changed_tests.go b/acceptance/internal/changed_tests.go index 097b30a0f3a..496a5abf6ca 100644 --- a/acceptance/internal/changed_tests.go +++ b/acceptance/internal/changed_tests.go @@ -61,12 +61,11 @@ func ChangedTests(baseRef, headRef string, testDirs map[string]bool) (map[string // of affected acceptance test dirs. It is split from ChangedTests so the mapping // logic can be unit-tested with literal diff strings, without a git repo. // -// Each diff line is "\t" (or "\t\t" for renames); -// the last field is the current path. A changed invariant config -// (acceptance/bundle/invariant/configs/*.yml.tmpl) maps to all invariant subdirs -// with an INPUT_CONFIG= filter, so touching job.yml.tmpl re-enables all subdirs -// but only for their job.yml.tmpl variants. A direct change to a subdir takes -// precedence and re-enables all of its variants. +// A rename record "R\t\t" affects tests at both paths: the old +// path's removal and the new path's modification can each touch a different test +// or shared input. Each record is therefore expanded into one classification per +// affected path (a rename into a deletion of the old path plus a modification of +// the new) so a single classifier covers deletes, renames, and edits uniformly. func ParseChangedTests(diffOutput string, testDirs map[string]bool) map[string]ChangedTest { result := map[string]ChangedTest{} @@ -76,79 +75,94 @@ func ParseChangedTests(diffOutput string, testDirs map[string]bool) map[string]C continue } status := fields[0] - path := fields[len(fields)-1] - - // A changed invariant config re-enables all invariant subdirs with an - // INPUT_CONFIG filter, unless a subdir was already unlocked by a non-config change. - if strings.HasPrefix(path, invariantConfigsPrefix) { - // A deleted config has no variant left to run, so ignore it. Without - // this, a deletion would add an INPUT_CONFIG= filter matching nothing. - if status == "D" { - continue - } - configName := path[len(invariantConfigsPrefix):] - // Strip -init.sh / -cleanup.sh suffixes to get the base config name. - if i := strings.Index(configName, "-"); i > 0 && strings.HasSuffix(configName, ".sh") { - configName = configName[:i] - } - if strings.HasSuffix(configName, ".yml.tmpl") { - for dir := range testDirs { - if strings.HasPrefix(dir, invariantDirPrefix) { - existing, ok := result[dir] - if !ok || existing.VariantFilters != nil { - existing.VariantFilters = append(existing.VariantFilters, "INPUT_CONFIG="+configName) - result[dir] = existing - } - } - } - } + if strings.HasPrefix(status, "R") { + // A rename removes the old path and modifies content at the new one. + // The new path is "M", not "A": moving a script into a dir does not + // make that dir a brand-new test. + classifyPath("D", fields[1], testDirs, result) + classifyPath("M", fields[2], testDirs, result) continue } + classifyPath(status, fields[len(fields)-1], testDirs, result) + } - // bin/ holds helper scripts placed on PATH for every test, so a change to - // any of them can affect any test: re-enable the whole suite. - if strings.HasPrefix(path, binPrefix) { - markSubtree("", testDirs, result) - continue - } + return result +} - // A shared file (script.prepare/script.cleanup/test.toml) is concatenated - // or inherited into every test at or below its dir, so it re-enables that - // whole subtree. This runs before the single-dir mapping because such a - // file can live inside a test dir that also has nested test dirs, which - // the single-dir mapping would miss. - if sharedTestFile(path) { - markSubtree(containingDir(path), testDirs, result) - continue +// classifyPath records the test dirs affected by a single changed path with the +// given git status (A/M/D). It handles, in precedence order: invariant configs, +// bin/ helpers, shared inherited files, an owning test dir, and finally the +// subtree of any other shared input in a non-test dir. +func classifyPath(status, path string, testDirs map[string]bool, result map[string]ChangedTest) { + // A changed invariant config re-enables all invariant subdirs with an + // INPUT_CONFIG filter, unless a subdir was already unlocked by a non-config change. + if strings.HasPrefix(path, invariantConfigsPrefix) { + configName := path[len(invariantConfigsPrefix):] + // Strip -init.sh / -cleanup.sh suffixes to get the base config name. + if i := strings.Index(configName, "-"); i > 0 && strings.HasSuffix(configName, ".sh") { + configName = configName[:i] } - - dir := testDirForFile(path, testDirs) - if dir == "" { - // The file is not owned by any test dir. It is a shared input in a - // non-test dir (a sourced _script, a fixture read via $TESTDIR/..), - // so conservatively re-enable every test dir in its subtree. A stray - // file at the acceptance root (no subtree dir) affects nothing - // identifiable and is ignored. - if base := containingDir(path); base != "" { - markSubtree(base, testDirs, result) - } - continue + if !strings.HasSuffix(configName, ".yml.tmpl") { + return + } + // A deleted config template has no variant left to run, so ignore it; + // otherwise it would add an INPUT_CONFIG= filter matching nothing. A + // deleted -init.sh/-cleanup.sh helper is not the template (configName was + // stripped above), so its variant still exists and is re-enabled. + if status == "D" && strings.HasSuffix(path, ".yml.tmpl") { + return } - // A direct change re-enables all variants, so clear any config-scoped - // filter (nil VariantFilters = all variants). Added is sticky: once any - // line marks the dir new it stays new, since diff lines for the same dir - // arrive in arbitrary order. A script file with status A means the test - // dir is brand new; renames (R) land here as the destination path but are - // not "added". - ct := result[dir] - ct.VariantFilters = nil - if status == "A" && strings.HasSuffix(path, "/script") { - ct.Added = true + for dir := range testDirs { + if strings.HasPrefix(dir, invariantDirPrefix) { + existing, ok := result[dir] + if !ok || existing.VariantFilters != nil { + existing.VariantFilters = append(existing.VariantFilters, "INPUT_CONFIG="+configName) + result[dir] = existing + } + } } - result[dir] = ct + return } - return result + // bin/ holds helper scripts placed on PATH for every test, so a change to + // any of them can affect any test: re-enable the whole suite. + if strings.HasPrefix(path, binPrefix) { + markSubtree("", testDirs, result) + return + } + + // A shared file (script.prepare/script.cleanup/test.toml) is concatenated or + // inherited into every test at or below its dir, so it re-enables that whole + // subtree. This runs before the single-dir mapping because such a file can + // live inside a test dir that also has nested test dirs, which the single-dir + // mapping would miss. + if sharedTestFile(path) { + markSubtree(containingDir(path), testDirs, result) + return + } + + dir := testDirForFile(path, testDirs) + if dir == "" { + // The file is not owned by any test dir. It is a shared input in a + // non-test dir (a sourced _script, a fixture read via $TESTDIR/..), so + // conservatively re-enable every test dir in its subtree. A stray file at + // the acceptance root (no subtree dir) affects nothing identifiable and is + // ignored. + if base := containingDir(path); base != "" { + markSubtree(base, testDirs, result) + } + return + } + // A direct change re-enables all variants, so clear any config-scoped filter + // (nil VariantFilters = all variants). Added is sticky: once any path marks + // the dir new it stays new, since paths for the same dir arrive in arbitrary + // order. A script file with status A means the test dir is brand new. + ct := result[dir] + ct.VariantFilters = nil + if status == "A" && strings.HasSuffix(path, "/script") { + ct.Added = true + } + result[dir] = ct } // sharedTestFilenames are files that, when they live in an ancestor dir, affect diff --git a/acceptance/internal/changed_tests_test.go b/acceptance/internal/changed_tests_test.go index 930806865e4..361b65065ff 100644 --- a/acceptance/internal/changed_tests_test.go +++ b/acceptance/internal/changed_tests_test.go @@ -178,6 +178,32 @@ func TestParseChangedTests(t *testing.T) { diff: "D\tacceptance/bundle/invariant/configs/job.yml.tmpl", want: map[string]ChangedTest{}, }, + { + name: "deleted invariant config helper still re-enables its variant", + diff: "D\tacceptance/bundle/invariant/configs/job.yml.tmpl-init.sh", + want: map[string]ChangedTest{ + "bundle/invariant/no_drift": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + "bundle/invariant/migrate": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: []string{"INPUT_CONFIG=job.yml.tmpl"}}, + }, + }, + { + name: "renamed invariant config re-enables the new variant only", + diff: "R096\tacceptance/bundle/invariant/configs/job.yml.tmpl\tacceptance/bundle/invariant/configs/task.yml.tmpl", + want: map[string]ChangedTest{ + "bundle/invariant/no_drift": {Added: false, VariantFilters: []string{"INPUT_CONFIG=task.yml.tmpl"}}, + "bundle/invariant/migrate": {Added: false, VariantFilters: []string{"INPUT_CONFIG=task.yml.tmpl"}}, + "bundle/invariant/continue_293": {Added: false, VariantFilters: []string{"INPUT_CONFIG=task.yml.tmpl"}}, + }, + }, + { + name: "renamed shared file re-enables descendants via its old path", + diff: "R100\tacceptance/bundle/resources/test.toml\tacceptance/bundle/resources/jobs/test.toml", + want: map[string]ChangedTest{ + "bundle/resources": {Added: false, VariantFilters: nil}, + "bundle/resources/jobs": {Added: false, VariantFilters: nil}, + }, + }, { name: "deleted config alongside a modified config keeps only the modified filter", diff: "M\tacceptance/bundle/invariant/configs/job.yml.tmpl\nD\tacceptance/bundle/invariant/configs/job_with_permissions.yml.tmpl", From 658e3b78b058cec1593cb06666f303e9f3087f8a Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 20 Jul 2026 11:45:39 +0200 Subject: [PATCH 6/6] acc: simplify classifyPath into a switch with named helpers Pure refactor, no behavior change: classifyPath was a five-branch precedence ladder mixing dispatch with dense per-case logic. Turn it into a switch that delegates the two knowledge-dense cases to named helpers (markInvariantConfig, markTestDir), so the dispatch reads at a glance and the details are opt-in. Co-authored-by: Isaac --- acceptance/internal/changed_tests.go | 100 ++++++++++++++------------- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/acceptance/internal/changed_tests.go b/acceptance/internal/changed_tests.go index 496a5abf6ca..f32524054b4 100644 --- a/acceptance/internal/changed_tests.go +++ b/acceptance/internal/changed_tests.go @@ -90,73 +90,75 @@ func ParseChangedTests(diffOutput string, testDirs map[string]bool) map[string]C } // classifyPath records the test dirs affected by a single changed path with the -// given git status (A/M/D). It handles, in precedence order: invariant configs, -// bin/ helpers, shared inherited files, an owning test dir, and finally the -// subtree of any other shared input in a non-test dir. +// given git status (A/M/D), in precedence order. func classifyPath(status, path string, testDirs map[string]bool, result map[string]ChangedTest) { - // A changed invariant config re-enables all invariant subdirs with an - // INPUT_CONFIG filter, unless a subdir was already unlocked by a non-config change. - if strings.HasPrefix(path, invariantConfigsPrefix) { - configName := path[len(invariantConfigsPrefix):] - // Strip -init.sh / -cleanup.sh suffixes to get the base config name. - if i := strings.Index(configName, "-"); i > 0 && strings.HasSuffix(configName, ".sh") { - configName = configName[:i] - } - if !strings.HasSuffix(configName, ".yml.tmpl") { - return - } - // A deleted config template has no variant left to run, so ignore it; - // otherwise it would add an INPUT_CONFIG= filter matching nothing. A - // deleted -init.sh/-cleanup.sh helper is not the template (configName was - // stripped above), so its variant still exists and is re-enabled. - if status == "D" && strings.HasSuffix(path, ".yml.tmpl") { - return - } - for dir := range testDirs { - if strings.HasPrefix(dir, invariantDirPrefix) { - existing, ok := result[dir] - if !ok || existing.VariantFilters != nil { - existing.VariantFilters = append(existing.VariantFilters, "INPUT_CONFIG="+configName) - result[dir] = existing - } - } - } - return - } + switch { + case strings.HasPrefix(path, invariantConfigsPrefix): + markInvariantConfig(status, path, testDirs, result) - // bin/ holds helper scripts placed on PATH for every test, so a change to - // any of them can affect any test: re-enable the whole suite. - if strings.HasPrefix(path, binPrefix) { + // bin/ helpers are on PATH for every test, so a change re-enables the whole suite. + case strings.HasPrefix(path, binPrefix): markSubtree("", testDirs, result) - return - } // A shared file (script.prepare/script.cleanup/test.toml) is concatenated or // inherited into every test at or below its dir, so it re-enables that whole - // subtree. This runs before the single-dir mapping because such a file can - // live inside a test dir that also has nested test dirs, which the single-dir - // mapping would miss. - if sharedTestFile(path) { + // subtree. This precedes the single-dir mapping because such a file can live + // inside a test dir that also has nested test dirs, which the mapping misses. + case sharedTestFile(path): markSubtree(containingDir(path), testDirs, result) + + default: + markTestDir(status, path, testDirs, result) + } +} + +// markInvariantConfig handles a changed acceptance/bundle/invariant/configs file. +// It re-enables all invariant subdirs with an INPUT_CONFIG filter for the config, +// unless a subdir was already unlocked (all variants) by a non-config change. +func markInvariantConfig(status, path string, testDirs map[string]bool, result map[string]ChangedTest) { + configName := path[len(invariantConfigsPrefix):] + // Strip -init.sh / -cleanup.sh suffixes to get the base config name. + if i := strings.Index(configName, "-"); i > 0 && strings.HasSuffix(configName, ".sh") { + configName = configName[:i] + } + if !strings.HasSuffix(configName, ".yml.tmpl") { + return + } + // A deleted config template has no variant left to run, so ignore it; otherwise + // it would add an INPUT_CONFIG= filter matching nothing. A deleted + // -init.sh/-cleanup.sh helper is not the template (configName was stripped + // above), so its variant still exists and is re-enabled. + if status == "D" && strings.HasSuffix(path, ".yml.tmpl") { return } + for dir := range testDirs { + if strings.HasPrefix(dir, invariantDirPrefix) { + existing, ok := result[dir] + if !ok || existing.VariantFilters != nil { + existing.VariantFilters = append(existing.VariantFilters, "INPUT_CONFIG="+configName) + result[dir] = existing + } + } + } +} +// markTestDir handles a path that is neither an invariant config, a bin/ helper, +// nor a shared file. If a test dir owns it, only that dir is re-enabled; otherwise +// the path is a shared input in a non-test dir (a sourced _script, a fixture read +// via $TESTDIR/..) and its whole subtree is re-enabled. A stray file at the +// acceptance root affects nothing identifiable and is ignored. +func markTestDir(status, path string, testDirs map[string]bool, result map[string]ChangedTest) { dir := testDirForFile(path, testDirs) if dir == "" { - // The file is not owned by any test dir. It is a shared input in a - // non-test dir (a sourced _script, a fixture read via $TESTDIR/..), so - // conservatively re-enable every test dir in its subtree. A stray file at - // the acceptance root (no subtree dir) affects nothing identifiable and is - // ignored. if base := containingDir(path); base != "" { markSubtree(base, testDirs, result) } return } // A direct change re-enables all variants, so clear any config-scoped filter - // (nil VariantFilters = all variants). Added is sticky: once any path marks - // the dir new it stays new, since paths for the same dir arrive in arbitrary - // order. A script file with status A means the test dir is brand new. + // (nil VariantFilters = all variants). Added is sticky: once any path marks the + // dir new it stays new, since paths for the same dir arrive in arbitrary order. + // A script file with status A means the test dir is brand new. ct := result[dir] ct.VariantFilters = nil if status == "A" && strings.HasSuffix(path, "/script") {