diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 34dd52f5cb9..3805300a8e7 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -641,25 +641,28 @@ func testAccept(t *testing.T, inprocessMode bool, selectedTests []string, skipTo } runTest(t, dir, 0, coverDir, repls.Clone(), config, nil, envFilters, sandboxProxyURL) } else { + matched := false for ind, envset := range expanded { + // Under selection.EnvVar, an invariant dir re-enabled by a + // specific config change runs only its matching variants. + if selectChanged && !selection.MatchesFilters(envset, changedTests[dir]) { + continue + } + matched = true envname := strings.Join(envset, "/") t.Run(envname, func(t *testing.T) { if runParallel { t.Parallel() } - // Under selection.EnvVar, an invariant dir re-enabled by a - // specific config change runs only its matching variants. - if selectChanged { - if variantFilters := changedTests[dir]; variantFilters != nil { - checkEnvFilters(t, envset, variantFilters) - } - } if reason := subset.skipReason(dir, envset); reason != "" { t.Skip(reason) } runTest(t, dir, ind, coverDir, repls.Clone(), config, envset, envFilters, sandboxProxyURL) }) } + if selectChanged && !matched { + t.Skip("No environment matrix variants match changed-test selection") + } } }) } diff --git a/acceptance/internal/selection/selection.go b/acceptance/internal/selection/selection.go index 95aa66eda95..84fd12c5f89 100644 --- a/acceptance/internal/selection/selection.go +++ b/acceptance/internal/selection/selection.go @@ -12,6 +12,7 @@ import ( "os" "os/exec" "path/filepath" + "reflect" "slices" "strconv" "strings" @@ -225,7 +226,119 @@ func FromGit(root string, testDirs map[string]bool, limit int) (Result, error) { return Result{}, fmt.Errorf("git diff --merge-base origin/main failed: %w", err) } - return FromDiff(root, strings.TrimSpace(string(out)), testDirs, limit), nil + diff, err := withoutAddedExclusions(root, strings.TrimSpace(string(out))) + if err != nil { + return Result{}, err + } + return FromDiff(root, diff, testDirs, limit), nil +} + +// withoutAddedExclusions ignores invariant configs that only disable more variants: +// none of the remaining variants changed, so running the whole directory adds no coverage. +func withoutAddedExclusions(root, diff string) (string, error) { + lines := strings.Split(diff, "\n") + var base string + for i, line := range lines { + fields := strings.Split(line, "\t") + if len(fields) != 2 || fields[0] != "M" { + continue + } + path := fields[1] + if !strings.HasPrefix(path, acceptanceDirPrefix+invariantDirPrefix) || filepath.Base(path) != internal.ConfigFilename { + continue + } + if base == "" { + out, err := exec.Command("git", "merge-base", "HEAD", "origin/main").CombinedOutput() + if err != nil { + return "", fmt.Errorf("finding selection merge base: %w: %s", err, strings.TrimSpace(string(out))) + } + base = strings.TrimSpace(string(out)) + } + before, err := exec.Command("git", "show", base+":"+path).CombinedOutput() + if err != nil { + return "", fmt.Errorf("reading %s at %s: %w: %s", path, base, err, strings.TrimSpace(string(before))) + } + after, err := os.ReadFile(filepath.Join(root, strings.TrimPrefix(path, acceptanceDirPrefix))) + if err != nil { + return "", fmt.Errorf("reading %s: %w", path, err) + } + added := addedExclusions(before, after) + if len(added) == 0 { + continue + } + inherited, err := inheritedExclusions(base, path) + if err != nil { + return "", err + } + // A new leaf rule can override an inherited rule and re-enable its old variants. + shadowsInherited := false + for name := range added { + if inherited[name] { + shadowsInherited = true + break + } + } + if !shadowsInherited { + lines[i] = "" + } + } + return strings.Join(lines, "\n"), nil +} + +// addedExclusions returns the new rules if adding exclusions is the only config change. +func addedExclusions(before, after []byte) map[string][]string { + var oldConfig, newConfig internal.TestConfig + if metadata, err := toml.Decode(string(before), &oldConfig); err != nil || len(metadata.Undecoded()) > 0 { + return nil + } + if metadata, err := toml.Decode(string(after), &newConfig); err != nil || len(metadata.Undecoded()) > 0 { + return nil + } + oldRules, newRules := oldConfig.EnvMatrixExclude, newConfig.EnvMatrixExclude + if len(newRules) <= len(oldRules) { + return nil + } + for name, rule := range oldRules { + if !reflect.DeepEqual(rule, newRules[name]) { + return nil + } + } + oldConfig.EnvMatrixExclude, newConfig.EnvMatrixExclude = nil, nil + if !reflect.DeepEqual(oldConfig, newConfig) { + return nil + } + for name := range oldRules { + delete(newRules, name) + } + return newRules +} + +// inheritedExclusions returns the ancestor exclusion rule names at base. +func inheritedExclusions(base, path string) (map[string]bool, error) { + args := []string{"ls-tree", "-r", "--full-tree", "--name-only", base, "--"} + for dir := filepath.Dir(filepath.Dir(path)); filepath.ToSlash(dir)+"/" != acceptanceDirPrefix; dir = filepath.Dir(dir) { + args = append(args, filepath.ToSlash(filepath.Join(dir, internal.ConfigFilename))) + } + args = append(args, acceptanceDirPrefix+internal.ConfigFilename) + out, err := exec.Command("git", args...).CombinedOutput() + if err != nil { + return nil, fmt.Errorf("listing inherited configs at %s: %w: %s", base, err, strings.TrimSpace(string(out))) + } + rules := map[string]bool{} + for path := range strings.FieldsSeq(string(out)) { + contents, err := exec.Command("git", "show", base+":"+path).CombinedOutput() + if err != nil { + return nil, fmt.Errorf("reading %s at %s: %w: %s", path, base, err, strings.TrimSpace(string(contents))) + } + var config internal.TestConfig + if _, err := toml.Decode(string(contents), &config); err != nil { + return nil, fmt.Errorf("parsing %s at %s: %w", path, base, err) + } + for name := range config.EnvMatrixExclude { + rules[name] = true + } + } + return rules, nil } // testDirForFile maps a repo-relative changed file (e.g. acceptance/bundle/foo/script) diff --git a/acceptance/internal/selection/selection_test.go b/acceptance/internal/selection/selection_test.go index 89b23e7666b..5c5db43185c 100644 --- a/acceptance/internal/selection/selection_test.go +++ b/acceptance/internal/selection/selection_test.go @@ -2,6 +2,7 @@ package selection_test import ( "os" + "os/exec" "path/filepath" "strings" "testing" @@ -345,3 +346,81 @@ func TestFromDiffTiesAreAlphabetical(t *testing.T) { assert.Equal(t, expected, names) } } + +func TestFromGitExclusionChanges(t *testing.T) { + const before = "Cloud = false\nEnvMatrixExclude.old = [\"INPUT_CONFIG=old.yml.tmpl\"]\n" + const added = before + "EnvMatrixExclude.new = [\"INPUT_CONFIG=new.yml.tmpl\"]\n" + for _, tc := range []struct { + name string + before string + after string + parent string + script bool + stage bool + commit bool + wantDir bool + }{ + {name: "added exclusion", before: before, after: added}, + {name: "first exclusion", before: "Cloud = false\n", after: "Cloud = false\nEnvMatrixExclude.new = [\"INPUT_CONFIG=new.yml.tmpl\"]\n"}, + {name: "empty config", before: "", after: "EnvMatrixExclude.new = [\"INPUT_CONFIG=new.yml.tmpl\"]\n"}, + {name: "staged exclusion", before: before, after: added, stage: true}, + {name: "committed exclusion", before: before, after: added, stage: true, commit: true}, + {name: "removed exclusion", before: before, after: "Cloud = false\n", wantDir: true}, + {name: "replaced exclusion", before: before, after: "Cloud = false\nEnvMatrixExclude.old = [\"INPUT_CONFIG=new.yml.tmpl\"]\n", wantDir: true}, + {name: "other setting changed", before: before, after: strings.Replace(added, "false", "true", 1), wantDir: true}, + {name: "script changed too", before: before, after: added, script: true, wantDir: true}, + {name: "invalid config", before: before, after: added + "invalid = [\n", wantDir: true}, + {name: "invalid rule type", before: before, after: before + "EnvMatrixExclude.new = 1\n", wantDir: true}, + {name: "unknown setting", before: before, after: added + "Unknown = true\n", wantDir: true}, + {name: "overrides inherited exclusion", before: before, after: added, parent: "EnvMatrixExclude.new = [\"INPUT_CONFIG=other.yml.tmpl\"]\n", wantDir: true}, + {name: "distinct inherited exclusion", before: before, after: added, parent: "EnvMatrixExclude.other = [\"INPUT_CONFIG=other.yml.tmpl\"]\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + repo := t.TempDir() + t.Chdir(repo) + runSelectionGit(t, "init", "--quiet", "--initial-branch=main") + root := filepath.Join(repo, "acceptance") + dir := "bundle/invariant/jobs" + configPath := filepath.Join(root, dir, "test.toml") + scriptPath := filepath.Join(root, dir, "script") + require.NoError(t, os.MkdirAll(filepath.Dir(configPath), 0o755)) + require.NoError(t, os.WriteFile(configPath, []byte(tc.before), 0o644)) + require.NoError(t, os.WriteFile(scriptPath, []byte("echo before\n"), 0o644)) + if tc.parent != "" { + require.NoError(t, os.WriteFile(filepath.Join(root, "bundle/invariant/test.toml"), []byte(tc.parent), 0o644)) + } + runSelectionGit(t, "add", ".") + runSelectionGit(t, "commit", "--quiet", "-m", "baseline") + runSelectionGit(t, "update-ref", "refs/remotes/origin/main", "HEAD") + require.NoError(t, os.WriteFile(configPath, []byte(tc.after), 0o644)) + if tc.script { + require.NoError(t, os.WriteFile(scriptPath, []byte("echo after\n"), 0o644)) + } + if tc.stage { + runSelectionGit(t, "add", ".") + } + if tc.commit { + runSelectionGit(t, "commit", "--quiet", "-m", "add exclusion") + } + + // Both the inspection command (repo root) and harness (acceptance/) use FromGit. + for _, cwd := range []string{repo, root} { + t.Chdir(cwd) + result, err := selection.FromGit(root, map[string]bool{dir: true}, 10) + require.NoError(t, err) + want := map[string][]string{} + if tc.wantDir { + want[dir] = nil + } + assert.Equal(t, want, result.Tests()) + } + }) + } +} + +func runSelectionGit(t *testing.T, args ...string) { + t.Helper() + args = append([]string{"-c", "user.name=Test", "-c", "user.email=test@example.test", "-c", "commit.gpgsign=false"}, args...) + out, err := exec.Command("git", args...).CombinedOutput() + require.NoError(t, err, "git %v: %s", args, out) +} diff --git a/acceptance/selftest/selection_exclusions/fixtures/base.toml b/acceptance/selftest/selection_exclusions/fixtures/base.toml new file mode 100644 index 00000000000..18b1a88417e --- /dev/null +++ b/acceptance/selftest/selection_exclusions/fixtures/base.toml @@ -0,0 +1 @@ +Cloud = false diff --git a/acceptance/selftest/selection_exclusions/fixtures/changed_script.txt b/acceptance/selftest/selection_exclusions/fixtures/changed_script.txt new file mode 100644 index 00000000000..dad3176d6d5 --- /dev/null +++ b/acceptance/selftest/selection_exclusions/fixtures/changed_script.txt @@ -0,0 +1 @@ +echo changed test diff --git a/acceptance/selftest/selection_exclusions/fixtures/excluded.toml b/acceptance/selftest/selection_exclusions/fixtures/excluded.toml new file mode 100644 index 00000000000..ef534fca884 --- /dev/null +++ b/acceptance/selftest/selection_exclusions/fixtures/excluded.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrixExclude.new = ["INPUT_CONFIG=new.yml.tmpl"] diff --git a/acceptance/selftest/selection_exclusions/fixtures/new.yml.tmpl b/acceptance/selftest/selection_exclusions/fixtures/new.yml.tmpl new file mode 100644 index 00000000000..61de65e9163 --- /dev/null +++ b/acceptance/selftest/selection_exclusions/fixtures/new.yml.tmpl @@ -0,0 +1 @@ +# New invariant template. diff --git a/acceptance/selftest/selection_exclusions/fixtures/new_matrix.toml b/acceptance/selftest/selection_exclusions/fixtures/new_matrix.toml new file mode 100644 index 00000000000..2ea584d95e6 --- /dev/null +++ b/acceptance/selftest/selection_exclusions/fixtures/new_matrix.toml @@ -0,0 +1 @@ +EnvMatrix.INPUT_CONFIG = ["old.yml.tmpl", "new.yml.tmpl"] diff --git a/acceptance/selftest/selection_exclusions/fixtures/old.yml.tmpl b/acceptance/selftest/selection_exclusions/fixtures/old.yml.tmpl new file mode 100644 index 00000000000..27cbd460d8e --- /dev/null +++ b/acceptance/selftest/selection_exclusions/fixtures/old.yml.tmpl @@ -0,0 +1 @@ +# Existing invariant template. diff --git a/acceptance/selftest/selection_exclusions/fixtures/old_matrix.toml b/acceptance/selftest/selection_exclusions/fixtures/old_matrix.toml new file mode 100644 index 00000000000..38bf516dc2e --- /dev/null +++ b/acceptance/selftest/selection_exclusions/fixtures/old_matrix.toml @@ -0,0 +1 @@ +EnvMatrix.INPUT_CONFIG = ["old.yml.tmpl"] diff --git a/acceptance/selftest/selection_exclusions/fixtures/script.txt b/acceptance/selftest/selection_exclusions/fixtures/script.txt new file mode 100644 index 00000000000..68fe87affaa --- /dev/null +++ b/acceptance/selftest/selection_exclusions/fixtures/script.txt @@ -0,0 +1 @@ +echo original test diff --git a/acceptance/selftest/selection_exclusions/out.test.toml b/acceptance/selftest/selection_exclusions/out.test.toml new file mode 100644 index 00000000000..18b1a88417e --- /dev/null +++ b/acceptance/selftest/selection_exclusions/out.test.toml @@ -0,0 +1 @@ +Cloud = false diff --git a/acceptance/selftest/selection_exclusions/output.txt b/acceptance/selftest/selection_exclusions/output.txt new file mode 100644 index 00000000000..1f207ea9fbb --- /dev/null +++ b/acceptance/selftest/selection_exclusions/output.txt @@ -0,0 +1,26 @@ + +=== A new template excluded from migration selects only its no_drift variant + +>>> cat acceptance/bundle/invariant/migrate/test.toml +Cloud = false +EnvMatrixExclude.new = ["INPUT_CONFIG=new.yml.tmpl"] + +>>> selection +Selected 1 changed tests (limit=50, 0 not selected) + 10 ^bundle$/^invariant$/^no_drift$/^INPUT_CONFIG=new\.yml\.tmpl$ + +=== Changing the migration script also selects its existing variants + +>>> selection +Selected 2 changed tests (limit=50, 0 not selected) + 10 ^bundle$/^invariant$/^no_drift$/^INPUT_CONFIG=new\.yml\.tmpl$ + 5 ^bundle$/^invariant$/^migrate$ + +=== Removing the exclusion selects migration, including the re-enabled variant + +>>> cat acceptance/bundle/invariant/migrate/test.toml +Cloud = false + +>>> selection +Selected 1 changed tests (limit=50, 0 not selected) + 5 ^bundle$/^invariant$/^migrate$ diff --git a/acceptance/selftest/selection_exclusions/script b/acceptance/selftest/selection_exclusions/script new file mode 100644 index 00000000000..6dd48e78021 --- /dev/null +++ b/acceptance/selftest/selection_exclusions/script @@ -0,0 +1,43 @@ +# Use a small Git repo so selection sees the contents of changed test.toml files. +# File arguments to $SELECTION only exercise path-based selection. +mkdir -p repo/acceptance/bundle/invariant/{configs,migrate,no_drift} +for test in migrate no_drift; do + cp fixtures/script.txt "repo/acceptance/bundle/invariant/$test/script" + cp fixtures/old_matrix.toml "repo/acceptance/bundle/invariant/$test/out.test.toml" +done +cp fixtures/base.toml repo/acceptance/bundle/invariant/migrate/test.toml +cp fixtures/old.yml.tmpl repo/acceptance/bundle/invariant/configs/ +cd repo +git init -qb main +git config user.name Tester +git config user.email tester@example.test +git add . +git commit -qm baseline +git update-ref refs/remotes/origin/main HEAD + +selection() { + "$SELECTION" -root acceptance +} + +title "A new template excluded from migration selects only its no_drift variant\n" +cp ../fixtures/new.yml.tmpl acceptance/bundle/invariant/configs/ +cp ../fixtures/excluded.toml acceptance/bundle/invariant/migrate/test.toml +cp ../fixtures/new_matrix.toml acceptance/bundle/invariant/no_drift/out.test.toml +git add . +trace cat acceptance/bundle/invariant/migrate/test.toml +trace selection + +title "Changing the migration script also selects its existing variants\n" +cp ../fixtures/changed_script.txt acceptance/bundle/invariant/migrate/script +trace selection + +# Make the added template and exclusion part of the baseline for the next change. +cp ../fixtures/script.txt acceptance/bundle/invariant/migrate/script +git commit -qm 'Add template and exclusion' +git update-ref refs/remotes/origin/main HEAD + +title "Removing the exclusion selects migration, including the re-enabled variant\n" +cp ../fixtures/base.toml acceptance/bundle/invariant/migrate/test.toml +cp ../fixtures/new_matrix.toml acceptance/bundle/invariant/migrate/out.test.toml +trace cat acceptance/bundle/invariant/migrate/test.toml +trace selection diff --git a/acceptance/selftest/selection_exclusions/test.toml b/acceptance/selftest/selection_exclusions/test.toml new file mode 100644 index 00000000000..e4435812870 --- /dev/null +++ b/acceptance/selftest/selection_exclusions/test.toml @@ -0,0 +1 @@ +Ignore = ["repo"]