From 57ec43dcc3f64976d225007f6290f57f052f0445 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 12 Aug 2026 12:26:22 +0200 Subject: [PATCH 01/21] acc: replace DATABRICKS_TEST_SKIPLOCAL with DATABRICKS_TEST_SELECT_CHANGED=N N is the number of changed tests to select, previously hardcoded at 50. Moved tests are now selected after added and modified ones, and the selection is logged at startup. --- .github/workflows/push.yml | 5 +- Taskfile.yml | 6 +- acceptance/acceptance_test.go | 34 +++--- ...kiplocal_test.go => selectchanged_test.go} | 114 +++++++++++++----- acceptance/selectchanged_unit_test.go | 99 +++++++++++++++ acceptance/subset_test.go | 14 +-- 6 files changed, 211 insertions(+), 61 deletions(-) rename acceptance/{skiplocal_test.go => selectchanged_test.go} (56%) create mode 100644 acceptance/selectchanged_unit_test.go diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index b4c5238326b..5e6988d3ca0 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -602,8 +602,9 @@ jobs: -f commit_sha=${{ github.event.pull_request.head.sha }} # Dispatch the nightly workflow; as a workflow_dispatch event it runs - # integration-short (CloudSlow still skips via -short, but locally-covered - # tests run against real clouds, unlike the PR task's skiplocal variant). + # integration-short (CloudSlow still skips via -short, but the whole acceptance + # suite runs against real clouds, unlike the PR task which only runs the + # acceptance tests the PR touches). - name: Trigger integration tests (push to main) if: ${{ github.event_name == 'push' }} env: diff --git a/Taskfile.yml b/Taskfile.yml index 0a2030181f6..1144eafba57 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -759,12 +759,14 @@ tasks: --packages "./acceptance ./integration/..." \ -- -parallel 4 -timeout=12h -short + # The name is referenced by the eng-dev-ecosystem workflows that dispatch it; renaming + # it here would silently fall back to integration-short (the full cloud suite). integration-short-skiplocal: - desc: Run integration tests for PRs (skips CloudSlow and tests with testserver coverage) + desc: Run integration tests for PRs (skips CloudSlow and acceptance tests the PR does not touch) deps: [install-pythons] cmds: - | - DATABRICKS_TEST_SKIPLOCAL=withchanged VERBOSE_TEST=1 \ + DATABRICKS_TEST_SELECT_CHANGED=50 VERBOSE_TEST=1 \ go run -modfile=tools/go.mod ./tools/testrunner/main.go \ {{.GO_TOOL}} gotestsum \ --format github-actions \ diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 16fd53a4785..d0d88a59ea6 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -445,22 +445,20 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { testDirsSet[d] = true } - skipLocalMode := os.Getenv(SkipLocalEnvVar) subset := newSubsetSelector(t, testdiff.OverwriteMode, Forcerun) - switch skipLocalMode { - case "", SkipLocalWithChanged: - default: - t.Fatalf("Unsupported %s=%q, expected %q", SkipLocalEnvVar, skipLocalMode, SkipLocalWithChanged) + changedLimit := getSelectChangedLimit(t) + selectChanged := changedLimit > 0 + if !selectChanged && subset.enabled { + changedLimit = subsetChangedLimit } - skipLocalWithChanged := skipLocalMode == SkipLocalWithChanged - // changedTests maps test dir to extra env filters for added/modified tests; nil - // filters means all variants of that dir changed. Both SkipLocalWithChanged and the - // subset selector keep these tests, so detect them at most once here. + // changedTests maps test dir to extra env filters for changed tests; nil filters + // means all variants of that dir changed. Both SelectChangedEnvVar and the subset + // selector keep these tests, so detect them at most once here. var changedTests map[string][]string - if skipLocalWithChanged || subset.enabled { - changedTests = selectChangedLocalTests(t, testDirsSet) + if changedLimit > 0 { + changedTests = selectChangedTests(t, testDirsSet, changedLimit) } subset.changed = changedTests @@ -518,7 +516,7 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { t.Skip("Skipping test execution (only regenerating out.test.toml)") } - skipReason := getSkipReason(&config, configPath, dir, skipLocalMode, changedTests) + skipReason := getSkipReason(&config, configPath, dir, selectChanged, changedTests) if skipReason != "" { skippedDirs += 1 t.Skip(skipReason) @@ -572,9 +570,9 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { if runParallel { t.Parallel() } - // Under SkipLocalWithChanged, an invariant dir re-enabled by a + // Under SelectChangedEnvVar, an invariant dir re-enabled by a // specific config change runs only its matching variants. - if skipLocalWithChanged { + if selectChanged { if variantFilters := changedTests[dir]; variantFilters != nil { checkEnvFilters(t, envset, variantFilters) } @@ -654,12 +652,12 @@ func validateTestPhase(phase int) error { } // Return a reason to skip the test. Empty string means "don't skip". -// skipLocalMode is the value of DATABRICKS_TEST_SKIPLOCAL read once at startup. +// selectChanged reports whether DATABRICKS_TEST_SELECT_CHANGED was set at startup. // changedTests maps test dirs to extra env filters; nil map means feature is off. -func getSkipReason(config *internal.TestConfig, configPath, dir, skipLocalMode string, changedTests map[string][]string) string { - if skipLocalMode == SkipLocalWithChanged { +func getSkipReason(config *internal.TestConfig, configPath, dir string, selectChanged bool, changedTests map[string][]string) string { + if selectChanged { if _, ok := changedTests[dir]; !ok { - return "Disabled via DATABRICKS_TEST_SKIPLOCAL=" + SkipLocalWithChanged + " in " + configPath + return "Not selected by " + SelectChangedEnvVar } } diff --git a/acceptance/skiplocal_test.go b/acceptance/selectchanged_test.go similarity index 56% rename from acceptance/skiplocal_test.go rename to acceptance/selectchanged_test.go index 4f3c7af22da..7f85af4d3d6 100644 --- a/acceptance/skiplocal_test.go +++ b/acceptance/selectchanged_test.go @@ -2,27 +2,45 @@ package acceptance_test import ( "errors" + "os" "os/exec" "path/filepath" "slices" + "strconv" "strings" "testing" ) -// Cloud PR runs set DATABRICKS_TEST_SKIPLOCAL=withchanged to skip acceptance -// tests that already run locally, except those this branch touches. +// Cloud PR runs set DATABRICKS_TEST_SELECT_CHANGED=N to run only the acceptance +// tests this branch touches, at most N of them, instead of the full suite. const ( - SkipLocalEnvVar = "DATABRICKS_TEST_SKIPLOCAL" + SelectChangedEnvVar = "DATABRICKS_TEST_SELECT_CHANGED" - SkipLocalWithChanged = "withchanged" - - // Cap re-enabled tests so cloud PR runs stay bounded; prefer added over modified. - maxChangedLocalTests = 50 + // Cap for runs that need change detection without setting the env var: the subset + // selector keeps changed tests on top of its hash-selected fraction, so a PR that + // edits hundreds of test dirs must not turn the subset cells back into a full run. + subsetChangedLimit = 50 invariantConfigsPrefix = "acceptance/bundle/invariant/configs/" invariantDirPrefix = "bundle/invariant/" ) +// getSelectChangedLimit returns the number of changed tests to select, or 0 when +// DATABRICKS_TEST_SELECT_CHANGED is unset (feature off). +func getSelectChangedLimit(t *testing.T) int { + raw := os.Getenv(SelectChangedEnvVar) + if raw == "" { + return 0 + } + + limit, err := strconv.Atoi(raw) + if err != nil || limit <= 0 { + t.Fatalf("Invalid %s=%q, expected a positive integer", SelectChangedEnvVar, raw) + } + + return limit +} + // 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. @@ -41,15 +59,10 @@ func testDirForFile(repoRelPath string, testDirs map[string]bool) string { 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. +// selectChangedTests returns a map of test dir → extra env filters for the tests this +// branch changed. 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). // // --merge-base diffs the working tree against the merge base of HEAD and // origin/main. This covers committed, staged, and unstaged changes alike — @@ -58,7 +71,7 @@ func testDirForFile(repoRelPath string, testDirs map[string]bool) string { // 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). -func selectChangedLocalTests(t *testing.T, testDirs map[string]bool) map[string][]string { +func selectChangedTests(t *testing.T, testDirs map[string]bool, limit int) map[string][]string { out, err := exec.Command("git", "diff", "--name-status", "--merge-base", "-M", "origin/main").Output() if err != nil { // A failed diff (most commonly a missing origin/main in a shallow CI @@ -71,12 +84,38 @@ func selectChangedLocalTests(t *testing.T, testDirs map[string]bool) map[string] } t.Fatalf("git diff --merge-base origin/main failed: %v\n%s", err, stderr) } - diff := strings.TrimSpace(string(out)) - // result accumulates dirs with their filters; added tracks brand-new dirs. + changed, dropped := classifyChangedTests(strings.TrimSpace(string(out)), testDirs, limit) + + // Log the outcome up front: which tests the diff picked, and how many the limit + // cut, so a CI run shows what it is about to cover without reading every skip line. + names := make([]string, 0, len(changed)) + for dir, filters := range changed { + if filters != nil { + dir += "[" + strings.Join(filters, ",") + "]" + } + names = append(names, dir) + } + slices.Sort(names) + t.Logf("Selected %d changed tests (limit=%d, %d not selected): %s", len(names), limit, dropped, strings.Join(names, " ")) + + return changed +} + +// classifyChangedTests maps `git diff --name-status` output to test dirs, keeping at +// most limit of them: added dirs first, then modified ones, then moved ones. It also +// returns how many changed dirs the limit dropped. +// +// 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. +func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map[string][]string, int) { + // result accumulates dirs with their filters; added and moved record how each dir + // itself changed, so the cap can prefer added over modified over moved. // nil filter slice = all variants run; non-nil = restricted to those filters. result := map[string][]string{} added := map[string]bool{} + moved := map[string]bool{} for line := range strings.SplitSeq(diff, "\n") { fields := strings.Split(line, "\t") @@ -120,32 +159,43 @@ func selectChangedLocalTests(t *testing.T, testDirs map[string]bool) map[string] 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 + // The status of a dir's script file says how the dir itself changed: + // A means brand new, R (Rnnn) means moved here from another path. + if strings.HasSuffix(path, "/script") { + switch { + case status == "A": + added[dir] = true + case strings.HasPrefix(status, "R"): + moved[dir] = true + } } } - var addedDirs, modifiedDirs []string + var addedDirs, modifiedDirs, movedDirs []string for dir := range result { - if added[dir] { + switch { + case added[dir]: addedDirs = append(addedDirs, dir) - } else { + case moved[dir]: + movedDirs = append(movedDirs, dir) + default: modifiedDirs = append(modifiedDirs, dir) } } slices.Sort(addedDirs) slices.Sort(modifiedDirs) + slices.Sort(movedDirs) - selected := append(addedDirs, modifiedDirs...) - if len(selected) > maxChangedLocalTests { - selected = selected[:maxChangedLocalTests] + selected := slices.Concat(addedDirs, modifiedDirs, movedDirs) + dropped := 0 + if len(selected) > limit { + dropped = len(selected) - limit + selected = selected[:limit] } - out2 := make(map[string][]string, len(selected)) + out := make(map[string][]string, len(selected)) for _, dir := range selected { - out2[dir] = result[dir] + out[dir] = result[dir] } - return out2 + return out, dropped } diff --git a/acceptance/selectchanged_unit_test.go b/acceptance/selectchanged_unit_test.go new file mode 100644 index 00000000000..406fcd6d5ec --- /dev/null +++ b/acceptance/selectchanged_unit_test.go @@ -0,0 +1,99 @@ +package acceptance_test + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +var changedTestDirs = map[string]bool{ + "bundle/added": true, + "bundle/modified": true, + "bundle/moved": true, + "bundle/untouched": true, + "bundle/invariant/jobs": true, + "bundle/invariant/apps": true, + "cmd/sync/nested": true, + "cmd/sync/nested/deeper": true, +} + +// diffLines joins name-status records the way `git diff --name-status` prints them. +func diffLines(lines ...string) string { + return strings.Join(lines, "\n") +} + +func TestClassifyChangedTestsStatuses(t *testing.T) { + diff := diffLines( + "A\tacceptance/bundle/added/script", + "M\tacceptance/bundle/modified/databricks.yml", + "R100\tacceptance/bundle/old/script\tacceptance/bundle/moved/script", + "M\tlibs/dyn/value.go", + ) + changed, dropped := classifyChangedTests(diff, changedTestDirs, 10) + assert.Zero(t, dropped) + assert.Equal(t, map[string][]string{ + "bundle/added": nil, + "bundle/modified": nil, + "bundle/moved": nil, + }, changed) +} + +func TestClassifyChangedTestsPriority(t *testing.T) { + diff := diffLines( + "R090\tacceptance/bundle/old/script\tacceptance/bundle/moved/script", + "M\tacceptance/bundle/modified/script", + "A\tacceptance/bundle/added/script", + ) + // The cap keeps added first, then modified, then moved. + for limit, expected := range map[int][]string{ + 1: {"bundle/added"}, + 2: {"bundle/added", "bundle/modified"}, + 3: {"bundle/added", "bundle/modified", "bundle/moved"}, + } { + changed, dropped := classifyChangedTests(diff, changedTestDirs, limit) + assert.Len(t, changed, limit) + assert.Equal(t, 3-limit, dropped, "limit=%d", limit) + for _, dir := range expected { + assert.Contains(t, changed, dir, "limit=%d", limit) + } + } +} + +func TestClassifyChangedTestsNestedDir(t *testing.T) { + // A file maps to the innermost test dir that owns it. + diff := diffLines("M\tacceptance/cmd/sync/nested/deeper/script") + changed, _ := classifyChangedTests(diff, changedTestDirs, 10) + assert.Equal(t, map[string][]string{"cmd/sync/nested/deeper": nil}, changed) +} + +func TestClassifyChangedTestsInvariantConfig(t *testing.T) { + // A changed invariant config re-enables every invariant dir, restricted to that config. + diff := diffLines("M\tacceptance/bundle/invariant/configs/job.yml.tmpl") + changed, _ := classifyChangedTests(diff, changedTestDirs, 10) + assert.Equal(t, map[string][]string{ + "bundle/invariant/jobs": {"INPUT_CONFIG=job.yml.tmpl"}, + "bundle/invariant/apps": {"INPUT_CONFIG=job.yml.tmpl"}, + }, changed) +} + +func TestClassifyChangedTestsInvariantConfigAndDir(t *testing.T) { + // A non-config change to an invariant dir unlocks all of its variants; the + // regenerated test.toml files are ignored. + diff := diffLines( + "M\tacceptance/bundle/invariant/configs/job.yml.tmpl", + "M\tacceptance/bundle/invariant/jobs/script", + "M\tacceptance/bundle/invariant/apps/out.test.toml", + ) + changed, _ := classifyChangedTests(diff, changedTestDirs, 10) + assert.Equal(t, map[string][]string{ + "bundle/invariant/jobs": nil, + "bundle/invariant/apps": {"INPUT_CONFIG=job.yml.tmpl"}, + }, changed) +} + +func TestClassifyChangedTestsEmptyDiff(t *testing.T) { + changed, dropped := classifyChangedTests("", changedTestDirs, 10) + assert.Empty(t, changed) + assert.Zero(t, dropped) +} diff --git a/acceptance/subset_test.go b/acceptance/subset_test.go index 22eda164b3d..435f3bf385e 100644 --- a/acceptance/subset_test.go +++ b/acceptance/subset_test.go @@ -26,10 +26,10 @@ const ( ) // subsetSelector decides, per subtest, whether it runs under TESTS_SELECT_SUBSET_PCT. -// A subtest runs if it is an added/modified test on this branch (always kept, reusing -// the same change detection as SkipLocalWithChanged), or if its seeded hash falls -// under the percentage. The decision is independent per subtest, so added/modified -// tests run on top of the hash-selected subset rather than displacing anything. +// A subtest runs if it is a changed test on this branch (always kept, reusing the same +// change detection as DATABRICKS_TEST_SELECT_CHANGED), or if its seeded hash falls +// under the percentage. The decision is independent per subtest, so changed tests run +// on top of the hash-selected subset rather than displacing anything. type subsetSelector struct { enabled bool pct int @@ -42,8 +42,8 @@ type subsetSelector struct { // newSubsetSelector reads the subset env vars. Subsetting is disabled in update mode // and under -forcerun so that every output is regenerated and forced runs are honored. -// The caller assigns .changed (the added/modified tests to always keep) so that the -// change detection is shared with SkipLocalWithChanged and runs at most once. +// The caller assigns .changed (the changed tests to always keep) so that the change +// detection is shared with DATABRICKS_TEST_SELECT_CHANGED and runs at most once. func newSubsetSelector(t *testing.T, overwrite, forcerun bool) subsetSelector { raw := os.Getenv(SubsetPctEnvVar) if raw == "" || overwrite || forcerun { @@ -88,7 +88,7 @@ func (s subsetSelector) skipReason(dir string, envset []string) string { return "Skipped by " + SubsetPctEnvVar } -// isChanged reports whether the subtest belongs to an added/modified test dir. For an +// isChanged reports whether the subtest belongs to a changed test dir. For an // invariant dir re-enabled by a specific config change, only the matching variants // count as changed. func (s subsetSelector) isChanged(dir string, envset []string) bool { From 2a05dcfe055bc48491ed03014c4cb888400f1977 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 12 Aug 2026 12:57:37 +0200 Subject: [PATCH 02/21] acc: rename integration-short-skiplocal to integration-short-selection Keep the old name as an alias: it is what the eng-dev-ecosystem CI job dispatches. --- Taskfile.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index 1144eafba57..872aa2f8eb1 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -759,10 +759,12 @@ tasks: --packages "./acceptance ./integration/..." \ -- -parallel 4 -timeout=12h -short - # The name is referenced by the eng-dev-ecosystem workflows that dispatch it; renaming - # it here would silently fall back to integration-short (the full cloud suite). - integration-short-skiplocal: + integration-short-selection: desc: Run integration tests for PRs (skips CloudSlow and acceptance tests the PR does not touch) + # The alias is the name the eng-dev-ecosystem CI job dispatches (cli-isolated-pr.yml + # passes it as integration_task, and cli-isolated-tests.yml greps Taskfile.yml for it + # and falls back to integration-short, the full cloud suite, if it is missing). + aliases: [integration-short-skiplocal] deps: [install-pythons] cmds: - | From 04346a3062541f7a867c944301e7d0c2a747fb3b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 12 Aug 2026 12:59:16 +0200 Subject: [PATCH 03/21] acc: rename integration-short-selection task to cloud-selection --- Taskfile.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Taskfile.yml b/Taskfile.yml index 872aa2f8eb1..23445e15ea2 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -759,7 +759,7 @@ tasks: --packages "./acceptance ./integration/..." \ -- -parallel 4 -timeout=12h -short - integration-short-selection: + cloud-selection: desc: Run integration tests for PRs (skips CloudSlow and acceptance tests the PR does not touch) # The alias is the name the eng-dev-ecosystem CI job dispatches (cli-isolated-pr.yml # passes it as integration_task, and cli-isolated-tests.yml greps Taskfile.yml for it From 0497fc5afc70d3f24eced8d78dc414b5a5344f71 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 12 Aug 2026 13:00:04 +0200 Subject: [PATCH 04/21] acc: reword cloud-selection task description --- Taskfile.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Taskfile.yml b/Taskfile.yml index 23445e15ea2..869cf45900d 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -760,7 +760,7 @@ tasks: -- -parallel 4 -timeout=12h -short cloud-selection: - desc: Run integration tests for PRs (skips CloudSlow and acceptance tests the PR does not touch) + desc: Run selection of integration tests (tests changed in this branch) # The alias is the name the eng-dev-ecosystem CI job dispatches (cli-isolated-pr.yml # passes it as integration_task, and cli-isolated-tests.yml greps Taskfile.yml for it # and falls back to integration-short, the full cloud suite, if it is missing). From cba2c2fe5b34c3ea9e6d33dcbd3963af0ea5cf5c Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 12 Aug 2026 14:51:58 +0200 Subject: [PATCH 05/21] acc: add local-select task, rename cloud-selection to cloud-select local-select runs the same selection of changed tests locally against the testserver. TestInprocessMode is skipped under test selection: it asserts that two specific selftests ran, which does not hold when the selection excludes them. --- Taskfile.yml | 17 ++++++++++++++++- acceptance/acceptance_test.go | 5 +++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/Taskfile.yml b/Taskfile.yml index 869cf45900d..2fb812e90f0 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -550,6 +550,21 @@ tasks: --packages ./acceptance/... \ -- -timeout=${LOCAL_TIMEOUT:-60m}{{if .ACCEPTANCE_TEST_FILTER}} -run "{{.ACCEPTANCE_TEST_FILTER}}"{{end}} + local-select: + desc: Run selection of local tests (tests changed in this branch) + # Local counterpart of cloud-select: the same DATABRICKS_TEST_SELECT_CHANGED selection, + # against the testserver instead of a real workspace. No sources/generates because the + # selection depends on the git diff, which Task cannot checksum. Run with + # GOTESTSUM_FORMAT=standard-verbose to see which tests were selected. + cmds: + - | + DATABRICKS_TEST_SELECT_CHANGED=${DATABRICKS_TEST_SELECT_CHANGED:-50} \ + {{.GO_TOOL}} gotestsum \ + --format ${GOTESTSUM_FORMAT:-pkgname-and-test-fails} \ + --no-summary=skipped \ + --packages ./acceptance \ + -- -timeout=${LOCAL_TIMEOUT:-60m} + test-update: desc: Update acceptance test output (local) # Excludes out* because the task rewrites them; keeping them in sources would @@ -759,7 +774,7 @@ tasks: --packages "./acceptance ./integration/..." \ -- -parallel 4 -timeout=12h -short - cloud-selection: + cloud-select: desc: Run selection of integration tests (tests changed in this branch) # The alias is the name the eng-dev-ecosystem CI job dispatches (cli-isolated-pr.yml # passes it as integration_task, and cli-isolated-tests.yml greps Taskfile.yml for it diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index d0d88a59ea6..f4bdfd689b1 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -141,6 +141,11 @@ func TestInprocessMode(t *testing.T) { if os.Getenv("CLOUD_ENV") != "" { t.Skip("No need to run this as integration test.") } + if os.Getenv(SelectChangedEnvVar) != "" { + // The two selftests below only run if this branch changed them, so the + // assertions on the returned count do not hold under test selection. + t.Skip("Disabled via " + SelectChangedEnvVar) + } // Uncomment to load ~/.databricks/debug-env.json to debug integration tests // testutil.LoadDebugEnvIfRunFromIDE(t, "workspace") From 7eaeba3f6ccbcb5af7f04db70cd77f9eb63a5e42 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 12 Aug 2026 15:10:47 +0200 Subject: [PATCH 06/21] acc: drop unselected tests instead of skipping them one by one A skip per test dir buried the run in ~1100 SKIP lines and hid the selection summary. local-select now runs plain `go test -v`, which shows the summary and the tests that ran. --- Taskfile.yml | 14 ++++---------- acceptance/acceptance_test.go | 22 ++++++++++++---------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index 2fb812e90f0..a82c83689b6 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -553,17 +553,11 @@ tasks: local-select: desc: Run selection of local tests (tests changed in this branch) # Local counterpart of cloud-select: the same DATABRICKS_TEST_SELECT_CHANGED selection, - # against the testserver instead of a real workspace. No sources/generates because the - # selection depends on the git diff, which Task cannot checksum. Run with - # GOTESTSUM_FORMAT=standard-verbose to see which tests were selected. + # against the testserver instead of a real workspace. -v to show the selection summary + # and the tests that ran; no sources/generates because the selection depends on the git + # diff, which Task cannot checksum. cmds: - - | - DATABRICKS_TEST_SELECT_CHANGED=${DATABRICKS_TEST_SELECT_CHANGED:-50} \ - {{.GO_TOOL}} gotestsum \ - --format ${GOTESTSUM_FORMAT:-pkgname-and-test-fails} \ - --no-summary=skipped \ - --packages ./acceptance \ - -- -timeout=${LOCAL_TIMEOUT:-60m} + - "DATABRICKS_TEST_SELECT_CHANGED=${DATABRICKS_TEST_SELECT_CHANGED:-50} go test ./acceptance -run '^TestAccept$' -v -timeout=${LOCAL_TIMEOUT:-60m}" test-update: desc: Update acceptance test output (local) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index f4bdfd689b1..2c7626baa03 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -467,6 +467,16 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { } subset.changed = changedTests + // Drop the tests that were not selected instead of skipping them per dir: a skip + // per dir buries the run in a thousand SKIP lines and hides the selection summary. + // Their out.test.toml is left alone, which is what a partial run should do. + if selectChanged { + testDirs = slices.DeleteFunc(testDirs, func(dir string) bool { + _, ok := changedTests[dir] + return !ok + }) + } + if singleTest != "" { testDirs = slices.DeleteFunc(testDirs, func(n string) bool { return n != singleTest @@ -521,7 +531,7 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { t.Skip("Skipping test execution (only regenerating out.test.toml)") } - skipReason := getSkipReason(&config, configPath, dir, selectChanged, changedTests) + skipReason := getSkipReason(&config, configPath) if skipReason != "" { skippedDirs += 1 t.Skip(skipReason) @@ -657,15 +667,7 @@ func validateTestPhase(phase int) error { } // Return a reason to skip the test. Empty string means "don't skip". -// selectChanged reports whether DATABRICKS_TEST_SELECT_CHANGED was set at startup. -// changedTests maps test dirs to extra env filters; nil map means feature is off. -func getSkipReason(config *internal.TestConfig, configPath, dir string, selectChanged bool, changedTests map[string][]string) string { - if selectChanged { - if _, ok := changedTests[dir]; !ok { - return "Not selected by " + SelectChangedEnvVar - } - } - +func getSkipReason(config *internal.TestConfig, configPath string) string { if Forcerun { return "" } From c7f6735319008e80e7a41087484e08c301a3f3e6 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 13 Aug 2026 11:00:24 +0200 Subject: [PATCH 07/21] acc: rank changed fixtures above regenerated output in test selection A regenerated golden usually comes from a change elsewhere and lands on hundreds of dirs at once, which fills the quota and crowds out the tests the branch actually edits. Order is now added, changed fixture, changed output, moved. --- acceptance/selectchanged_test.go | 32 +++++++++++++++------ acceptance/selectchanged_unit_test.go | 40 +++++++++++++++++++++------ 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/acceptance/selectchanged_test.go b/acceptance/selectchanged_test.go index 7f85af4d3d6..572f72b8aef 100644 --- a/acceptance/selectchanged_test.go +++ b/acceptance/selectchanged_test.go @@ -103,18 +103,24 @@ func selectChangedTests(t *testing.T, testDirs map[string]bool, limit int) map[s } // classifyChangedTests maps `git diff --name-status` output to test dirs, keeping at -// most limit of them: added dirs first, then modified ones, then moved ones. It also -// returns how many changed dirs the limit dropped. +// most limit of them, in this order: added dirs, dirs with a changed fixture (script, +// test.toml, databricks.yml, ...), dirs where only generated files changed (out*), and +// finally moved dirs. It also returns how many changed dirs the limit dropped. +// +// Generated files rank below fixtures because a regenerated golden usually comes from a +// change elsewhere in the tree and lands on hundreds of dirs at once, which would +// otherwise fill the whole quota and crowd out the tests this branch actually edits. // // 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. func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map[string][]string, int) { - // result accumulates dirs with their filters; added and moved record how each dir - // itself changed, so the cap can prefer added over modified over moved. + // result accumulates dirs with their filters; added, fixture and moved record how + // each dir changed, so the cap can rank them. // nil filter slice = all variants run; non-nil = restricted to those filters. result := map[string][]string{} added := map[string]bool{} + fixture := map[string]bool{} moved := map[string]bool{} for line := range strings.SplitSeq(diff, "\n") { @@ -139,6 +145,8 @@ func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map if existing, ok := result[dir]; !ok || existing != nil { result[dir] = append(result[dir], "INPUT_CONFIG="+configName) } + // The config is the fixture these dirs are generated from. + fixture[dir] = true } } } @@ -159,6 +167,11 @@ func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map continue } result[dir] = nil // nil = all variants; overrides any prior config-scoped filter + // Everything starting with "out" is generated (output.txt, out.requests.txt, + // out.test.toml); the rest is a fixture the test is defined by. + if !strings.HasPrefix(filepath.Base(path), "out") { + fixture[dir] = true + } // The status of a dir's script file says how the dir itself changed: // A means brand new, R (Rnnn) means moved here from another path. if strings.HasSuffix(path, "/script") { @@ -171,22 +184,25 @@ func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map } } - var addedDirs, modifiedDirs, movedDirs []string + var addedDirs, fixtureDirs, generatedDirs, movedDirs []string for dir := range result { switch { case added[dir]: addedDirs = append(addedDirs, dir) case moved[dir]: movedDirs = append(movedDirs, dir) + case fixture[dir]: + fixtureDirs = append(fixtureDirs, dir) default: - modifiedDirs = append(modifiedDirs, dir) + generatedDirs = append(generatedDirs, dir) } } slices.Sort(addedDirs) - slices.Sort(modifiedDirs) + slices.Sort(fixtureDirs) + slices.Sort(generatedDirs) slices.Sort(movedDirs) - selected := slices.Concat(addedDirs, modifiedDirs, movedDirs) + selected := slices.Concat(addedDirs, fixtureDirs, generatedDirs, movedDirs) dropped := 0 if len(selected) > limit { dropped = len(selected) - limit diff --git a/acceptance/selectchanged_unit_test.go b/acceptance/selectchanged_unit_test.go index 406fcd6d5ec..f457ec7cb6a 100644 --- a/acceptance/selectchanged_unit_test.go +++ b/acceptance/selectchanged_unit_test.go @@ -10,6 +10,7 @@ import ( var changedTestDirs = map[string]bool{ "bundle/added": true, "bundle/modified": true, + "bundle/regenerated": true, "bundle/moved": true, "bundle/untouched": true, "bundle/invariant/jobs": true, @@ -42,24 +43,47 @@ func TestClassifyChangedTestsStatuses(t *testing.T) { func TestClassifyChangedTestsPriority(t *testing.T) { diff := diffLines( "R090\tacceptance/bundle/old/script\tacceptance/bundle/moved/script", + "M\tacceptance/bundle/regenerated/output.txt", "M\tacceptance/bundle/modified/script", "A\tacceptance/bundle/added/script", ) - // The cap keeps added first, then modified, then moved. - for limit, expected := range map[int][]string{ - 1: {"bundle/added"}, - 2: {"bundle/added", "bundle/modified"}, - 3: {"bundle/added", "bundle/modified", "bundle/moved"}, - } { + // The cap keeps added first, then a changed fixture, then a regenerated output, + // then a moved dir. + ranked := []string{"bundle/added", "bundle/modified", "bundle/regenerated", "bundle/moved"} + for limit := 1; limit <= len(ranked); limit++ { changed, dropped := classifyChangedTests(diff, changedTestDirs, limit) assert.Len(t, changed, limit) - assert.Equal(t, 3-limit, dropped, "limit=%d", limit) - for _, dir := range expected { + assert.Equal(t, len(ranked)-limit, dropped, "limit=%d", limit) + for _, dir := range ranked[:limit] { assert.Contains(t, changed, dir, "limit=%d", limit) } } } +func TestClassifyChangedTestsFixtureBeatsOutputInSameDir(t *testing.T) { + // A dir with both a fixture and an output change ranks as a fixture change. + diff := diffLines( + "M\tacceptance/bundle/modified/output.txt", + "M\tacceptance/bundle/modified/databricks.yml", + "M\tacceptance/bundle/regenerated/out.requests.txt", + ) + changed, dropped := classifyChangedTests(diff, changedTestDirs, 1) + assert.Equal(t, map[string][]string{"bundle/modified": nil}, changed) + assert.Equal(t, 1, dropped) +} + +func TestClassifyChangedTestsInvariantConfigRanksAsFixture(t *testing.T) { + // The invariant config is the fixture its dirs are generated from, so it outranks + // a dir whose output was regenerated. + diff := diffLines( + "M\tacceptance/bundle/regenerated/output.txt", + "M\tacceptance/bundle/invariant/configs/job.yml.tmpl", + ) + changed, dropped := classifyChangedTests(diff, changedTestDirs, 2) + assert.NotContains(t, changed, "bundle/regenerated") + assert.Equal(t, 1, dropped) +} + func TestClassifyChangedTestsNestedDir(t *testing.T) { // A file maps to the innermost test dir that owns it. diff := diffLines("M\tacceptance/cmd/sync/nested/deeper/script") From 0a97bf4518fc6313900db016cc8ec0743b29d4ff Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 13 Aug 2026 15:53:24 +0200 Subject: [PATCH 08/21] acc: match "out" against the test-dir-relative path, not the base name Same rule the harness uses to split inputs from outputs, so a nested fixture such as subdir/outer.py is no longer mistaken for generated output. --- acceptance/selectchanged_test.go | 9 ++++++--- acceptance/selectchanged_unit_test.go | 12 ++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/acceptance/selectchanged_test.go b/acceptance/selectchanged_test.go index 572f72b8aef..8731629cc99 100644 --- a/acceptance/selectchanged_test.go +++ b/acceptance/selectchanged_test.go @@ -167,9 +167,12 @@ func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map continue } result[dir] = nil // nil = all variants; overrides any prior config-scoped filter - // Everything starting with "out" is generated (output.txt, out.requests.txt, - // out.test.toml); the rest is a fixture the test is defined by. - if !strings.HasPrefix(filepath.Base(path), "out") { + // A file is generated output if its path relative to the test dir starts with + // "out" (output.txt, out.requests.txt, out.test.toml) — the same rule the + // harness uses to split inputs from outputs when it copies a test dir. Matching + // on the relative path and not the base name keeps a nested file such as + // subdir/outer.py a fixture. Everything else is a fixture the test is made of. + if !strings.HasPrefix(strings.TrimPrefix(path, "acceptance/"+dir+"/"), "out") { fixture[dir] = true } // The status of a dir's script file says how the dir itself changed: diff --git a/acceptance/selectchanged_unit_test.go b/acceptance/selectchanged_unit_test.go index f457ec7cb6a..4beaf9019f2 100644 --- a/acceptance/selectchanged_unit_test.go +++ b/acceptance/selectchanged_unit_test.go @@ -72,6 +72,18 @@ func TestClassifyChangedTestsFixtureBeatsOutputInSameDir(t *testing.T) { assert.Equal(t, 1, dropped) } +func TestClassifyChangedTestsNestedFixture(t *testing.T) { + // "out" is matched against the path relative to the test dir, so a file in a + // subdirectory is a fixture even when its own name starts with "out". + diff := diffLines( + "M\tacceptance/bundle/modified/subdir/outer.py", + "M\tacceptance/bundle/regenerated/output.txt", + ) + changed, dropped := classifyChangedTests(diff, changedTestDirs, 1) + assert.Equal(t, map[string][]string{"bundle/modified": nil}, changed) + assert.Equal(t, 1, dropped) +} + func TestClassifyChangedTestsInvariantConfigRanksAsFixture(t *testing.T) { // The invariant config is the fixture its dirs are generated from, so it outranks // a dir whose output was regenerated. From b2bf5f735df81f5992f6d8b902e07f806ea030c5 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 13 Aug 2026 17:14:47 +0200 Subject: [PATCH 09/21] acc: simplify classifyChangedTests Replace the four parallel maps with one map of a documented changedDir struct, pull the invariant-config and generated-file rules into named helpers, and rank by sorting once instead of concatenating four slices. --- acceptance/selectchanged_test.go | 194 +++++++++++++++++++------------ 1 file changed, 121 insertions(+), 73 deletions(-) diff --git a/acceptance/selectchanged_test.go b/acceptance/selectchanged_test.go index 8731629cc99..15e1302a562 100644 --- a/acceptance/selectchanged_test.go +++ b/acceptance/selectchanged_test.go @@ -2,6 +2,7 @@ package acceptance_test import ( "errors" + "maps" "os" "os/exec" "path/filepath" @@ -102,28 +103,99 @@ func selectChangedTests(t *testing.T, testDirs map[string]bool, limit int) map[s return changed } -// classifyChangedTests maps `git diff --name-status` output to test dirs, keeping at -// most limit of them, in this order: added dirs, dirs with a changed fixture (script, -// test.toml, databricks.yml, ...), dirs where only generated files changed (out*), and -// finally moved dirs. It also returns how many changed dirs the limit dropped. -// -// Generated files rank below fixtures because a regenerated golden usually comes from a -// change elsewhere in the tree and lands on hundreds of dirs at once, which would -// otherwise fill the whole quota and crowd out the tests this branch actually edits. -// -// 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. +// changedDir records how one test dir changed and which of its variants should run. +type changedDir struct { + // filters restricts the run to the variants matching these KEY=value filters. + // Empty means every variant of the dir runs. + filters []string + + // allVariants is set by a change to the dir itself, as opposed to a change to an + // invariant config the dir is generated from. It clears filters and keeps a later + // config change from narrowing the dir back down to one config. + allVariants bool + + // added is set when the dir's script is new, moved when the script arrived as a + // rename, and fixture when any file the test is made of changed (that is, anything + // but the generated out* files). A dir with no fixture change is one whose golden + // output was regenerated. + added bool + moved bool + fixture bool +} + +// Order the cap selects in. An added test is the most likely to be broken and a moved one +// the least, since its content did not change. A regenerated golden ranks below a changed +// fixture because it usually comes from a change elsewhere in the tree and lands on +// hundreds of dirs at once, which would otherwise fill the quota with tests this branch +// never edited. +const ( + rankAdded = iota + rankFixture + rankGenerated + rankMoved +) + +func (d *changedDir) rank() int { + switch { + case d.added: + return rankAdded + case d.moved: + return rankMoved + case d.fixture: + return rankFixture + default: + return rankGenerated + } +} + +// changedDirs maps a test dir, relative to acceptance/, to how it changed. +type changedDirs map[string]*changedDir + +func (c changedDirs) get(dir string) *changedDir { + if d, ok := c[dir]; ok { + return d + } + d := &changedDir{} + c[dir] = d + return d +} + +// invariantConfigName returns the config a changed file under acceptance/bundle/invariant/ +// configs/ belongs to (job.yml.tmpl for both job.yml.tmpl and job.yml.tmpl-init.sh), or "" +// for any other path. +func invariantConfigName(path string) string { + if !strings.HasPrefix(path, invariantConfigsPrefix) { + return "" + } + name := strings.TrimPrefix(path, invariantConfigsPrefix) + // Strip -init.sh / -cleanup.sh suffixes to get the base config name. + if i := strings.Index(name, "-"); i > 0 && strings.HasSuffix(name, ".sh") { + name = name[:i] + } + if !strings.HasSuffix(name, ".yml.tmpl") { + return "" + } + return name +} + +// isGeneratedFile reports whether path is a file the test generates rather than a fixture +// the test is made of. A file counts as generated when its path relative to the test dir +// starts with "out" (output.txt, out.requests.txt, out.test.toml) — the same rule the +// harness uses to split inputs from outputs when it copies a test dir, so a nested file +// such as subdir/outer.py stays a fixture. +func isGeneratedFile(path, dir string) bool { + return strings.HasPrefix(strings.TrimPrefix(path, "acceptance/"+dir+"/"), "out") +} + +// classifyChangedTests maps `git diff --name-status` output to test dirs and keeps at most +// limit of them, in the order documented on the rank constants. It also returns how many +// changed dirs the limit dropped. func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map[string][]string, int) { - // result accumulates dirs with their filters; added, fixture and moved record how - // each dir changed, so the cap can rank them. - // nil filter slice = all variants run; non-nil = restricted to those filters. - result := map[string][]string{} - added := map[string]bool{} - fixture := map[string]bool{} - moved := map[string]bool{} + dirs := changedDirs{} for line := range strings.SplitSeq(diff, "\n") { + // A rename line carries both paths ("R100\told\tnew"); the last field is the + // path that exists now. fields := strings.Split(line, "\t") if len(fields) < 2 { continue @@ -131,23 +203,18 @@ func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map 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) - } - // The config is the fixture these dirs are generated from. - fixture[dir] = true - } + // A changed invariant config re-enables every invariant subdir, but only for the + // variants generated from that config. + if configName := invariantConfigName(path); configName != "" { + for dir := range testDirs { + if !strings.HasPrefix(dir, invariantDirPrefix) { + continue + } + d := dirs.get(dir) + // The config is the fixture these dirs are generated from. + d.fixture = true + if !d.allVariants { + d.filters = append(d.filters, "INPUT_CONFIG="+configName) } } continue @@ -166,55 +233,36 @@ func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map if dir == "" { continue } - result[dir] = nil // nil = all variants; overrides any prior config-scoped filter - // A file is generated output if its path relative to the test dir starts with - // "out" (output.txt, out.requests.txt, out.test.toml) — the same rule the - // harness uses to split inputs from outputs when it copies a test dir. Matching - // on the relative path and not the base name keeps a nested file such as - // subdir/outer.py a fixture. Everything else is a fixture the test is made of. - if !strings.HasPrefix(strings.TrimPrefix(path, "acceptance/"+dir+"/"), "out") { - fixture[dir] = true + + d := dirs.get(dir) + d.allVariants = true + d.filters = nil + if !isGeneratedFile(path, dir) { + d.fixture = true } - // The status of a dir's script file says how the dir itself changed: - // A means brand new, R (Rnnn) means moved here from another path. + // The status of the dir's script says how the dir itself changed. if strings.HasSuffix(path, "/script") { switch { case status == "A": - added[dir] = true + d.added = true case strings.HasPrefix(status, "R"): - moved[dir] = true + d.moved = true } } } - var addedDirs, fixtureDirs, generatedDirs, movedDirs []string - for dir := range result { - switch { - case added[dir]: - addedDirs = append(addedDirs, dir) - case moved[dir]: - movedDirs = append(movedDirs, dir) - case fixture[dir]: - fixtureDirs = append(fixtureDirs, dir) - default: - generatedDirs = append(generatedDirs, dir) - } - } - slices.Sort(addedDirs) - slices.Sort(fixtureDirs) - slices.Sort(generatedDirs) - slices.Sort(movedDirs) - - selected := slices.Concat(addedDirs, fixtureDirs, generatedDirs, movedDirs) - dropped := 0 - if len(selected) > limit { - dropped = len(selected) - limit - selected = selected[:limit] - } + // Sort by name first, then stably by rank, so dirs of equal rank stay alphabetical. + selected := slices.Sorted(maps.Keys(dirs)) + slices.SortStableFunc(selected, func(a, b string) int { + return dirs[a].rank() - dirs[b].rank() + }) + + dropped := max(len(selected)-limit, 0) + selected = selected[:len(selected)-dropped] out := make(map[string][]string, len(selected)) for _, dir := range selected { - out[dir] = result[dir] + out[dir] = dirs[dir].filters } return out, dropped } From 9cb05b99d04b23aa5dca146f52a3d308856a829c Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 13 Aug 2026 17:36:55 +0200 Subject: [PATCH 10/21] acc: move test selection into acceptance/internal/selection Scores instead of ranks, and the selection is now a real package rather than test-only code, with a command to inspect what a change selects: go run ./acceptance/internal/selection/cmd go run ./acceptance/internal/selection/cmd acceptance/bundle/invariant/configs/job.yml.tmpl --- acceptance/acceptance_test.go | 39 ++-- acceptance/internal/selection/cmd/main.go | 79 +++++++ .../selection/selection.go} | 214 +++++++++++------- .../selection/selection_test.go} | 84 +++---- acceptance/subset_test.go | 6 + 5 files changed, 277 insertions(+), 145 deletions(-) create mode 100644 acceptance/internal/selection/cmd/main.go rename acceptance/{selectchanged_test.go => internal/selection/selection.go} (54%) rename acceptance/{selectchanged_unit_test.go => internal/selection/selection_test.go} (57%) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 2c7626baa03..f3006c0224b 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -30,6 +30,7 @@ import ( "github.com/google/uuid" "github.com/databricks/cli/acceptance/internal" + "github.com/databricks/cli/acceptance/internal/selection" "github.com/databricks/cli/internal/build" "github.com/databricks/cli/internal/testutil" "github.com/databricks/cli/libs/auth" @@ -89,7 +90,7 @@ func init() { } const ( - EntryPointScript = "script" + EntryPointScript = selection.EntryPointScript CleanupScript = "script.cleanup" PrepareScript = "script.prepare" MaxFileSize = 1_000_000 @@ -141,10 +142,10 @@ func TestInprocessMode(t *testing.T) { if os.Getenv("CLOUD_ENV") != "" { t.Skip("No need to run this as integration test.") } - if os.Getenv(SelectChangedEnvVar) != "" { + if os.Getenv(selection.EnvVar) != "" { // The two selftests below only run if this branch changed them, so the // assertions on the returned count do not hold under test selection. - t.Skip("Disabled via " + SelectChangedEnvVar) + t.Skip("Disabled via " + selection.EnvVar) } // Uncomment to load ~/.databricks/debug-env.json to debug integration tests @@ -452,18 +453,24 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { subset := newSubsetSelector(t, testdiff.OverwriteMode, Forcerun) - changedLimit := getSelectChangedLimit(t) + changedLimit, err := selection.ParseLimit(os.Getenv(selection.EnvVar)) + require.NoError(t, err) selectChanged := changedLimit > 0 if !selectChanged && subset.enabled { changedLimit = subsetChangedLimit } // changedTests maps test dir to extra env filters for changed tests; nil filters - // means all variants of that dir changed. Both SelectChangedEnvVar and the subset + // means all variants of that dir changed. Both selection.EnvVar and the subset // selector keep these tests, so detect them at most once here. var changedTests map[string][]string if changedLimit > 0 { - changedTests = selectChangedTests(t, testDirsSet, changedLimit) + // A failed selection (e.g. no origin/main in a shallow checkout) must fail the + // run: treating it as "nothing changed" would silently skip new tests. + result, err := selection.FromGit(testDirsSet, changedLimit) + require.NoError(t, err) + t.Log(result.Summary()) + changedTests = result.Tests } subset.changed = changedTests @@ -585,7 +592,7 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { if runParallel { t.Parallel() } - // Under SelectChangedEnvVar, an invariant dir re-enabled by a + // 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 { @@ -638,23 +645,9 @@ func getEnvFilters(t *testing.T) []string { } func getTests(t *testing.T) []string { - testDirs := make([]string, 0, 128) - - err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - name := filepath.Base(path) - if name == EntryPointScript { - // Presence of 'script' marks a test case in this directory - testName := filepath.ToSlash(filepath.Dir(path)) - testDirs = append(testDirs, testName) - } - return nil - }) + // Tests are discovered relative to the acceptance dir, which is the working directory. + testDirs, err := selection.FindTestDirs(".") require.NoError(t, err) - - slices.Sort(testDirs) return testDirs } diff --git a/acceptance/internal/selection/cmd/main.go b/acceptance/internal/selection/cmd/main.go new file mode 100644 index 00000000000..ae6878ad05b --- /dev/null +++ b/acceptance/internal/selection/cmd/main.go @@ -0,0 +1,79 @@ +// Command cmd prints the acceptance tests that DATABRICKS_TEST_SELECT_CHANGED would run, +// either for the current branch or for the changed paths given as arguments. It is a way +// to see what a change selects without running the suite. Run it from the repo root: +// +// go run ./acceptance/internal/selection/cmd +// go run ./acceptance/internal/selection/cmd -limit 5 +// go run ./acceptance/internal/selection/cmd acceptance/bundle/invariant/configs/job.yml.tmpl +// +// An argument may carry a git status ("A:path" for an added file, "R100:old:new" for a +// rename); without one the file counts as modified. +package main + +import ( + "flag" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/databricks/cli/acceptance/internal/selection" +) + +func main() { + limit := flag.Int("limit", 50, "maximum number of changed tests to select") + root := flag.String("root", "acceptance", "path to the acceptance directory") + flag.Parse() + + dirs, err := selection.FindTestDirs(*root) + if err != nil { + fatalf("cannot list test dirs in %s: %s", *root, err) + } + testDirs := make(map[string]bool, len(dirs)) + for _, dir := range dirs { + testDirs[dir] = true + } + + var result selection.Result + if args := flag.Args(); len(args) > 0 { + result = selection.FromDiff(diffFromArgs(args), testDirs, *limit) + } else { + result, err = selection.FromGit(testDirs, *limit) + if err != nil { + fatalf("%s", err) + } + } + + fmt.Println(result.Counts()) + for _, name := range result.Names() { + fmt.Println(" ", name) + } + if len(result.Tests) == 0 { + fmt.Println(" none of the changed files belong to a test dir") + } +} + +// diffFromArgs renders command line arguments as `git diff --name-status` lines, so the +// same selection runs on them as on a real diff. +func diffFromArgs(args []string) string { + lines := make([]string, 0, len(args)) + for _, arg := range args { + // The status prefix is optional: "A:path", "R100:old:new", or a plain "path". + status, rest, ok := strings.Cut(arg, ":") + if !ok { + status, rest = "M", arg + } + // A rename carries both paths ("R100:old:new"), which git separates by tabs. + paths := strings.Split(rest, ":") + for i, path := range paths { + paths[i] = strings.TrimPrefix(filepath.ToSlash(filepath.Clean(path)), "./") + } + lines = append(lines, status+"\t"+strings.Join(paths, "\t")) + } + return strings.Join(lines, "\n") +} + +func fatalf(format string, args ...any) { + fmt.Fprintf(os.Stderr, format+"\n", args...) + os.Exit(1) +} diff --git a/acceptance/selectchanged_test.go b/acceptance/internal/selection/selection.go similarity index 54% rename from acceptance/selectchanged_test.go rename to acceptance/internal/selection/selection.go index 15e1302a562..d05e8353bc9 100644 --- a/acceptance/selectchanged_test.go +++ b/acceptance/internal/selection/selection.go @@ -1,69 +1,117 @@ -package acceptance_test +// Package selection picks the acceptance tests a branch changed, so a run can cover what a +// PR touches instead of the whole suite. The acceptance harness uses it for +// DATABRICKS_TEST_SELECT_CHANGED; the cmd subpackage exposes the same selection as a +// command, to inspect what a given change would run. +package selection import ( "errors" + "fmt" + "io/fs" "maps" - "os" "os/exec" "path/filepath" "slices" "strconv" "strings" - "testing" ) -// Cloud PR runs set DATABRICKS_TEST_SELECT_CHANGED=N to run only the acceptance -// tests this branch touches, at most N of them, instead of the full suite. const ( - SelectChangedEnvVar = "DATABRICKS_TEST_SELECT_CHANGED" + // EnvVar holds the number of changed tests to select. Unset means no selection. + EnvVar = "DATABRICKS_TEST_SELECT_CHANGED" - // Cap for runs that need change detection without setting the env var: the subset - // selector keeps changed tests on top of its hash-selected fraction, so a PR that - // edits hundreds of test dirs must not turn the subset cells back into a full run. - subsetChangedLimit = 50 + // EntryPointScript is the file whose presence marks a directory as a test case. + EntryPointScript = "script" - invariantConfigsPrefix = "acceptance/bundle/invariant/configs/" + // acceptanceDirPrefix is where the test dirs live relative to the repo root, since + // git reports repo-relative paths while test dirs are named relative to acceptance/. + acceptanceDirPrefix = "acceptance/" + + invariantConfigsPrefix = acceptanceDirPrefix + "bundle/invariant/configs/" invariantDirPrefix = "bundle/invariant/" ) -// getSelectChangedLimit returns the number of changed tests to select, or 0 when -// DATABRICKS_TEST_SELECT_CHANGED is unset (feature off). -func getSelectChangedLimit(t *testing.T) int { - raw := os.Getenv(SelectChangedEnvVar) +// Result is the outcome of a selection. +type Result struct { + // Tests maps a selected test dir to the variant filters it runs with. A nil slice + // means every variant of that dir runs; a non-nil slice restricts the run to the + // variants matching those filters. + Tests map[string][]string + + // Dropped counts the changed tests that did not fit Limit. + Dropped int + + // Limit is the cap this selection was made with. + Limit int +} + +// Counts says how many tests were selected and how many the limit cut. +func (r Result) Counts() string { + return fmt.Sprintf("Selected %d changed tests (limit=%d, %d not selected)", len(r.Tests), r.Limit, r.Dropped) +} + +// Names lists the selected tests in sorted order, each with the variant filters it runs +// with, if any. +func (r Result) Names() []string { + names := make([]string, 0, len(r.Tests)) + for dir, filters := range r.Tests { + if filters != nil { + dir += "[" + strings.Join(filters, ",") + "]" + } + names = append(names, dir) + } + slices.Sort(names) + return names +} + +// Summary is the whole outcome on one line, for the test log. +func (r Result) Summary() string { + return r.Counts() + ": " + strings.Join(r.Names(), " ") +} + +// ParseLimit reads the number of tests to select from a raw EnvVar value. An empty value +// yields 0, which means no selection. +func ParseLimit(raw string) (int, error) { if raw == "" { - return 0 + return 0, nil } limit, err := strconv.Atoi(raw) if err != nil || limit <= 0 { - t.Fatalf("Invalid %s=%q, expected a positive integer", SelectChangedEnvVar, raw) + return 0, fmt.Errorf("invalid %s=%q, expected a positive integer", EnvVar, raw) } - return limit + return limit, nil } -// 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 +// FindTestDirs returns every test dir under root, named relative to root with forward +// slashes, sorted. +func FindTestDirs(root string) ([]string, error) { + var dirs []string + + err := filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() || entry.Name() != EntryPointScript { + return nil + } + rel, err := filepath.Rel(root, filepath.Dir(path)) + if err != nil { + return err } + dirs = append(dirs, filepath.ToSlash(rel)) + return nil + }) + if err != nil { + return nil, err } - return "" + + slices.Sort(dirs) + return dirs, nil } -// selectChangedTests returns a map of test dir → extra env filters for the tests this -// branch changed. 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). +// FromGit selects among testDirs the tests this branch changed, at most limit of them. // // --merge-base diffs the working tree against the merge base of HEAD and // origin/main. This covers committed, staged, and unstaged changes alike — @@ -72,35 +120,40 @@ func testDirForFile(repoRelPath string, testDirs map[string]bool) string { // 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). -func selectChangedTests(t *testing.T, testDirs map[string]bool, limit int) map[string][]string { +func FromGit(testDirs map[string]bool, limit int) (Result, error) { out, err := exec.Command("git", "diff", "--name-status", "--merge-base", "-M", "origin/main").Output() if err != nil { - // A failed diff (most commonly a missing origin/main in a shallow CI - // checkout) must not be silently treated as "nothing changed": that - // disables change detection and lets newly added tests skip. Fail loudly. - // Every caller (push.yml PR cells, integration runs) now fetches origin/main. - stderr := "" + // A failed diff (most commonly a missing origin/main in a shallow CI checkout) + // must not be silently treated as "nothing changed": that disables change + // detection and lets newly added tests skip. Every caller (push.yml PR cells, + // integration runs) fetches origin/main, so the caller should fail loudly. if exitErr, ok := errors.AsType[*exec.ExitError](err); ok { - stderr = strings.TrimSpace(string(exitErr.Stderr)) + if stderr := strings.TrimSpace(string(exitErr.Stderr)); stderr != "" { + return Result{}, fmt.Errorf("git diff --merge-base origin/main failed: %w: %s", err, stderr) + } } - t.Fatalf("git diff --merge-base origin/main failed: %v\n%s", err, stderr) + return Result{}, fmt.Errorf("git diff --merge-base origin/main failed: %w", err) } - changed, dropped := classifyChangedTests(strings.TrimSpace(string(out)), testDirs, limit) + return FromDiff(strings.TrimSpace(string(out)), testDirs, limit), nil +} - // Log the outcome up front: which tests the diff picked, and how many the limit - // cut, so a CI run shows what it is about to cover without reading every skip line. - names := make([]string, 0, len(changed)) - for dir, filters := range changed { - if filters != nil { - dir += "[" + strings.Join(filters, ",") + "]" +// 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]+"/" != acceptanceDirPrefix { + 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 } - names = append(names, dir) } - slices.Sort(names) - t.Logf("Selected %d changed tests (limit=%d, %d not selected): %s", len(names), limit, dropped, strings.Join(names, " ")) - - return changed + return "" } // changedDir records how one test dir changed and which of its variants should run. @@ -123,28 +176,27 @@ type changedDir struct { fixture bool } -// Order the cap selects in. An added test is the most likely to be broken and a moved one -// the least, since its content did not change. A regenerated golden ranks below a changed -// fixture because it usually comes from a change elsewhere in the tree and lands on -// hundreds of dirs at once, which would otherwise fill the quota with tests this branch -// never edited. +// The cap takes the highest scoring dirs. An added test is the most likely to be broken; a +// moved one only changed location. A dir where nothing but the golden output changed scores +// negative: that usually follows a change elsewhere in the tree and lands on hundreds of +// dirs at once, which would otherwise fill the quota with tests this branch never edited. const ( - rankAdded = iota - rankFixture - rankGenerated - rankMoved + scoreAdd = 10 + scoreChange = 5 + scoreMoved = 2 + scoreGenerated = -1 ) -func (d *changedDir) rank() int { +func (d *changedDir) score() int { switch { case d.added: - return rankAdded + return scoreAdd case d.moved: - return rankMoved + return scoreMoved case d.fixture: - return rankFixture + return scoreChange default: - return rankGenerated + return scoreGenerated } } @@ -184,13 +236,12 @@ func invariantConfigName(path string) string { // harness uses to split inputs from outputs when it copies a test dir, so a nested file // such as subdir/outer.py stays a fixture. func isGeneratedFile(path, dir string) bool { - return strings.HasPrefix(strings.TrimPrefix(path, "acceptance/"+dir+"/"), "out") + return strings.HasPrefix(strings.TrimPrefix(path, acceptanceDirPrefix+dir+"/"), "out") } -// classifyChangedTests maps `git diff --name-status` output to test dirs and keeps at most -// limit of them, in the order documented on the rank constants. It also returns how many -// changed dirs the limit dropped. -func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map[string][]string, int) { +// FromDiff selects among testDirs the tests touched by `git diff --name-status` output, +// keeping at most limit of them in the order documented on the score constants. +func FromDiff(diff string, testDirs map[string]bool, limit int) Result { dirs := changedDirs{} for line := range strings.SplitSeq(diff, "\n") { @@ -223,7 +274,7 @@ func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map // test.toml and out.test.toml under the invariant tree regenerate // automatically when INPUT_CONFIG changes; ignore them so they don't // unlock all variants of every invariant subdir. - if strings.HasPrefix(path, "acceptance/"+invariantDirPrefix) { + if strings.HasPrefix(path, acceptanceDirPrefix+invariantDirPrefix) { if name := filepath.Base(path); name == "test.toml" || name == "out.test.toml" { continue } @@ -251,18 +302,19 @@ func classifyChangedTests(diff string, testDirs map[string]bool, limit int) (map } } - // Sort by name first, then stably by rank, so dirs of equal rank stay alphabetical. + // Sort by name first, then stably by descending score, so dirs that score the same + // stay alphabetical. selected := slices.Sorted(maps.Keys(dirs)) slices.SortStableFunc(selected, func(a, b string) int { - return dirs[a].rank() - dirs[b].rank() + return dirs[b].score() - dirs[a].score() }) dropped := max(len(selected)-limit, 0) selected = selected[:len(selected)-dropped] - out := make(map[string][]string, len(selected)) + tests := make(map[string][]string, len(selected)) for _, dir := range selected { - out[dir] = dirs[dir].filters + tests[dir] = dirs[dir].filters } - return out, dropped + return Result{Tests: tests, Dropped: dropped, Limit: limit} } diff --git a/acceptance/selectchanged_unit_test.go b/acceptance/internal/selection/selection_test.go similarity index 57% rename from acceptance/selectchanged_unit_test.go rename to acceptance/internal/selection/selection_test.go index 4beaf9019f2..547b7cbdedb 100644 --- a/acceptance/selectchanged_unit_test.go +++ b/acceptance/internal/selection/selection_test.go @@ -1,13 +1,15 @@ -package acceptance_test +package selection_test import ( "strings" "testing" "github.com/stretchr/testify/assert" + + "github.com/databricks/cli/acceptance/internal/selection" ) -var changedTestDirs = map[string]bool{ +var testDirs = map[string]bool{ "bundle/added": true, "bundle/modified": true, "bundle/regenerated": true, @@ -24,96 +26,96 @@ func diffLines(lines ...string) string { return strings.Join(lines, "\n") } -func TestClassifyChangedTestsStatuses(t *testing.T) { +func TestFromDiffStatuses(t *testing.T) { diff := diffLines( "A\tacceptance/bundle/added/script", "M\tacceptance/bundle/modified/databricks.yml", "R100\tacceptance/bundle/old/script\tacceptance/bundle/moved/script", "M\tlibs/dyn/value.go", ) - changed, dropped := classifyChangedTests(diff, changedTestDirs, 10) - assert.Zero(t, dropped) + result := selection.FromDiff(diff, testDirs, 10) + assert.Zero(t, result.Dropped) assert.Equal(t, map[string][]string{ "bundle/added": nil, "bundle/modified": nil, "bundle/moved": nil, - }, changed) + }, result.Tests) } -func TestClassifyChangedTestsPriority(t *testing.T) { +func TestFromDiffPriority(t *testing.T) { diff := diffLines( "R090\tacceptance/bundle/old/script\tacceptance/bundle/moved/script", "M\tacceptance/bundle/regenerated/output.txt", "M\tacceptance/bundle/modified/script", "A\tacceptance/bundle/added/script", ) - // The cap keeps added first, then a changed fixture, then a regenerated output, - // then a moved dir. - ranked := []string{"bundle/added", "bundle/modified", "bundle/regenerated", "bundle/moved"} - for limit := 1; limit <= len(ranked); limit++ { - changed, dropped := classifyChangedTests(diff, changedTestDirs, limit) - assert.Len(t, changed, limit) - assert.Equal(t, len(ranked)-limit, dropped, "limit=%d", limit) - for _, dir := range ranked[:limit] { - assert.Contains(t, changed, dir, "limit=%d", limit) + // The cap keeps added first, then a changed fixture, then a moved dir, and last a dir + // where only the golden output was regenerated. + scored := []string{"bundle/added", "bundle/modified", "bundle/moved", "bundle/regenerated"} + for limit := 1; limit <= len(scored); limit++ { + result := selection.FromDiff(diff, testDirs, limit) + assert.Len(t, result.Tests, limit) + assert.Equal(t, len(scored)-limit, result.Dropped, "limit=%d", limit) + for _, dir := range scored[:limit] { + assert.Contains(t, result.Tests, dir, "limit=%d", limit) } } } -func TestClassifyChangedTestsFixtureBeatsOutputInSameDir(t *testing.T) { +func TestFromDiffFixtureBeatsOutputInSameDir(t *testing.T) { // A dir with both a fixture and an output change ranks as a fixture change. diff := diffLines( "M\tacceptance/bundle/modified/output.txt", "M\tacceptance/bundle/modified/databricks.yml", "M\tacceptance/bundle/regenerated/out.requests.txt", ) - changed, dropped := classifyChangedTests(diff, changedTestDirs, 1) - assert.Equal(t, map[string][]string{"bundle/modified": nil}, changed) - assert.Equal(t, 1, dropped) + result := selection.FromDiff(diff, testDirs, 1) + assert.Equal(t, map[string][]string{"bundle/modified": nil}, result.Tests) + assert.Equal(t, 1, result.Dropped) } -func TestClassifyChangedTestsNestedFixture(t *testing.T) { +func TestFromDiffNestedFixture(t *testing.T) { // "out" is matched against the path relative to the test dir, so a file in a // subdirectory is a fixture even when its own name starts with "out". diff := diffLines( "M\tacceptance/bundle/modified/subdir/outer.py", "M\tacceptance/bundle/regenerated/output.txt", ) - changed, dropped := classifyChangedTests(diff, changedTestDirs, 1) - assert.Equal(t, map[string][]string{"bundle/modified": nil}, changed) - assert.Equal(t, 1, dropped) + result := selection.FromDiff(diff, testDirs, 1) + assert.Equal(t, map[string][]string{"bundle/modified": nil}, result.Tests) + assert.Equal(t, 1, result.Dropped) } -func TestClassifyChangedTestsInvariantConfigRanksAsFixture(t *testing.T) { +func TestFromDiffInvariantConfigRanksAsFixture(t *testing.T) { // The invariant config is the fixture its dirs are generated from, so it outranks // a dir whose output was regenerated. diff := diffLines( "M\tacceptance/bundle/regenerated/output.txt", "M\tacceptance/bundle/invariant/configs/job.yml.tmpl", ) - changed, dropped := classifyChangedTests(diff, changedTestDirs, 2) - assert.NotContains(t, changed, "bundle/regenerated") - assert.Equal(t, 1, dropped) + result := selection.FromDiff(diff, testDirs, 2) + assert.NotContains(t, result.Tests, "bundle/regenerated") + assert.Equal(t, 1, result.Dropped) } -func TestClassifyChangedTestsNestedDir(t *testing.T) { +func TestFromDiffNestedDir(t *testing.T) { // A file maps to the innermost test dir that owns it. diff := diffLines("M\tacceptance/cmd/sync/nested/deeper/script") - changed, _ := classifyChangedTests(diff, changedTestDirs, 10) - assert.Equal(t, map[string][]string{"cmd/sync/nested/deeper": nil}, changed) + result := selection.FromDiff(diff, testDirs, 10) + assert.Equal(t, map[string][]string{"cmd/sync/nested/deeper": nil}, result.Tests) } -func TestClassifyChangedTestsInvariantConfig(t *testing.T) { +func TestFromDiffInvariantConfig(t *testing.T) { // A changed invariant config re-enables every invariant dir, restricted to that config. diff := diffLines("M\tacceptance/bundle/invariant/configs/job.yml.tmpl") - changed, _ := classifyChangedTests(diff, changedTestDirs, 10) + result := selection.FromDiff(diff, testDirs, 10) assert.Equal(t, map[string][]string{ "bundle/invariant/jobs": {"INPUT_CONFIG=job.yml.tmpl"}, "bundle/invariant/apps": {"INPUT_CONFIG=job.yml.tmpl"}, - }, changed) + }, result.Tests) } -func TestClassifyChangedTestsInvariantConfigAndDir(t *testing.T) { +func TestFromDiffInvariantConfigAndDir(t *testing.T) { // A non-config change to an invariant dir unlocks all of its variants; the // regenerated test.toml files are ignored. diff := diffLines( @@ -121,15 +123,15 @@ func TestClassifyChangedTestsInvariantConfigAndDir(t *testing.T) { "M\tacceptance/bundle/invariant/jobs/script", "M\tacceptance/bundle/invariant/apps/out.test.toml", ) - changed, _ := classifyChangedTests(diff, changedTestDirs, 10) + result := selection.FromDiff(diff, testDirs, 10) assert.Equal(t, map[string][]string{ "bundle/invariant/jobs": nil, "bundle/invariant/apps": {"INPUT_CONFIG=job.yml.tmpl"}, - }, changed) + }, result.Tests) } -func TestClassifyChangedTestsEmptyDiff(t *testing.T) { - changed, dropped := classifyChangedTests("", changedTestDirs, 10) - assert.Empty(t, changed) - assert.Zero(t, dropped) +func TestFromDiffEmptyDiff(t *testing.T) { + result := selection.FromDiff("", testDirs, 10) + assert.Empty(t, result.Tests) + assert.Zero(t, result.Dropped) } diff --git a/acceptance/subset_test.go b/acceptance/subset_test.go index 435f3bf385e..77ab97c9b3d 100644 --- a/acceptance/subset_test.go +++ b/acceptance/subset_test.go @@ -23,6 +23,12 @@ const ( // a new commit reshuffles the subset while a retry of the same commit repeats it. // If unset, a random seed is generated and logged so the run can be reproduced. SubsetSeedEnvVar = "TESTS_SELECT_SUBSET_SEED" + + // subsetChangedLimit caps how many changed tests the subset selector keeps on top of + // its hash-selected fraction. Unlike selection.EnvVar it carries no count of its own, + // and a PR that edits hundreds of test dirs must not turn the subset cells back into + // a full run. + subsetChangedLimit = 50 ) // subsetSelector decides, per subtest, whether it runs under TESTS_SELECT_SUBSET_PCT. From 20a09ef6ca28fdf89ffb0972039e777b19287d48 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 13 Aug 2026 18:52:36 +0200 Subject: [PATCH 11/21] acc: infer status of bare paths in the selection command, print scores A path given without a status prefix now takes the status git reports for it: added when git does not track it, modified otherwise. Each selected test is printed with the score it was picked by. --- acceptance/acceptance_test.go | 2 +- acceptance/internal/selection/cmd/main.go | 42 +++++++++--- acceptance/internal/selection/selection.go | 66 ++++++++++++------- .../internal/selection/selection_test.go | 40 ++++++++--- 4 files changed, 106 insertions(+), 44 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index f3006c0224b..12764d90150 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -470,7 +470,7 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { result, err := selection.FromGit(testDirsSet, changedLimit) require.NoError(t, err) t.Log(result.Summary()) - changedTests = result.Tests + changedTests = result.Tests() } subset.changed = changedTests diff --git a/acceptance/internal/selection/cmd/main.go b/acceptance/internal/selection/cmd/main.go index ae6878ad05b..370418e6839 100644 --- a/acceptance/internal/selection/cmd/main.go +++ b/acceptance/internal/selection/cmd/main.go @@ -1,19 +1,21 @@ // Command cmd prints the acceptance tests that DATABRICKS_TEST_SELECT_CHANGED would run, -// either for the current branch or for the changed paths given as arguments. It is a way -// to see what a change selects without running the suite. Run it from the repo root: +// either for the current branch or for the paths given as arguments, together with the +// score each test was picked by. It is a way to see what a change selects without running +// the suite. Run it from the repo root: // // go run ./acceptance/internal/selection/cmd // go run ./acceptance/internal/selection/cmd -limit 5 // go run ./acceptance/internal/selection/cmd acceptance/bundle/invariant/configs/job.yml.tmpl // -// An argument may carry a git status ("A:path" for an added file, "R100:old:new" for a -// rename); without one the file counts as modified. +// A bare path takes the status git reports for it: added for a path git does not track, +// modified otherwise. Prefix a path to force one ("A:path", "M:path", "R100:old:new"). package main import ( "flag" "fmt" "os" + "os/exec" "path/filepath" "strings" @@ -45,11 +47,11 @@ func main() { } fmt.Println(result.Counts()) - for _, name := range result.Names() { - fmt.Println(" ", name) + for _, test := range result.Selected { + fmt.Printf(" %3d %s\n", test.Score, test.Name()) } - if len(result.Tests) == 0 { - fmt.Println(" none of the changed files belong to a test dir") + if len(result.Selected) == 0 { + fmt.Println(" none of the changed files belong to a test dir") } } @@ -58,21 +60,41 @@ func main() { func diffFromArgs(args []string) string { lines := make([]string, 0, len(args)) for _, arg := range args { - // The status prefix is optional: "A:path", "R100:old:new", or a plain "path". status, rest, ok := strings.Cut(arg, ":") if !ok { - status, rest = "M", arg + status, rest = "", arg } // A rename carries both paths ("R100:old:new"), which git separates by tabs. paths := strings.Split(rest, ":") for i, path := range paths { paths[i] = strings.TrimPrefix(filepath.ToSlash(filepath.Clean(path)), "./") } + if status == "" { + status = gitStatus(paths[0]) + } lines = append(lines, status+"\t"+strings.Join(paths, "\t")) } return strings.Join(lines, "\n") } +// gitStatus is the status to assume for a path given without one: the status git reports +// against the merge base if the path is changed there, otherwise "M" for a tracked path +// and "A" for one git does not know, so a bare argument also stands for a hypothetical +// change to an existing test or a brand new one. +func gitStatus(path string) string { + out, err := exec.Command("git", "diff", "--name-status", "--merge-base", "-M", "origin/main", "--", path).Output() + if err == nil { + if fields := strings.Fields(string(out)); len(fields) > 0 { + return fields[0] + } + } + + if exec.Command("git", "ls-files", "--error-unmatch", "--", path).Run() == nil { + return "M" + } + return "A" +} + func fatalf(format string, args ...any) { fmt.Fprintf(os.Stderr, format+"\n", args...) os.Exit(1) diff --git a/acceptance/internal/selection/selection.go b/acceptance/internal/selection/selection.go index d05e8353bc9..7c380e55a14 100644 --- a/acceptance/internal/selection/selection.go +++ b/acceptance/internal/selection/selection.go @@ -31,12 +31,32 @@ const ( invariantDirPrefix = "bundle/invariant/" ) +// Test is one test the selection picked. +type Test struct { + // Dir is the test dir, relative to acceptance/. + Dir string + + // Filters restricts the run to the variants matching these KEY=value filters. A nil + // slice means every variant of the dir runs. + Filters []string + + // Score is why the test was picked; see the score constants. + Score int +} + +// Name is the test dir with its variant filters, as the log and the command print it. +func (t Test) Name() string { + if t.Filters == nil { + return t.Dir + } + return t.Dir + "[" + strings.Join(t.Filters, ",") + "]" +} + // Result is the outcome of a selection. type Result struct { - // Tests maps a selected test dir to the variant filters it runs with. A nil slice - // means every variant of that dir runs; a non-nil slice restricts the run to the - // variants matching those filters. - Tests map[string][]string + // Selected lists the picked tests, highest score first and alphabetical within a + // score. + Selected []Test // Dropped counts the changed tests that did not fit Limit. Dropped int @@ -45,28 +65,28 @@ type Result struct { Limit int } -// Counts says how many tests were selected and how many the limit cut. -func (r Result) Counts() string { - return fmt.Sprintf("Selected %d changed tests (limit=%d, %d not selected)", len(r.Tests), r.Limit, r.Dropped) +// Tests maps each selected test dir to the variant filters it runs with, the form the +// acceptance harness looks tests up by. +func (r Result) Tests() map[string][]string { + tests := make(map[string][]string, len(r.Selected)) + for _, test := range r.Selected { + tests[test.Dir] = test.Filters + } + return tests } -// Names lists the selected tests in sorted order, each with the variant filters it runs -// with, if any. -func (r Result) Names() []string { - names := make([]string, 0, len(r.Tests)) - for dir, filters := range r.Tests { - if filters != nil { - dir += "[" + strings.Join(filters, ",") + "]" - } - names = append(names, dir) - } - slices.Sort(names) - return names +// Counts says how many tests were selected and how many the limit cut. +func (r Result) Counts() string { + return fmt.Sprintf("Selected %d changed tests (limit=%d, %d not selected)", len(r.Selected), r.Limit, r.Dropped) } // Summary is the whole outcome on one line, for the test log. func (r Result) Summary() string { - return r.Counts() + ": " + strings.Join(r.Names(), " ") + names := make([]string, 0, len(r.Selected)) + for _, test := range r.Selected { + names = append(names, test.Name()) + } + return r.Counts() + ": " + strings.Join(names, " ") } // ParseLimit reads the number of tests to select from a raw EnvVar value. An empty value @@ -312,9 +332,9 @@ func FromDiff(diff string, testDirs map[string]bool, limit int) Result { dropped := max(len(selected)-limit, 0) selected = selected[:len(selected)-dropped] - tests := make(map[string][]string, len(selected)) + tests := make([]Test, 0, len(selected)) for _, dir := range selected { - tests[dir] = dirs[dir].filters + tests = append(tests, Test{Dir: dir, Filters: dirs[dir].filters, Score: dirs[dir].score()}) } - return Result{Tests: tests, Dropped: dropped, Limit: limit} + return Result{Selected: tests, Dropped: dropped, Limit: limit} } diff --git a/acceptance/internal/selection/selection_test.go b/acceptance/internal/selection/selection_test.go index 547b7cbdedb..0da94ea6011 100644 --- a/acceptance/internal/selection/selection_test.go +++ b/acceptance/internal/selection/selection_test.go @@ -39,7 +39,7 @@ func TestFromDiffStatuses(t *testing.T) { "bundle/added": nil, "bundle/modified": nil, "bundle/moved": nil, - }, result.Tests) + }, result.Tests()) } func TestFromDiffPriority(t *testing.T) { @@ -54,14 +54,34 @@ func TestFromDiffPriority(t *testing.T) { scored := []string{"bundle/added", "bundle/modified", "bundle/moved", "bundle/regenerated"} for limit := 1; limit <= len(scored); limit++ { result := selection.FromDiff(diff, testDirs, limit) - assert.Len(t, result.Tests, limit) + assert.Len(t, result.Selected, limit) assert.Equal(t, len(scored)-limit, result.Dropped, "limit=%d", limit) for _, dir := range scored[:limit] { - assert.Contains(t, result.Tests, dir, "limit=%d", limit) + assert.Contains(t, result.Tests(), dir, "limit=%d", limit) } } } +func TestFromDiffScores(t *testing.T) { + diff := diffLines( + "A\tacceptance/bundle/added/script", + "M\tacceptance/bundle/modified/script", + "R090\tacceptance/bundle/old/script\tacceptance/bundle/moved/script", + "M\tacceptance/bundle/regenerated/output.txt", + ) + result := selection.FromDiff(diff, testDirs, 10) + scores := map[string]int{} + for _, test := range result.Selected { + scores[test.Dir] = test.Score + } + assert.Equal(t, map[string]int{ + "bundle/added": 10, + "bundle/modified": 5, + "bundle/moved": 2, + "bundle/regenerated": -1, + }, scores) +} + func TestFromDiffFixtureBeatsOutputInSameDir(t *testing.T) { // A dir with both a fixture and an output change ranks as a fixture change. diff := diffLines( @@ -70,7 +90,7 @@ func TestFromDiffFixtureBeatsOutputInSameDir(t *testing.T) { "M\tacceptance/bundle/regenerated/out.requests.txt", ) result := selection.FromDiff(diff, testDirs, 1) - assert.Equal(t, map[string][]string{"bundle/modified": nil}, result.Tests) + assert.Equal(t, map[string][]string{"bundle/modified": nil}, result.Tests()) assert.Equal(t, 1, result.Dropped) } @@ -82,7 +102,7 @@ func TestFromDiffNestedFixture(t *testing.T) { "M\tacceptance/bundle/regenerated/output.txt", ) result := selection.FromDiff(diff, testDirs, 1) - assert.Equal(t, map[string][]string{"bundle/modified": nil}, result.Tests) + assert.Equal(t, map[string][]string{"bundle/modified": nil}, result.Tests()) assert.Equal(t, 1, result.Dropped) } @@ -94,7 +114,7 @@ func TestFromDiffInvariantConfigRanksAsFixture(t *testing.T) { "M\tacceptance/bundle/invariant/configs/job.yml.tmpl", ) result := selection.FromDiff(diff, testDirs, 2) - assert.NotContains(t, result.Tests, "bundle/regenerated") + assert.NotContains(t, result.Tests(), "bundle/regenerated") assert.Equal(t, 1, result.Dropped) } @@ -102,7 +122,7 @@ func TestFromDiffNestedDir(t *testing.T) { // A file maps to the innermost test dir that owns it. diff := diffLines("M\tacceptance/cmd/sync/nested/deeper/script") result := selection.FromDiff(diff, testDirs, 10) - assert.Equal(t, map[string][]string{"cmd/sync/nested/deeper": nil}, result.Tests) + assert.Equal(t, map[string][]string{"cmd/sync/nested/deeper": nil}, result.Tests()) } func TestFromDiffInvariantConfig(t *testing.T) { @@ -112,7 +132,7 @@ func TestFromDiffInvariantConfig(t *testing.T) { assert.Equal(t, map[string][]string{ "bundle/invariant/jobs": {"INPUT_CONFIG=job.yml.tmpl"}, "bundle/invariant/apps": {"INPUT_CONFIG=job.yml.tmpl"}, - }, result.Tests) + }, result.Tests()) } func TestFromDiffInvariantConfigAndDir(t *testing.T) { @@ -127,11 +147,11 @@ func TestFromDiffInvariantConfigAndDir(t *testing.T) { assert.Equal(t, map[string][]string{ "bundle/invariant/jobs": nil, "bundle/invariant/apps": {"INPUT_CONFIG=job.yml.tmpl"}, - }, result.Tests) + }, result.Tests()) } func TestFromDiffEmptyDiff(t *testing.T) { result := selection.FromDiff("", testDirs, 10) - assert.Empty(t, result.Tests) + assert.Empty(t, result.Tests()) assert.Zero(t, result.Dropped) } From ae707e0836fc341a63342fdbe167377662a1f1a5 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 13 Aug 2026 19:15:11 +0200 Subject: [PATCH 12/21] acc: make selection scores additive A dir that changed in several ways now outranks one that changed in a single way: new dir 5, changed fixtures 5, changed goldens 1, move 1. New and moved stay exclusive, and a move does not also count the renames of the files it brings along. --- acceptance/internal/selection/selection.go | 61 +++++++++++-------- .../internal/selection/selection_test.go | 16 +++-- 2 files changed, 49 insertions(+), 28 deletions(-) diff --git a/acceptance/internal/selection/selection.go b/acceptance/internal/selection/selection.go index 7c380e55a14..2273a521ccc 100644 --- a/acceptance/internal/selection/selection.go +++ b/acceptance/internal/selection/selection.go @@ -187,37 +187,48 @@ type changedDir struct { // config change from narrowing the dir back down to one config. allVariants bool - // added is set when the dir's script is new, moved when the script arrived as a - // rename, and fixture when any file the test is made of changed (that is, anything - // but the generated out* files). A dir with no fixture change is one whose golden + // newDir is set when the dir's script is new, so the whole test is new, and moved when + // the script arrived as a rename, so the test only changed location. The two are + // exclusive: a script is either added or renamed. + newDir bool + moved bool + + // fixture is set when a file the test is made of changed, generated when a file the + // test produces changed (out*). A dir with only generated changes is one whose golden // output was regenerated. - added bool - moved bool - fixture bool + fixture bool + generated bool } -// The cap takes the highest scoring dirs. An added test is the most likely to be broken; a -// moved one only changed location. A dir where nothing but the golden output changed scores -// negative: that usually follows a change elsewhere in the tree and lands on hundreds of -// dirs at once, which would otherwise fill the quota with tests this branch never edited. +// The cap takes the highest scoring dirs, and the scores add up, so a dir that changed in +// several ways outranks one that changed in a single way. A new test is the most likely to +// be broken, while a dir where nothing but the golden output changed scores lowest: that +// usually follows a change elsewhere in the tree and lands on hundreds of dirs at once, +// which would otherwise fill the quota with tests this branch never edited. const ( - scoreAdd = 10 + scoreNewDir = 5 scoreChange = 5 - scoreMoved = 2 - scoreGenerated = -1 + scoreGenerated = 1 + scoreMoved = 1 ) func (d *changedDir) score() int { - switch { - case d.added: - return scoreAdd - case d.moved: - return scoreMoved - case d.fixture: - return scoreChange - default: - return scoreGenerated + score := 0 + if d.newDir { + score += scoreNewDir } + if d.moved { + // The files of a moved dir all arrive as renames. Moving a test does not change + // what it does, so those renames do not also count as changes. + return score + scoreMoved + } + if d.fixture { + score += scoreChange + } + if d.generated { + score += scoreGenerated + } + return score } // changedDirs maps a test dir, relative to acceptance/, to how it changed. @@ -308,14 +319,16 @@ func FromDiff(diff string, testDirs map[string]bool, limit int) Result { d := dirs.get(dir) d.allVariants = true d.filters = nil - if !isGeneratedFile(path, dir) { + if isGeneratedFile(path, dir) { + d.generated = true + } else { d.fixture = true } // The status of the dir's script says how the dir itself changed. if strings.HasSuffix(path, "/script") { switch { case status == "A": - d.added = true + d.newDir = true case strings.HasPrefix(status, "R"): d.moved = true } diff --git a/acceptance/internal/selection/selection_test.go b/acceptance/internal/selection/selection_test.go index 0da94ea6011..3f3893e3f22 100644 --- a/acceptance/internal/selection/selection_test.go +++ b/acceptance/internal/selection/selection_test.go @@ -63,11 +63,18 @@ func TestFromDiffPriority(t *testing.T) { } func TestFromDiffScores(t *testing.T) { + // Scores add up: a new dir counts as new (5) plus its fixtures (5) plus its goldens + // (1); a dir whose script and golden both changed counts 5+1. A moved dir scores the + // move alone, since the files it brings along are renames rather than changes. diff := diffLines( "A\tacceptance/bundle/added/script", + "A\tacceptance/bundle/added/output.txt", "M\tacceptance/bundle/modified/script", + "M\tacceptance/bundle/modified/output.txt", "R090\tacceptance/bundle/old/script\tacceptance/bundle/moved/script", + "R090\tacceptance/bundle/old/output.txt\tacceptance/bundle/moved/output.txt", "M\tacceptance/bundle/regenerated/output.txt", + "M\tacceptance/bundle/untouched/databricks.yml", ) result := selection.FromDiff(diff, testDirs, 10) scores := map[string]int{} @@ -75,10 +82,11 @@ func TestFromDiffScores(t *testing.T) { scores[test.Dir] = test.Score } assert.Equal(t, map[string]int{ - "bundle/added": 10, - "bundle/modified": 5, - "bundle/moved": 2, - "bundle/regenerated": -1, + "bundle/added": 11, + "bundle/modified": 6, + "bundle/untouched": 5, + "bundle/moved": 1, + "bundle/regenerated": 1, }, scores) } From 657b27f412a1d929d673e13d7328d813cc98745b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 14 Aug 2026 10:54:33 +0200 Subject: [PATCH 13/21] acc: score a new invariant config as a new test An added config adds a variant of every invariant dir, so it now scores 10 where changing an existing config scores 5. The companion setup scripts of a config only change how an existing variant runs, so they stay at 5. --- acceptance/internal/selection/selection.go | 27 ++++++++++++------- .../internal/selection/selection_test.go | 13 +++++++++ 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/acceptance/internal/selection/selection.go b/acceptance/internal/selection/selection.go index 2273a521ccc..560f668641c 100644 --- a/acceptance/internal/selection/selection.go +++ b/acceptance/internal/selection/selection.go @@ -187,11 +187,12 @@ type changedDir struct { // config change from narrowing the dir back down to one config. allVariants bool - // newDir is set when the dir's script is new, so the whole test is new, and moved when - // the script arrived as a rename, so the test only changed location. The two are - // exclusive: a script is either added or renamed. - newDir bool - moved bool + // newTest is set when the test itself is new: the dir's script is new, or a new + // invariant config adds a variant of the dir. moved is set when the script arrived as + // a rename, so the test only changed location. The two are exclusive: a script is + // either added or renamed. + newTest bool + moved bool // fixture is set when a file the test is made of changed, generated when a file the // test produces changed (out*). A dir with only generated changes is one whose golden @@ -206,7 +207,7 @@ type changedDir struct { // usually follows a change elsewhere in the tree and lands on hundreds of dirs at once, // which would otherwise fill the quota with tests this branch never edited. const ( - scoreNewDir = 5 + scoreNewTest = 5 scoreChange = 5 scoreGenerated = 1 scoreMoved = 1 @@ -214,8 +215,8 @@ const ( func (d *changedDir) score() int { score := 0 - if d.newDir { - score += scoreNewDir + if d.newTest { + score += scoreNewTest } if d.moved { // The files of a moved dir all arrive as renames. Moving a test does not change @@ -293,8 +294,14 @@ func FromDiff(diff string, testDirs map[string]bool, limit int) Result { continue } d := dirs.get(dir) - // The config is the fixture these dirs are generated from. + // The config is the fixture these dirs are generated from, and a new + // config adds a variant of each of them. Its -init.sh / -cleanup.sh + // companions change how an existing variant runs, so only the config + // itself counts as a new test. d.fixture = true + if status == "A" && strings.HasSuffix(path, configName) { + d.newTest = true + } if !d.allVariants { d.filters = append(d.filters, "INPUT_CONFIG="+configName) } @@ -328,7 +335,7 @@ func FromDiff(diff string, testDirs map[string]bool, limit int) Result { if strings.HasSuffix(path, "/script") { switch { case status == "A": - d.newDir = true + d.newTest = true case strings.HasPrefix(status, "R"): d.moved = true } diff --git a/acceptance/internal/selection/selection_test.go b/acceptance/internal/selection/selection_test.go index 3f3893e3f22..3bc899d425a 100644 --- a/acceptance/internal/selection/selection_test.go +++ b/acceptance/internal/selection/selection_test.go @@ -126,6 +126,19 @@ func TestFromDiffInvariantConfigRanksAsFixture(t *testing.T) { assert.Equal(t, 1, result.Dropped) } +func TestFromDiffNewInvariantConfig(t *testing.T) { + // A new invariant config adds a variant of every invariant dir, so it scores as a new + // test on top of the fixture change. Changing an existing config, or adding one of its + // -init.sh companions, only changes how an existing variant runs. + for status, want := range map[string]int{"A": 10, "M": 5} { + result := selection.FromDiff(diffLines(status+"\tacceptance/bundle/invariant/configs/job.yml.tmpl"), testDirs, 10) + assert.Equal(t, want, result.Selected[0].Score, "status=%s", status) + } + + result := selection.FromDiff(diffLines("A\tacceptance/bundle/invariant/configs/job.yml.tmpl-init.sh"), testDirs, 10) + assert.Equal(t, 5, result.Selected[0].Score) +} + func TestFromDiffNestedDir(t *testing.T) { // A file maps to the innermost test dir that owns it. diff := diffLines("M\tacceptance/cmd/sync/nested/deeper/script") From 30b09b03e0a8c77e595aa914189ee554129f8ec9 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Fri, 14 Aug 2026 11:12:25 +0200 Subject: [PATCH 14/21] acc: fix selection issues found in review Three fixes, all in the selection package: - Two changed invariant configs produced two INPUT_CONFIG filters for the same dir. The harness requires every filter to match, so every variant skipped and those dirs ran nothing while still filling the quota. Such a dir now runs all of its variants. This one predates the branch. - A dir that was both new and moved scored 6; new and moved are meant to be exclusive, and it now scores as new. - The command resolved a bare path with a path-scoped git diff, which cannot pair a rename and reports the destination as added, so a moved test ranked as new. It now reads one whole diff, which also drops two git processes per argument, and rejects a non-positive limit instead of panicking on a negative slice bound. --- acceptance/internal/selection/cmd/main.go | 49 ++++++++++++++----- acceptance/internal/selection/selection.go | 29 +++++++---- .../internal/selection/selection_test.go | 42 ++++++++++++++++ 3 files changed, 99 insertions(+), 21 deletions(-) diff --git a/acceptance/internal/selection/cmd/main.go b/acceptance/internal/selection/cmd/main.go index 370418e6839..559eb1d952d 100644 --- a/acceptance/internal/selection/cmd/main.go +++ b/acceptance/internal/selection/cmd/main.go @@ -27,6 +27,10 @@ func main() { root := flag.String("root", "acceptance", "path to the acceptance directory") flag.Parse() + if *limit <= 0 { + fatalf("-limit must be a positive integer, got %d", *limit) + } + dirs, err := selection.FindTestDirs(*root) if err != nil { fatalf("cannot list test dirs in %s: %s", *root, err) @@ -58,6 +62,8 @@ func main() { // diffFromArgs renders command line arguments as `git diff --name-status` lines, so the // same selection runs on them as on a real diff. func diffFromArgs(args []string) string { + changed := changedLines() + lines := make([]string, 0, len(args)) for _, arg := range args { status, rest, ok := strings.Cut(arg, ":") @@ -69,26 +75,45 @@ func diffFromArgs(args []string) string { for i, path := range paths { paths[i] = strings.TrimPrefix(filepath.ToSlash(filepath.Clean(path)), "./") } - if status == "" { - status = gitStatus(paths[0]) + if status != "" { + lines = append(lines, status+"\t"+strings.Join(paths, "\t")) + continue } - lines = append(lines, status+"\t"+strings.Join(paths, "\t")) + // A path given without a status takes the line git reports for it, kept whole so a + // rename keeps both its status and its source path. A path git does not report as + // changed stands for a hypothetical change: modified when tracked, added when not. + if line, ok := changed[paths[0]]; ok { + lines = append(lines, line) + continue + } + lines = append(lines, gitStatus(paths[0])+"\t"+paths[0]) } return strings.Join(lines, "\n") } -// gitStatus is the status to assume for a path given without one: the status git reports -// against the merge base if the path is changed there, otherwise "M" for a tracked path -// and "A" for one git does not know, so a bare argument also stands for a hypothetical -// change to an existing test or a brand new one. -func gitStatus(path string) string { - out, err := exec.Command("git", "diff", "--name-status", "--merge-base", "-M", "origin/main", "--", path).Output() - if err == nil { - if fields := strings.Fields(string(out)); len(fields) > 0 { - return fields[0] +// changedLines maps each path this branch changed to its whole `git diff --name-status` +// line, keyed the way the diff names it now: the destination path of a rename, and the +// path itself otherwise. A path filter cannot be used for this, because git only pairs a +// rename when both of its paths are in the diff — asking about the destination alone +// reports an addition. +func changedLines() map[string]string { + out, err := exec.Command("git", "diff", "--name-status", "--merge-base", "-M", "origin/main").Output() + if err != nil { + fatalf("git diff --merge-base origin/main failed: %s", err) + } + + lines := map[string]string{} + for line := range strings.SplitSeq(strings.TrimSpace(string(out)), "\n") { + if fields := strings.Split(line, "\t"); len(fields) >= 2 { + lines[fields[len(fields)-1]] = line } } + return lines +} +// gitStatus is the status to assume for a path git does not report as changed: "M" for a +// tracked path and "A" for one git does not know. +func gitStatus(path string) string { if exec.Command("git", "ls-files", "--error-unmatch", "--", path).Run() == nil { return "M" } diff --git a/acceptance/internal/selection/selection.go b/acceptance/internal/selection/selection.go index 560f668641c..d3e1d18e4f0 100644 --- a/acceptance/internal/selection/selection.go +++ b/acceptance/internal/selection/selection.go @@ -189,8 +189,7 @@ type changedDir struct { // newTest is set when the test itself is new: the dir's script is new, or a new // invariant config adds a variant of the dir. moved is set when the script arrived as - // a rename, so the test only changed location. The two are exclusive: a script is - // either added or renamed. + // a rename, so the test only changed location. score treats them as exclusive. newTest bool moved bool @@ -215,13 +214,15 @@ const ( func (d *changedDir) score() int { score := 0 - if d.newTest { + switch { + case d.newTest: score += scoreNewTest - } - if d.moved { + case d.moved: // The files of a moved dir all arrive as renames. Moving a test does not change - // what it does, so those renames do not also count as changes. - return score + scoreMoved + // what it does, so those renames do not also count as changes. A dir that is new + // and moved at once (a renamed invariant dir picking up a new config) is scored as + // new, since being new says more about it than the move does. + return scoreMoved } if d.fixture { score += scoreChange @@ -302,8 +303,18 @@ func FromDiff(diff string, testDirs map[string]bool, limit int) Result { if status == "A" && strings.HasSuffix(path, configName) { d.newTest = true } - if !d.allVariants { - d.filters = append(d.filters, "INPUT_CONFIG="+configName) + filter := "INPUT_CONFIG=" + configName + switch { + case d.allVariants: + // A change to the dir itself already runs every variant. + case len(d.filters) == 0: + d.filters = []string{filter} + case d.filters[0] != filter: + // The harness requires every filter to match (see checkEnvFilters), so + // two different INPUT_CONFIG values would skip every variant and the + // dir would run nothing. Run all of its variants instead. + d.allVariants = true + d.filters = nil } } continue diff --git a/acceptance/internal/selection/selection_test.go b/acceptance/internal/selection/selection_test.go index 3bc899d425a..044f0e2c97a 100644 --- a/acceptance/internal/selection/selection_test.go +++ b/acceptance/internal/selection/selection_test.go @@ -139,6 +139,48 @@ func TestFromDiffNewInvariantConfig(t *testing.T) { assert.Equal(t, 5, result.Selected[0].Score) } +func TestFromDiffTwoInvariantConfigs(t *testing.T) { + // The harness requires every filter to match, so two INPUT_CONFIG filters would skip + // every variant. Two changed configs run all variants of the invariant dirs instead. + diff := diffLines( + "M\tacceptance/bundle/invariant/configs/job.yml.tmpl", + "M\tacceptance/bundle/invariant/configs/pipeline.yml.tmpl", + ) + result := selection.FromDiff(diff, testDirs, 10) + assert.Equal(t, map[string][]string{ + "bundle/invariant/jobs": nil, + "bundle/invariant/apps": nil, + }, result.Tests()) +} + +func TestFromDiffConfigAndItsInitScript(t *testing.T) { + // A config and its setup script name the same variant, so the filter is kept. + diff := diffLines( + "M\tacceptance/bundle/invariant/configs/job.yml.tmpl", + "M\tacceptance/bundle/invariant/configs/job.yml.tmpl-init.sh", + ) + result := selection.FromDiff(diff, testDirs, 10) + assert.Equal(t, map[string][]string{ + "bundle/invariant/jobs": {"INPUT_CONFIG=job.yml.tmpl"}, + "bundle/invariant/apps": {"INPUT_CONFIG=job.yml.tmpl"}, + }, result.Tests()) +} + +func TestFromDiffNewAndMovedAreExclusive(t *testing.T) { + // A moved invariant dir that also picks up a new config scores as new, not as new plus + // moved. + diff := diffLines( + "A\tacceptance/bundle/invariant/configs/job.yml.tmpl", + "R100\tacceptance/bundle/invariant/old/script\tacceptance/bundle/invariant/jobs/script", + ) + result := selection.FromDiff(diff, testDirs, 10) + scores := map[string]int{} + for _, test := range result.Selected { + scores[test.Dir] = test.Score + } + assert.Equal(t, 10, scores["bundle/invariant/jobs"]) +} + func TestFromDiffNestedDir(t *testing.T) { // A file maps to the innermost test dir that owns it. diff := diffLines("M\tacceptance/cmd/sync/nested/deeper/script") From 00d2daadf67f02ec8fea15fb5a77e05c44a89fe7 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 17 Aug 2026 10:11:25 +0200 Subject: [PATCH 15/21] acc: filters naming the same key are alternatives, add selection selftest Two changed invariant configs produced two INPUT_CONFIG filters for the same dir, and the harness required every filter to match, so every variant skipped and those dirs ran nothing. The environment has one value per key, so filters sharing a key now mean any of them: two changed configs run the variants of both. This also makes ENVFILTER=INPUT_CONFIG=a,INPUT_CONFIG=b run both variants rather than none, and lets checkEnvFilters reuse one matcher. acceptance/selftest/selection prints what the selection command reports for a changed test, a new test, a regenerated golden, a moved test, a touched invariant config, and a new config alongside two touched ones. --- acceptance/acceptance_test.go | 17 +++---- acceptance/internal/selection/cmd/main.go | 7 ++- acceptance/internal/selection/selection.go | 15 ++---- .../internal/selection/selection_test.go | 9 ++-- acceptance/selftest/selection/out.test.toml | 3 ++ acceptance/selftest/selection/output.txt | 47 +++++++++++++++++++ acceptance/selftest/selection/script | 33 +++++++++++++ acceptance/selftest/selection/test.toml | 9 ++++ acceptance/subset_test.go | 19 ++++++-- acceptance/subset_unit_test.go | 14 ++++++ 10 files changed, 141 insertions(+), 32 deletions(-) create mode 100644 acceptance/selftest/selection/out.test.toml create mode 100644 acceptance/selftest/selection/output.txt create mode 100644 acceptance/selftest/selection/script create mode 100644 acceptance/selftest/selection/test.toml diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 12764d90150..13c6647dacf 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1056,18 +1056,13 @@ func runTest(t *testing.T, } } -// checkEnvFilters skips the test if any env filter doesn't match testEnv. +// checkEnvFilters skips the test if any env filter doesn't match testEnv. Filters that +// share a key are alternatives, so INPUT_CONFIG=a together with INPUT_CONFIG=b runs both +// variants rather than neither (see envMatchesFilters). func checkEnvFilters(t *testing.T, testEnv, envFilters []string) { - envMap := make(map[string]string, len(testEnv)) - for _, kv := range testEnv { - key, value, _ := strings.Cut(kv, "=") - envMap[key] = value - } - for i, filter := range envFilters { - key, expected, _ := strings.Cut(filter, "=") - if actual, ok := envMap[key]; ok && actual != expected { - t.Skipf("Skipping because test environment %s=%s does not match ENVFILTER#%d: %s", key, actual, i, filter) - } + if !envMatchesFilters(testEnv, envFilters) { + t.Skipf("Skipping because test environment (%s) does not match filters (%s)", + strings.Join(testEnv, " "), strings.Join(envFilters, " ")) } } diff --git a/acceptance/internal/selection/cmd/main.go b/acceptance/internal/selection/cmd/main.go index 559eb1d952d..7d9ba28854e 100644 --- a/acceptance/internal/selection/cmd/main.go +++ b/acceptance/internal/selection/cmd/main.go @@ -62,7 +62,9 @@ func main() { // diffFromArgs renders command line arguments as `git diff --name-status` lines, so the // same selection runs on them as on a real diff. func diffFromArgs(args []string) string { - changed := changedLines() + // Looked up lazily: with a status on every argument the command needs no git at all, + // which is what lets a test drive it against a directory tree of its own. + var changed map[string]string lines := make([]string, 0, len(args)) for _, arg := range args { @@ -82,6 +84,9 @@ func diffFromArgs(args []string) string { // A path given without a status takes the line git reports for it, kept whole so a // rename keeps both its status and its source path. A path git does not report as // changed stands for a hypothetical change: modified when tracked, added when not. + if changed == nil { + changed = changedLines() + } if line, ok := changed[paths[0]]; ok { lines = append(lines, line) continue diff --git a/acceptance/internal/selection/selection.go b/acceptance/internal/selection/selection.go index d3e1d18e4f0..fbc41a92d32 100644 --- a/acceptance/internal/selection/selection.go +++ b/acceptance/internal/selection/selection.go @@ -303,18 +303,11 @@ func FromDiff(diff string, testDirs map[string]bool, limit int) Result { if status == "A" && strings.HasSuffix(path, configName) { d.newTest = true } + // Filters that name the same key are alternatives (see envMatchesFilters), + // so several changed configs restrict the dir to their variants together. filter := "INPUT_CONFIG=" + configName - switch { - case d.allVariants: - // A change to the dir itself already runs every variant. - case len(d.filters) == 0: - d.filters = []string{filter} - case d.filters[0] != filter: - // The harness requires every filter to match (see checkEnvFilters), so - // two different INPUT_CONFIG values would skip every variant and the - // dir would run nothing. Run all of its variants instead. - d.allVariants = true - d.filters = nil + if !d.allVariants && !slices.Contains(d.filters, filter) { + d.filters = append(d.filters, filter) } } continue diff --git a/acceptance/internal/selection/selection_test.go b/acceptance/internal/selection/selection_test.go index 044f0e2c97a..b4ad8930803 100644 --- a/acceptance/internal/selection/selection_test.go +++ b/acceptance/internal/selection/selection_test.go @@ -140,16 +140,17 @@ func TestFromDiffNewInvariantConfig(t *testing.T) { } func TestFromDiffTwoInvariantConfigs(t *testing.T) { - // The harness requires every filter to match, so two INPUT_CONFIG filters would skip - // every variant. Two changed configs run all variants of the invariant dirs instead. + // Two changed configs restrict the invariant dirs to the variants of both, as filters + // naming the same key are alternatives. diff := diffLines( "M\tacceptance/bundle/invariant/configs/job.yml.tmpl", "M\tacceptance/bundle/invariant/configs/pipeline.yml.tmpl", ) result := selection.FromDiff(diff, testDirs, 10) + both := []string{"INPUT_CONFIG=job.yml.tmpl", "INPUT_CONFIG=pipeline.yml.tmpl"} assert.Equal(t, map[string][]string{ - "bundle/invariant/jobs": nil, - "bundle/invariant/apps": nil, + "bundle/invariant/jobs": both, + "bundle/invariant/apps": both, }, result.Tests()) } diff --git a/acceptance/selftest/selection/out.test.toml b/acceptance/selftest/selection/out.test.toml new file mode 100644 index 00000000000..3c9d695f3b1 --- /dev/null +++ b/acceptance/selftest/selection/out.test.toml @@ -0,0 +1,3 @@ +Cloud = false +GOOS.windows = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = [] diff --git a/acceptance/selftest/selection/output.txt b/acceptance/selftest/selection/output.txt new file mode 100644 index 00000000000..55e85f87026 --- /dev/null +++ b/acceptance/selftest/selection/output.txt @@ -0,0 +1,47 @@ + +=== Changing a test + +>>> selection M:acceptance/mytest/script +Selected 1 changed tests (limit=50, 0 not selected) + 5 mytest + +=== Adding a test + +>>> selection A:acceptance/newtest/script +Selected 1 changed tests (limit=50, 0 not selected) + 10 newtest + +=== A regenerated golden scores below a changed fixture + +>>> selection M:acceptance/mytest/output.txt M:acceptance/newtest/script +Selected 2 changed tests (limit=50, 0 not selected) + 5 newtest + 1 mytest + +=== Moving a test scores lowest + +>>> selection R100:acceptance/oldtest/script:acceptance/newtest/script M:acceptance/mytest/script +Selected 2 changed tests (limit=50, 0 not selected) + 5 mytest + 1 newtest + +=== Touching an invariant config runs every invariant test, for that config only + +>>> selection M:acceptance/bundle/invariant/configs/job.yml.tmpl +Selected 2 changed tests (limit=50, 0 not selected) + 5 bundle/invariant/apps[INPUT_CONFIG=job.yml.tmpl] + 5 bundle/invariant/jobs[INPUT_CONFIG=job.yml.tmpl] + +=== Adding one invariant config and touching two others + +>>> selection A:acceptance/bundle/invariant/configs/new.yml.tmpl M:acceptance/bundle/invariant/configs/job.yml.tmpl M:acceptance/bundle/invariant/configs/pipeline.yml.tmpl +Selected 2 changed tests (limit=50, 0 not selected) + 10 bundle/invariant/apps[INPUT_CONFIG=new.yml.tmpl,INPUT_CONFIG=job.yml.tmpl,INPUT_CONFIG=pipeline.yml.tmpl] + 10 bundle/invariant/jobs[INPUT_CONFIG=new.yml.tmpl,INPUT_CONFIG=job.yml.tmpl,INPUT_CONFIG=pipeline.yml.tmpl] + +=== The limit keeps the highest scoring tests + +>>> selection -limit 2 A:acceptance/newtest/script M:acceptance/mytest/script M:acceptance/bundle/invariant/configs/job.yml.tmpl +Selected 2 changed tests (limit=2, 2 not selected) + 10 newtest + 5 bundle/invariant/apps[INPUT_CONFIG=job.yml.tmpl] diff --git a/acceptance/selftest/selection/script b/acceptance/selftest/selection/script new file mode 100644 index 00000000000..a6cb17917c9 --- /dev/null +++ b/acceptance/selftest/selection/script @@ -0,0 +1,33 @@ +# Build the fake acceptance tree the selection runs against. It cannot be committed: +# a directory holding a file named "script" is a test case, so the harness would pick +# these up as tests of their own. +mkdir -p tree/mytest tree/newtest tree/bundle/invariant/jobs tree/bundle/invariant/apps +touch tree/mytest/script tree/mytest/output.txt tree/newtest/script +touch tree/bundle/invariant/jobs/script tree/bundle/invariant/apps/script + +# Each argument is a changed file: "M:" modified, "A:" added, "R100:old:new" moved. The +# paths need not exist; only the tree of test dirs behind -root does. +selection() { + go -C "$TESTROOT" run ./internal/selection/cmd -root "$PWD/tree" "$@" +} + +title "Changing a test\n" +trace selection M:acceptance/mytest/script + +title "Adding a test\n" +trace selection A:acceptance/newtest/script + +title "A regenerated golden scores below a changed fixture\n" +trace selection M:acceptance/mytest/output.txt M:acceptance/newtest/script + +title "Moving a test scores lowest\n" +trace selection R100:acceptance/oldtest/script:acceptance/newtest/script M:acceptance/mytest/script + +title "Touching an invariant config runs every invariant test, for that config only\n" +trace selection M:acceptance/bundle/invariant/configs/job.yml.tmpl + +title "Adding one invariant config and touching two others\n" +trace selection A:acceptance/bundle/invariant/configs/new.yml.tmpl M:acceptance/bundle/invariant/configs/job.yml.tmpl M:acceptance/bundle/invariant/configs/pipeline.yml.tmpl + +title "The limit keeps the highest scoring tests\n" +trace selection -limit 2 A:acceptance/newtest/script M:acceptance/mytest/script M:acceptance/bundle/invariant/configs/job.yml.tmpl diff --git a/acceptance/selftest/selection/test.toml b/acceptance/selftest/selection/test.toml new file mode 100644 index 00000000000..ffc0dc86aa6 --- /dev/null +++ b/acceptance/selftest/selection/test.toml @@ -0,0 +1,9 @@ +# Test selection has nothing to do with the deployment engine. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = [] + +# The fake acceptance tree the command runs against, built by the script. +Ignore = ["tree"] + +# Git Bash rewrites an "A:..." argument as a Windows path (A: reads as a drive letter), +# so the statuses would not survive to the command. +GOOS.windows = false diff --git a/acceptance/subset_test.go b/acceptance/subset_test.go index 77ab97c9b3d..5618437bcb4 100644 --- a/acceptance/subset_test.go +++ b/acceptance/subset_test.go @@ -3,6 +3,7 @@ package acceptance_test import ( "hash/fnv" "os" + "slices" "strconv" "strings" "testing" @@ -108,18 +109,26 @@ func (s subsetSelector) isChanged(dir string, envset []string) bool { return envMatchesFilters(envset, filters) } -// envMatchesFilters reports whether envset satisfies every KEY=value filter, mirroring -// the skip semantics of checkEnvFilters: a filter matches unless its key is present in -// envset with a different value. +// envMatchesFilters reports whether envset satisfies every KEY=value filter. A filter +// matches unless its key is present in envset with a different value. Filters that share a +// key are alternatives: the environment has one value per key, so requiring each of them +// separately would match nothing, while a selection that names several values of a key +// (e.g. one INPUT_CONFIG per changed invariant config) means any of them. func envMatchesFilters(envset, filters []string) bool { envMap := make(map[string]string, len(envset)) for _, kv := range envset { key, value, _ := strings.Cut(kv, "=") envMap[key] = value } + + expected := make(map[string][]string, len(filters)) for _, filter := range filters { - key, expected, _ := strings.Cut(filter, "=") - if actual, ok := envMap[key]; ok && actual != expected { + key, value, _ := strings.Cut(filter, "=") + expected[key] = append(expected[key], value) + } + + for key, values := range expected { + if actual, ok := envMap[key]; ok && !slices.Contains(values, actual) { return false } } diff --git a/acceptance/subset_unit_test.go b/acceptance/subset_unit_test.go index 6f83e859841..607f01cdf4c 100644 --- a/acceptance/subset_unit_test.go +++ b/acceptance/subset_unit_test.go @@ -46,6 +46,20 @@ func TestSubsetChangedVariantFilter(t *testing.T) { assert.NotEmpty(t, s.skipReason("bundle/invariant/x", []string{"INPUT_CONFIG=pipeline.yml.tmpl"})) } +func TestSubsetChangedVariantFilterAlternatives(t *testing.T) { + // Filters naming the same key are alternatives, so the variants of both configs are + // kept rather than neither. + s := subsetSelector{ + enabled: true, + pct: 0, + seed: "seed", + changed: map[string][]string{"bundle/invariant/x": {"INPUT_CONFIG=job.yml.tmpl", "INPUT_CONFIG=pipeline.yml.tmpl"}}, + } + assert.Empty(t, s.skipReason("bundle/invariant/x", []string{"INPUT_CONFIG=job.yml.tmpl"})) + assert.Empty(t, s.skipReason("bundle/invariant/x", []string{"INPUT_CONFIG=pipeline.yml.tmpl"})) + assert.NotEmpty(t, s.skipReason("bundle/invariant/x", []string{"INPUT_CONFIG=other.yml.tmpl"})) +} + func TestSubsetSeedDeterministic(t *testing.T) { // Same seed selects the same subset; the decision does not depend on ordering. a := subsetSelector{enabled: true, pct: 50, seed: "abc"} From a01e90ab60cf423aa6ac469c8bf8c6d2389fc657 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 17 Aug 2026 13:46:49 +0200 Subject: [PATCH 16/21] acc: print one selected test per line, named the way go test names it A dir restricted to some of its variants printed them merged into one line, which read like a syntax of its own. Each variant is now its own line, named bundle/invariant/no_drift/INPUT_CONFIG=job.yml.tmpl. --- acceptance/internal/selection/cmd/main.go | 4 +++- acceptance/internal/selection/selection.go | 18 ++++++++++++------ acceptance/selftest/selection/output.txt | 14 +++++++++----- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/acceptance/internal/selection/cmd/main.go b/acceptance/internal/selection/cmd/main.go index 7d9ba28854e..cabab7fcff7 100644 --- a/acceptance/internal/selection/cmd/main.go +++ b/acceptance/internal/selection/cmd/main.go @@ -52,7 +52,9 @@ func main() { fmt.Println(result.Counts()) for _, test := range result.Selected { - fmt.Printf(" %3d %s\n", test.Score, test.Name()) + for _, name := range test.Names() { + fmt.Printf(" %3d %s\n", test.Score, name) + } } if len(result.Selected) == 0 { fmt.Println(" none of the changed files belong to a test dir") diff --git a/acceptance/internal/selection/selection.go b/acceptance/internal/selection/selection.go index fbc41a92d32..ebd183adddc 100644 --- a/acceptance/internal/selection/selection.go +++ b/acceptance/internal/selection/selection.go @@ -44,12 +44,18 @@ type Test struct { Score int } -// Name is the test dir with its variant filters, as the log and the command print it. -func (t Test) Name() string { - if t.Filters == nil { - return t.Dir +// Names is the test as go test names it: the dir on its own when every variant runs, and +// one name per variant when the selection is restricted to some of them. +func (t Test) Names() []string { + if len(t.Filters) == 0 { + return []string{t.Dir} } - return t.Dir + "[" + strings.Join(t.Filters, ",") + "]" + + names := make([]string, 0, len(t.Filters)) + for _, filter := range t.Filters { + names = append(names, t.Dir+"/"+filter) + } + return names } // Result is the outcome of a selection. @@ -84,7 +90,7 @@ func (r Result) Counts() string { func (r Result) Summary() string { names := make([]string, 0, len(r.Selected)) for _, test := range r.Selected { - names = append(names, test.Name()) + names = append(names, test.Names()...) } return r.Counts() + ": " + strings.Join(names, " ") } diff --git a/acceptance/selftest/selection/output.txt b/acceptance/selftest/selection/output.txt index 55e85f87026..e4f9924c2b9 100644 --- a/acceptance/selftest/selection/output.txt +++ b/acceptance/selftest/selection/output.txt @@ -29,19 +29,23 @@ Selected 2 changed tests (limit=50, 0 not selected) >>> selection M:acceptance/bundle/invariant/configs/job.yml.tmpl Selected 2 changed tests (limit=50, 0 not selected) - 5 bundle/invariant/apps[INPUT_CONFIG=job.yml.tmpl] - 5 bundle/invariant/jobs[INPUT_CONFIG=job.yml.tmpl] + 5 bundle/invariant/apps/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/jobs/INPUT_CONFIG=job.yml.tmpl === Adding one invariant config and touching two others >>> selection A:acceptance/bundle/invariant/configs/new.yml.tmpl M:acceptance/bundle/invariant/configs/job.yml.tmpl M:acceptance/bundle/invariant/configs/pipeline.yml.tmpl Selected 2 changed tests (limit=50, 0 not selected) - 10 bundle/invariant/apps[INPUT_CONFIG=new.yml.tmpl,INPUT_CONFIG=job.yml.tmpl,INPUT_CONFIG=pipeline.yml.tmpl] - 10 bundle/invariant/jobs[INPUT_CONFIG=new.yml.tmpl,INPUT_CONFIG=job.yml.tmpl,INPUT_CONFIG=pipeline.yml.tmpl] + 10 bundle/invariant/apps/INPUT_CONFIG=new.yml.tmpl + 10 bundle/invariant/apps/INPUT_CONFIG=job.yml.tmpl + 10 bundle/invariant/apps/INPUT_CONFIG=pipeline.yml.tmpl + 10 bundle/invariant/jobs/INPUT_CONFIG=new.yml.tmpl + 10 bundle/invariant/jobs/INPUT_CONFIG=job.yml.tmpl + 10 bundle/invariant/jobs/INPUT_CONFIG=pipeline.yml.tmpl === The limit keeps the highest scoring tests >>> selection -limit 2 A:acceptance/newtest/script M:acceptance/mytest/script M:acceptance/bundle/invariant/configs/job.yml.tmpl Selected 2 changed tests (limit=2, 2 not selected) 10 newtest - 5 bundle/invariant/apps[INPUT_CONFIG=job.yml.tmpl] + 5 bundle/invariant/apps/INPUT_CONFIG=job.yml.tmpl From 28c759c38cd886d6a3780f652ee39b03e65d5d79 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 17 Aug 2026 14:18:26 +0200 Subject: [PATCH 17/21] acc: score each variant on its own, print runnable test names Adding one invariant config marked the whole dir as new, so the variants of the configs that were merely changed scored as new too. A variant is now the unit of selection: it carries its own score, and only the new config's variant counts as new. The command prints each selected test as a name go test accepts, expanded from the variant matrix in the test's materialized config, so a line can be run as printed. It is built by the harness like the CLI is, because tests run with a sandboxed HOME that has no module cache. --- acceptance/acceptance_test.go | 25 ++- acceptance/internal/selection/cmd/main.go | 51 +++++- acceptance/internal/selection/selection.go | 170 ++++++++++++------ .../internal/selection/selection_test.go | 31 +++- acceptance/selftest/selection/output.txt | 31 ++-- acceptance/selftest/selection/script | 18 +- acceptance/subset_test.go | 31 +--- 7 files changed, 246 insertions(+), 111 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 13c6647dacf..a749aded6b5 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -352,6 +352,12 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { t.Setenv("CLI", execPath) repls.SetPath(execPath, "[CLI]") + // Built here rather than run with "go run" from a test: tests run with a sandboxed + // HOME, which has no module cache, so building inside one fails to resolve imports. + selectionPath := buildSelectionCmd(t, buildDir) + t.Setenv("SELECTION", selectionPath) + repls.SetPath(selectionPath, "[SELECTION]") + if !inprocessMode { cli293Path := DownloadCLI(t, buildDir, "0.293.0") t.Setenv("CLI_293", cli293Path) @@ -1058,9 +1064,9 @@ func runTest(t *testing.T, // checkEnvFilters skips the test if any env filter doesn't match testEnv. Filters that // share a key are alternatives, so INPUT_CONFIG=a together with INPUT_CONFIG=b runs both -// variants rather than neither (see envMatchesFilters). +// variants rather than neither (see selection.MatchesFilters). func checkEnvFilters(t *testing.T, testEnv, envFilters []string) { - if !envMatchesFilters(testEnv, envFilters) { + if !selection.MatchesFilters(testEnv, envFilters) { t.Skipf("Skipping because test environment (%s) does not match filters (%s)", strings.Join(testEnv, " "), strings.Join(envFilters, " ")) } @@ -1286,6 +1292,21 @@ func BuildCLI(t *testing.T, buildDir, coverDir, osName, arch string) string { return execPath } +// buildSelectionCmd builds the test selection command, so a test can run it the way a +// developer does. +func buildSelectionCmd(t *testing.T, buildDir string) string { + execPath := filepath.Join(buildDir, "selection"+exeSuffix) + + args := []string{"go", "build", "-o", execPath, "./internal/selection/cmd"} + if runtime.GOOS == "windows" { + // See BuildCLI: VCS stamping fails on Windows. + args = append(args, "-buildvcs=false") + } + RunCommand(t, args, ".", nil) + + return execPath +} + // CreateReleaseArtifacts builds release artifacts for the given OS using amd64 and arm64 architectures, // archives them into zip files, and returns the directory containing the release artifacts. func CreateReleaseArtifacts(t *testing.T, cwd, coverDir, osName string) string { diff --git a/acceptance/internal/selection/cmd/main.go b/acceptance/internal/selection/cmd/main.go index cabab7fcff7..7c840e77329 100644 --- a/acceptance/internal/selection/cmd/main.go +++ b/acceptance/internal/selection/cmd/main.go @@ -9,6 +9,14 @@ // // A bare path takes the status git reports for it: added for a path git does not track, // modified otherwise. Prefix a path to force one ("A:path", "M:path", "R100:old:new"). +// +// Each line is a name go test accepts, so a selected test can be run as printed: +// +// go test ./acceptance -run 'TestAccept/bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl' +// +// The variants come from each test's materialized config (out.test.toml), which already has +// the excluded matrix values removed. Excludes that name a combination of variables are not +// recorded there, so a listed variant can still turn out to be one the harness skips. package main import ( @@ -19,6 +27,9 @@ import ( "path/filepath" "strings" + "github.com/BurntSushi/toml" + + "github.com/databricks/cli/acceptance/internal" "github.com/databricks/cli/acceptance/internal/selection" ) @@ -52,7 +63,7 @@ func main() { fmt.Println(result.Counts()) for _, test := range result.Selected { - for _, name := range test.Names() { + for _, name := range variantNames(*root, test) { fmt.Printf(" %3d %s\n", test.Score, name) } } @@ -61,6 +72,44 @@ func main() { } } +// variantNames returns the go test names of the selected test: one per variant of its dir +// that the selection covers, or the dir alone when it has no variants. +func variantNames(root string, test selection.Test) []string { + var filters []string + if test.Filter != "" { + filters = []string{test.Filter} + } + + var names []string + for _, envset := range internal.ExpandEnvMatrix(envMatrix(root, test.Dir), nil, nil) { + if len(envset) == 0 { + // The harness runs a test without variants as a plain subtest of its dir. + return []string{test.Dir} + } + if selection.MatchesFilters(envset, filters) { + names = append(names, test.Dir+"/"+strings.Join(envset, "/")) + } + } + return names +} + +// envMatrix reads the variant matrix of a test from its materialized config, the same file +// the harness generates so that inherited settings are visible. +func envMatrix(root, dir string) map[string][]string { + path := filepath.Join(root, dir, internal.MaterializedConfigFile) + contents, err := os.ReadFile(path) + if err != nil { + // A test dir without a materialized config has no variants. + return nil + } + + var config internal.TestConfig + if _, err := toml.Decode(string(contents), &config); err != nil { + fatalf("cannot parse %s: %s", path, err) + } + return config.EnvMatrix +} + // diffFromArgs renders command line arguments as `git diff --name-status` lines, so the // same selection runs on them as on a real diff. func diffFromArgs(args []string) string { diff --git a/acceptance/internal/selection/selection.go b/acceptance/internal/selection/selection.go index ebd183adddc..3b7215f816a 100644 --- a/acceptance/internal/selection/selection.go +++ b/acceptance/internal/selection/selection.go @@ -31,31 +31,28 @@ const ( invariantDirPrefix = "bundle/invariant/" ) -// Test is one test the selection picked. +// Test is one test the selection picked: a whole test dir, or one variant of it when only +// some of its variants changed. type Test struct { // Dir is the test dir, relative to acceptance/. Dir string - // Filters restricts the run to the variants matching these KEY=value filters. A nil - // slice means every variant of the dir runs. - Filters []string + // Filter is the KEY=value the variant is selected by, empty when every variant of the + // dir runs. Each variant is picked, and scored, on its own: adding one invariant config + // makes that config's variant new without touching the others. + Filter string // Score is why the test was picked; see the score constants. Score int } -// Names is the test as go test names it: the dir on its own when every variant runs, and -// one name per variant when the selection is restricted to some of them. -func (t Test) Names() []string { - if len(t.Filters) == 0 { - return []string{t.Dir} +// Name is the test as go test names it, without the variants the selection says nothing +// about (see MatchesFilters). +func (t Test) Name() string { + if t.Filter == "" { + return t.Dir } - - names := make([]string, 0, len(t.Filters)) - for _, filter := range t.Filters { - names = append(names, t.Dir+"/"+filter) - } - return names + return t.Dir + "/" + t.Filter } // Result is the outcome of a selection. @@ -72,11 +69,17 @@ type Result struct { } // Tests maps each selected test dir to the variant filters it runs with, the form the -// acceptance harness looks tests up by. +// acceptance harness looks tests up by. A nil slice means every variant of the dir runs. func (r Result) Tests() map[string][]string { tests := make(map[string][]string, len(r.Selected)) for _, test := range r.Selected { - tests[test.Dir] = test.Filters + if test.Filter == "" { + tests[test.Dir] = nil + continue + } + if filters, ok := tests[test.Dir]; !ok || filters != nil { + tests[test.Dir] = append(filters, test.Filter) + } } return tests } @@ -90,7 +93,7 @@ func (r Result) Counts() string { func (r Result) Summary() string { names := make([]string, 0, len(r.Selected)) for _, test := range r.Selected { - names = append(names, test.Names()...) + names = append(names, test.Name()) } return r.Counts() + ": " + strings.Join(names, " ") } @@ -110,6 +113,32 @@ func ParseLimit(raw string) (int, error) { return limit, nil } +// MatchesFilters reports whether envset satisfies every KEY=value filter. A filter matches +// unless its key is present in envset with a different value. Filters that share a key are +// alternatives: the environment has one value per key, so requiring each of them separately +// would match nothing, while a selection naming several values of a key (one INPUT_CONFIG +// per changed invariant config) means any of them. +func MatchesFilters(envset, filters []string) bool { + envMap := make(map[string]string, len(envset)) + for _, kv := range envset { + key, value, _ := strings.Cut(kv, "=") + envMap[key] = value + } + + expected := make(map[string][]string, len(filters)) + for _, filter := range filters { + key, value, _ := strings.Cut(filter, "=") + expected[key] = append(expected[key], value) + } + + for key, values := range expected { + if actual, ok := envMap[key]; ok && !slices.Contains(values, actual) { + return false + } + } + return true +} + // FindTestDirs returns every test dir under root, named relative to root with forward // slashes, sorted. func FindTestDirs(root string) ([]string, error) { @@ -184,18 +213,18 @@ func testDirForFile(repoRelPath string, testDirs map[string]bool) string { // changedDir records how one test dir changed and which of its variants should run. type changedDir struct { - // filters restricts the run to the variants matching these KEY=value filters. - // Empty means every variant of the dir runs. - filters []string + // filters holds one record per variant the dir is restricted to. Empty means every + // variant runs. + filters []variantFilter // allVariants is set by a change to the dir itself, as opposed to a change to an // invariant config the dir is generated from. It clears filters and keeps a later // config change from narrowing the dir back down to one config. allVariants bool - // newTest is set when the test itself is new: the dir's script is new, or a new - // invariant config adds a variant of the dir. moved is set when the script arrived as - // a rename, so the test only changed location. score treats them as exclusive. + // newTest is set when the dir's script is new, so the whole test is new. moved is set + // when the script arrived as a rename, so the test only changed location. score treats + // them as exclusive. newTest bool moved bool @@ -218,17 +247,16 @@ const ( scoreMoved = 1 ) -func (d *changedDir) score() int { +func (d *changedDir) score(newVariant bool) int { + if d.moved { + // The files of a moved dir all arrive as renames. Moving a test does not change what + // it does, so those renames do not also count as changes. + return scoreMoved + } + score := 0 - switch { - case d.newTest: + if d.newTest || newVariant { score += scoreNewTest - case d.moved: - // The files of a moved dir all arrive as renames. Moving a test does not change - // what it does, so those renames do not also count as changes. A dir that is new - // and moved at once (a renamed invariant dir picking up a new config) is scored as - // new, since being new says more about it than the move does. - return scoreMoved } if d.fixture { score += scoreChange @@ -239,9 +267,36 @@ func (d *changedDir) score() int { return score } +// variantFilter is one variant of a dir, selected because the invariant config it is +// generated from changed. +type variantFilter struct { + // env is the KEY=value the variant is selected by. + env string + + // newVariant is set when the config is new, so this variant of the dir is new while its + // other variants are not. + newVariant bool +} + // changedDirs maps a test dir, relative to acceptance/, to how it changed. type changedDirs map[string]*changedDir +// addFilter restricts the dir to one more variant, unless a change to the dir itself +// already runs every variant. Repeating a variant (a config and its setup script) keeps the +// stronger record. +func (d *changedDir) addFilter(env string, newVariant bool) { + if d.allVariants { + return + } + for i, filter := range d.filters { + if filter.env == env { + d.filters[i].newVariant = filter.newVariant || newVariant + return + } + } + d.filters = append(d.filters, variantFilter{env: env, newVariant: newVariant}) +} + func (c changedDirs) get(dir string) *changedDir { if d, ok := c[dir]; ok { return d @@ -301,20 +356,13 @@ func FromDiff(diff string, testDirs map[string]bool, limit int) Result { continue } d := dirs.get(dir) - // The config is the fixture these dirs are generated from, and a new - // config adds a variant of each of them. Its -init.sh / -cleanup.sh - // companions change how an existing variant runs, so only the config - // itself counts as a new test. + // The config is the fixture these dirs are generated from. A new config + // adds one variant of each dir, so only that variant is new; the + // -init.sh / -cleanup.sh companions of a config change how an existing + // variant runs. d.fixture = true - if status == "A" && strings.HasSuffix(path, configName) { - d.newTest = true - } - // Filters that name the same key are alternatives (see envMatchesFilters), - // so several changed configs restrict the dir to their variants together. - filter := "INPUT_CONFIG=" + configName - if !d.allVariants && !slices.Contains(d.filters, filter) { - d.filters = append(d.filters, filter) - } + isNewConfig := status == "A" && strings.HasSuffix(path, configName) + d.addFilter("INPUT_CONFIG="+configName, isNewConfig) } continue } @@ -334,6 +382,8 @@ func FromDiff(diff string, testDirs map[string]bool, limit int) Result { } d := dirs.get(dir) + // A change to the dir itself runs every variant, so any variant filter it picked up + // from a config change is dropped and a later one is ignored. d.allVariants = true d.filters = nil if isGeneratedFile(path, dir) { @@ -352,19 +402,25 @@ func FromDiff(diff string, testDirs map[string]bool, limit int) Result { } } - // Sort by name first, then stably by descending score, so dirs that score the same + tests := make([]Test, 0, len(dirs)) + for _, dir := range slices.Sorted(maps.Keys(dirs)) { + d := dirs[dir] + if len(d.filters) == 0 { + tests = append(tests, Test{Dir: dir, Score: d.score(false)}) + continue + } + for _, filter := range d.filters { + tests = append(tests, Test{Dir: dir, Filter: filter.env, Score: d.score(filter.newVariant)}) + } + } + + // Sorted by name above, then stably by descending score, so tests that score the same // stay alphabetical. - selected := slices.Sorted(maps.Keys(dirs)) - slices.SortStableFunc(selected, func(a, b string) int { - return dirs[b].score() - dirs[a].score() + slices.SortStableFunc(tests, func(a, b Test) int { + return b.Score - a.Score }) - dropped := max(len(selected)-limit, 0) - selected = selected[:len(selected)-dropped] - - tests := make([]Test, 0, len(selected)) - for _, dir := range selected { - tests = append(tests, Test{Dir: dir, Filters: dirs[dir].filters, Score: dirs[dir].score()}) - } + dropped := max(len(tests)-limit, 0) + tests = tests[:len(tests)-dropped] return Result{Selected: tests, Dropped: dropped, Limit: limit} } diff --git a/acceptance/internal/selection/selection_test.go b/acceptance/internal/selection/selection_test.go index b4ad8930803..649fde32f47 100644 --- a/acceptance/internal/selection/selection_test.go +++ b/acceptance/internal/selection/selection_test.go @@ -167,9 +167,9 @@ func TestFromDiffConfigAndItsInitScript(t *testing.T) { }, result.Tests()) } -func TestFromDiffNewAndMovedAreExclusive(t *testing.T) { - // A moved invariant dir that also picks up a new config scores as new, not as new plus - // moved. +func TestFromDiffMovedDirWithNewConfig(t *testing.T) { + // The move is a change to the dir itself, so it runs every variant, including the one + // the new config adds, and scores the move. diff := diffLines( "A\tacceptance/bundle/invariant/configs/job.yml.tmpl", "R100\tacceptance/bundle/invariant/old/script\tacceptance/bundle/invariant/jobs/script", @@ -177,9 +177,30 @@ func TestFromDiffNewAndMovedAreExclusive(t *testing.T) { result := selection.FromDiff(diff, testDirs, 10) scores := map[string]int{} for _, test := range result.Selected { - scores[test.Dir] = test.Score + scores[test.Name()] = test.Score + } + assert.Equal(t, 1, scores["bundle/invariant/jobs"]) + assert.Nil(t, result.Tests()["bundle/invariant/jobs"]) +} + +func TestFromDiffNewConfigScoresOnlyItsVariant(t *testing.T) { + // Each variant is scored on its own: adding one config makes that config's variant new + // without making the variant of a config that was merely changed look new too. + diff := diffLines( + "M\tacceptance/bundle/invariant/configs/job.yml.tmpl", + "A\tacceptance/bundle/invariant/configs/pipeline.yml.tmpl", + ) + result := selection.FromDiff(diff, testDirs, 10) + scores := map[string]int{} + for _, test := range result.Selected { + scores[test.Name()] = test.Score } - assert.Equal(t, 10, scores["bundle/invariant/jobs"]) + assert.Equal(t, map[string]int{ + "bundle/invariant/apps/INPUT_CONFIG=job.yml.tmpl": 5, + "bundle/invariant/apps/INPUT_CONFIG=pipeline.yml.tmpl": 10, + "bundle/invariant/jobs/INPUT_CONFIG=job.yml.tmpl": 5, + "bundle/invariant/jobs/INPUT_CONFIG=pipeline.yml.tmpl": 10, + }, scores) } func TestFromDiffNestedDir(t *testing.T) { diff --git a/acceptance/selftest/selection/output.txt b/acceptance/selftest/selection/output.txt index e4f9924c2b9..f4f7cc28284 100644 --- a/acceptance/selftest/selection/output.txt +++ b/acceptance/selftest/selection/output.txt @@ -3,7 +3,8 @@ >>> selection M:acceptance/mytest/script Selected 1 changed tests (limit=50, 0 not selected) - 5 mytest + 5 mytest/DATABRICKS_BUNDLE_ENGINE=terraform + 5 mytest/DATABRICKS_BUNDLE_ENGINE=direct === Adding a test @@ -16,36 +17,36 @@ Selected 1 changed tests (limit=50, 0 not selected) >>> selection M:acceptance/mytest/output.txt M:acceptance/newtest/script Selected 2 changed tests (limit=50, 0 not selected) 5 newtest - 1 mytest + 1 mytest/DATABRICKS_BUNDLE_ENGINE=terraform + 1 mytest/DATABRICKS_BUNDLE_ENGINE=direct === Moving a test scores lowest >>> selection R100:acceptance/oldtest/script:acceptance/newtest/script M:acceptance/mytest/script Selected 2 changed tests (limit=50, 0 not selected) - 5 mytest + 5 mytest/DATABRICKS_BUNDLE_ENGINE=terraform + 5 mytest/DATABRICKS_BUNDLE_ENGINE=direct 1 newtest === Touching an invariant config runs every invariant test, for that config only >>> selection M:acceptance/bundle/invariant/configs/job.yml.tmpl Selected 2 changed tests (limit=50, 0 not selected) - 5 bundle/invariant/apps/INPUT_CONFIG=job.yml.tmpl - 5 bundle/invariant/jobs/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/apps/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/jobs/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl -=== Adding one invariant config and touching two others +=== Adding one invariant config and touching another: only the new variant is new ->>> selection A:acceptance/bundle/invariant/configs/new.yml.tmpl M:acceptance/bundle/invariant/configs/job.yml.tmpl M:acceptance/bundle/invariant/configs/pipeline.yml.tmpl -Selected 2 changed tests (limit=50, 0 not selected) - 10 bundle/invariant/apps/INPUT_CONFIG=new.yml.tmpl - 10 bundle/invariant/apps/INPUT_CONFIG=job.yml.tmpl - 10 bundle/invariant/apps/INPUT_CONFIG=pipeline.yml.tmpl - 10 bundle/invariant/jobs/INPUT_CONFIG=new.yml.tmpl - 10 bundle/invariant/jobs/INPUT_CONFIG=job.yml.tmpl - 10 bundle/invariant/jobs/INPUT_CONFIG=pipeline.yml.tmpl +>>> selection A:acceptance/bundle/invariant/configs/new.yml.tmpl M:acceptance/bundle/invariant/configs/job.yml.tmpl +Selected 4 changed tests (limit=50, 0 not selected) + 10 bundle/invariant/apps/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=new.yml.tmpl + 10 bundle/invariant/jobs/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=new.yml.tmpl + 5 bundle/invariant/apps/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/jobs/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl === The limit keeps the highest scoring tests >>> selection -limit 2 A:acceptance/newtest/script M:acceptance/mytest/script M:acceptance/bundle/invariant/configs/job.yml.tmpl Selected 2 changed tests (limit=2, 2 not selected) 10 newtest - 5 bundle/invariant/apps/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/apps/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl diff --git a/acceptance/selftest/selection/script b/acceptance/selftest/selection/script index a6cb17917c9..a38be3a85b5 100644 --- a/acceptance/selftest/selection/script +++ b/acceptance/selftest/selection/script @@ -5,10 +5,22 @@ mkdir -p tree/mytest tree/newtest tree/bundle/invariant/jobs tree/bundle/invaria touch tree/mytest/script tree/mytest/output.txt tree/newtest/script touch tree/bundle/invariant/jobs/script tree/bundle/invariant/apps/script +# The command reads each test's variants from its materialized config, the same file the +# harness generates. +for dir in tree/bundle/invariant/jobs tree/bundle/invariant/apps; do + cat > $dir/out.test.toml < tree/mytest/out.test.toml < Date: Tue, 18 Aug 2026 11:21:38 +0200 Subject: [PATCH 18/21] acc: run the selection selftest against the real acceptance tree The test built a tree of its own because a committed directory holding a file named "script" would be picked up as a test case. Pointing -root at the real tree instead drops that setup, exercises the real variant matrices, and prints names that name existing tests. The golden now moves when an invariant test dir or its matrix changes. --- acceptance/selftest/selection/output.txt | 76 ++++++++++++++++-------- acceptance/selftest/selection/script | 39 ++++-------- acceptance/selftest/selection/test.toml | 3 - 3 files changed, 61 insertions(+), 57 deletions(-) diff --git a/acceptance/selftest/selection/output.txt b/acceptance/selftest/selection/output.txt index f4f7cc28284..6828f50b902 100644 --- a/acceptance/selftest/selection/output.txt +++ b/acceptance/selftest/selection/output.txt @@ -1,52 +1,76 @@ === Changing a test ->>> selection M:acceptance/mytest/script +>>> selection M:acceptance/selftest/basic/script Selected 1 changed tests (limit=50, 0 not selected) - 5 mytest/DATABRICKS_BUNDLE_ENGINE=terraform - 5 mytest/DATABRICKS_BUNDLE_ENGINE=direct + 5 selftest/basic/DATABRICKS_BUNDLE_ENGINE=terraform + 5 selftest/basic/DATABRICKS_BUNDLE_ENGINE=direct === Adding a test ->>> selection A:acceptance/newtest/script +>>> selection A:acceptance/selftest/basic/script Selected 1 changed tests (limit=50, 0 not selected) - 10 newtest + 10 selftest/basic/DATABRICKS_BUNDLE_ENGINE=terraform + 10 selftest/basic/DATABRICKS_BUNDLE_ENGINE=direct === A regenerated golden scores below a changed fixture ->>> selection M:acceptance/mytest/output.txt M:acceptance/newtest/script +>>> selection M:acceptance/selftest/basic/output.txt M:acceptance/selftest/diff/script Selected 2 changed tests (limit=50, 0 not selected) - 5 newtest - 1 mytest/DATABRICKS_BUNDLE_ENGINE=terraform - 1 mytest/DATABRICKS_BUNDLE_ENGINE=direct + 5 selftest/diff/DATABRICKS_BUNDLE_ENGINE=terraform + 5 selftest/diff/DATABRICKS_BUNDLE_ENGINE=direct + 1 selftest/basic/DATABRICKS_BUNDLE_ENGINE=terraform + 1 selftest/basic/DATABRICKS_BUNDLE_ENGINE=direct === Moving a test scores lowest ->>> selection R100:acceptance/oldtest/script:acceptance/newtest/script M:acceptance/mytest/script +>>> selection R100:acceptance/selftest/old/script:acceptance/selftest/basic/script M:acceptance/selftest/diff/script Selected 2 changed tests (limit=50, 0 not selected) - 5 mytest/DATABRICKS_BUNDLE_ENGINE=terraform - 5 mytest/DATABRICKS_BUNDLE_ENGINE=direct - 1 newtest + 5 selftest/diff/DATABRICKS_BUNDLE_ENGINE=terraform + 5 selftest/diff/DATABRICKS_BUNDLE_ENGINE=direct + 1 selftest/basic/DATABRICKS_BUNDLE_ENGINE=terraform + 1 selftest/basic/DATABRICKS_BUNDLE_ENGINE=direct === Touching an invariant config runs every invariant test, for that config only >>> selection M:acceptance/bundle/invariant/configs/job.yml.tmpl -Selected 2 changed tests (limit=50, 0 not selected) - 5 bundle/invariant/apps/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl - 5 bundle/invariant/jobs/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl +Selected 5 changed tests (limit=50, 0 not selected) + 5 bundle/invariant/continue_293/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= + 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 + 5 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= + 5 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 + 5 bundle/invariant/migrate/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= + 5 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 === Adding one invariant config and touching another: only the new variant is new ->>> selection A:acceptance/bundle/invariant/configs/new.yml.tmpl M:acceptance/bundle/invariant/configs/job.yml.tmpl -Selected 4 changed tests (limit=50, 0 not selected) - 10 bundle/invariant/apps/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=new.yml.tmpl - 10 bundle/invariant/jobs/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=new.yml.tmpl - 5 bundle/invariant/apps/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl - 5 bundle/invariant/jobs/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl +>>> selection A:acceptance/bundle/invariant/configs/pipeline.yml.tmpl M:acceptance/bundle/invariant/configs/job.yml.tmpl +Selected 10 changed tests (limit=50, 0 not selected) + 10 bundle/invariant/continue_293/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl + 10 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN= + 10 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN=1 + 10 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN= + 10 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN=1 + 10 bundle/invariant/migrate/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl + 10 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN= + 10 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN=1 + 5 bundle/invariant/continue_293/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= + 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 + 5 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= + 5 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 + 5 bundle/invariant/migrate/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= + 5 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 === The limit keeps the highest scoring tests ->>> selection -limit 2 A:acceptance/newtest/script M:acceptance/mytest/script M:acceptance/bundle/invariant/configs/job.yml.tmpl -Selected 2 changed tests (limit=2, 2 not selected) - 10 newtest - 5 bundle/invariant/apps/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl +>>> selection -limit 3 A:acceptance/selftest/basic/script M:acceptance/selftest/diff/script M:acceptance/bundle/invariant/configs/job.yml.tmpl +Selected 3 changed tests (limit=3, 4 not selected) + 10 selftest/basic/DATABRICKS_BUNDLE_ENGINE=terraform + 10 selftest/basic/DATABRICKS_BUNDLE_ENGINE=direct + 5 bundle/invariant/continue_293/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= + 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 diff --git a/acceptance/selftest/selection/script b/acceptance/selftest/selection/script index a38be3a85b5..4ca6aa466b9 100644 --- a/acceptance/selftest/selection/script +++ b/acceptance/selftest/selection/script @@ -1,45 +1,28 @@ -# Build the fake acceptance tree the selection runs against. It cannot be committed: -# a directory holding a file named "script" is a test case, so the harness would pick -# these up as tests of their own. -mkdir -p tree/mytest tree/newtest tree/bundle/invariant/jobs tree/bundle/invariant/apps -touch tree/mytest/script tree/mytest/output.txt tree/newtest/script -touch tree/bundle/invariant/jobs/script tree/bundle/invariant/apps/script - -# The command reads each test's variants from its materialized config, the same file the -# harness generates. -for dir in tree/bundle/invariant/jobs tree/bundle/invariant/apps; do - cat > $dir/out.test.toml < tree/mytest/out.test.toml < Date: Wed, 19 Aug 2026 14:36:56 +0200 Subject: [PATCH 19/21] acc: fix the selection command build on Windows go build read the -buildvcs=false that Windows needs as a second package path, because it was appended after the one being built, and every Windows job failed on "malformed import path". --- acceptance/acceptance_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index a749aded6b5..bc0f722a339 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1297,11 +1297,13 @@ func BuildCLI(t *testing.T, buildDir, coverDir, osName, arch string) string { func buildSelectionCmd(t *testing.T, buildDir string) string { execPath := filepath.Join(buildDir, "selection"+exeSuffix) - args := []string{"go", "build", "-o", execPath, "./internal/selection/cmd"} + args := []string{"go", "build", "-o", execPath} if runtime.GOOS == "windows" { // See BuildCLI: VCS stamping fails on Windows. args = append(args, "-buildvcs=false") } + // The package path goes last: go build reads anything after it as another package. + args = append(args, "./internal/selection/cmd") RunCommand(t, args, ".", nil) return execPath From 2095134f3c034be912751fdddac7fd8f5ad97e36 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 20 Aug 2026 11:28:15 +0200 Subject: [PATCH 20/21] acc: keep git out of the selection command The command inferred the status of a bare path by running git itself. Now a bare path counts as modified, so the only thing that talks to git is the package, through the same call the test runner makes when the command is given no arguments at all. --- acceptance/internal/selection/cmd/main.go | 57 ++--------------------- 1 file changed, 5 insertions(+), 52 deletions(-) diff --git a/acceptance/internal/selection/cmd/main.go b/acceptance/internal/selection/cmd/main.go index 7c840e77329..e7835b8bebe 100644 --- a/acceptance/internal/selection/cmd/main.go +++ b/acceptance/internal/selection/cmd/main.go @@ -7,8 +7,9 @@ // go run ./acceptance/internal/selection/cmd -limit 5 // go run ./acceptance/internal/selection/cmd acceptance/bundle/invariant/configs/job.yml.tmpl // -// A bare path takes the status git reports for it: added for a path git does not track, -// modified otherwise. Prefix a path to force one ("A:path", "M:path", "R100:old:new"). +// With no arguments the selection comes from git, exactly as the test runner gets it. Each +// argument stands for a changed file instead, and counts as modified unless it carries a +// status ("A:path" for an added file, "R100:old:new" for a rename). // // Each line is a name go test accepts, so a selected test can be run as printed: // @@ -23,7 +24,6 @@ import ( "flag" "fmt" "os" - "os/exec" "path/filepath" "strings" @@ -113,69 +113,22 @@ func envMatrix(root, dir string) map[string][]string { // diffFromArgs renders command line arguments as `git diff --name-status` lines, so the // same selection runs on them as on a real diff. func diffFromArgs(args []string) string { - // Looked up lazily: with a status on every argument the command needs no git at all, - // which is what lets a test drive it against a directory tree of its own. - var changed map[string]string - lines := make([]string, 0, len(args)) for _, arg := range args { status, rest, ok := strings.Cut(arg, ":") if !ok { - status, rest = "", arg + status, rest = "M", arg } // A rename carries both paths ("R100:old:new"), which git separates by tabs. paths := strings.Split(rest, ":") for i, path := range paths { paths[i] = strings.TrimPrefix(filepath.ToSlash(filepath.Clean(path)), "./") } - if status != "" { - lines = append(lines, status+"\t"+strings.Join(paths, "\t")) - continue - } - // A path given without a status takes the line git reports for it, kept whole so a - // rename keeps both its status and its source path. A path git does not report as - // changed stands for a hypothetical change: modified when tracked, added when not. - if changed == nil { - changed = changedLines() - } - if line, ok := changed[paths[0]]; ok { - lines = append(lines, line) - continue - } - lines = append(lines, gitStatus(paths[0])+"\t"+paths[0]) + lines = append(lines, status+"\t"+strings.Join(paths, "\t")) } return strings.Join(lines, "\n") } -// changedLines maps each path this branch changed to its whole `git diff --name-status` -// line, keyed the way the diff names it now: the destination path of a rename, and the -// path itself otherwise. A path filter cannot be used for this, because git only pairs a -// rename when both of its paths are in the diff — asking about the destination alone -// reports an addition. -func changedLines() map[string]string { - out, err := exec.Command("git", "diff", "--name-status", "--merge-base", "-M", "origin/main").Output() - if err != nil { - fatalf("git diff --merge-base origin/main failed: %s", err) - } - - lines := map[string]string{} - for line := range strings.SplitSeq(strings.TrimSpace(string(out)), "\n") { - if fields := strings.Split(line, "\t"); len(fields) >= 2 { - lines[fields[len(fields)-1]] = line - } - } - return lines -} - -// gitStatus is the status to assume for a path git does not report as changed: "M" for a -// tracked path and "A" for one git does not know. -func gitStatus(path string) string { - if exec.Command("git", "ls-files", "--error-unmatch", "--", path).Run() == nil { - return "M" - } - return "A" -} - func fatalf(format string, args ...any) { fmt.Fprintf(os.Stderr, format+"\n", args...) os.Exit(1) From 0ef3939849ac78693c565fed8267ac9f7d13414a Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 20 Aug 2026 13:29:26 +0200 Subject: [PATCH 21/21] acc: print the shortest name that names a selected test go test reads a test name as a prefix, so the variants below the one the selection names do not need spelling out: a dir whose every variant runs is named by the dir alone, and an invariant test by its config. One line per selected test now, rather than one per variant of it. --- acceptance/internal/selection/cmd/main.go | 28 ++++++++----- acceptance/selftest/selection/output.txt | 51 ++++++++--------------- 2 files changed, 35 insertions(+), 44 deletions(-) diff --git a/acceptance/internal/selection/cmd/main.go b/acceptance/internal/selection/cmd/main.go index e7835b8bebe..94681587763 100644 --- a/acceptance/internal/selection/cmd/main.go +++ b/acceptance/internal/selection/cmd/main.go @@ -25,6 +25,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "github.com/BurntSushi/toml" @@ -72,22 +73,29 @@ func main() { } } -// variantNames returns the go test names of the selected test: one per variant of its dir -// that the selection covers, or the dir alone when it has no variants. +// variantNames returns the go test names of the selected test. go test reads a name as a +// prefix, so a name is only as long as it needs to be: the dir alone when every variant of +// it runs, and otherwise up to the variant the selection names. func variantNames(root string, test selection.Test) []string { - var filters []string - if test.Filter != "" { - filters = []string{test.Filter} + if test.Filter == "" { + return []string{test.Dir} } + key, _, _ := strings.Cut(test.Filter, "=") var names []string for _, envset := range internal.ExpandEnvMatrix(envMatrix(root, test.Dir), nil, nil) { - if len(envset) == 0 { - // The harness runs a test without variants as a plain subtest of its dir. - return []string{test.Dir} + if !selection.MatchesFilters(envset, []string{test.Filter}) { + continue } - if selection.MatchesFilters(envset, filters) { - names = append(names, test.Dir+"/"+strings.Join(envset, "/")) + // Cut the name after the variable the filter names; the variants below it all run. + for i, kv := range envset { + if name, _, _ := strings.Cut(kv, "="); name == key { + envset = envset[:i+1] + break + } + } + if name := test.Dir + "/" + strings.Join(envset, "/"); !slices.Contains(names, name) { + names = append(names, name) } } return names diff --git a/acceptance/selftest/selection/output.txt b/acceptance/selftest/selection/output.txt index 6828f50b902..63d335b91b9 100644 --- a/acceptance/selftest/selection/output.txt +++ b/acceptance/selftest/selection/output.txt @@ -3,74 +3,57 @@ >>> selection M:acceptance/selftest/basic/script Selected 1 changed tests (limit=50, 0 not selected) - 5 selftest/basic/DATABRICKS_BUNDLE_ENGINE=terraform - 5 selftest/basic/DATABRICKS_BUNDLE_ENGINE=direct + 5 selftest/basic === Adding a test >>> selection A:acceptance/selftest/basic/script Selected 1 changed tests (limit=50, 0 not selected) - 10 selftest/basic/DATABRICKS_BUNDLE_ENGINE=terraform - 10 selftest/basic/DATABRICKS_BUNDLE_ENGINE=direct + 10 selftest/basic === A regenerated golden scores below a changed fixture >>> selection M:acceptance/selftest/basic/output.txt M:acceptance/selftest/diff/script Selected 2 changed tests (limit=50, 0 not selected) - 5 selftest/diff/DATABRICKS_BUNDLE_ENGINE=terraform - 5 selftest/diff/DATABRICKS_BUNDLE_ENGINE=direct - 1 selftest/basic/DATABRICKS_BUNDLE_ENGINE=terraform - 1 selftest/basic/DATABRICKS_BUNDLE_ENGINE=direct + 5 selftest/diff + 1 selftest/basic === Moving a test scores lowest >>> selection R100:acceptance/selftest/old/script:acceptance/selftest/basic/script M:acceptance/selftest/diff/script Selected 2 changed tests (limit=50, 0 not selected) - 5 selftest/diff/DATABRICKS_BUNDLE_ENGINE=terraform - 5 selftest/diff/DATABRICKS_BUNDLE_ENGINE=direct - 1 selftest/basic/DATABRICKS_BUNDLE_ENGINE=terraform - 1 selftest/basic/DATABRICKS_BUNDLE_ENGINE=direct + 5 selftest/diff + 1 selftest/basic === Touching an invariant config runs every invariant test, for that config only >>> selection M:acceptance/bundle/invariant/configs/job.yml.tmpl Selected 5 changed tests (limit=50, 0 not selected) 5 bundle/invariant/continue_293/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl - 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= - 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 - 5 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= - 5 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 + 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl 5 bundle/invariant/migrate/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl - 5 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= - 5 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 + 5 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl === Adding one invariant config and touching another: only the new variant is new >>> selection A:acceptance/bundle/invariant/configs/pipeline.yml.tmpl M:acceptance/bundle/invariant/configs/job.yml.tmpl Selected 10 changed tests (limit=50, 0 not selected) 10 bundle/invariant/continue_293/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl - 10 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN= - 10 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN=1 - 10 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN= - 10 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN=1 + 10 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl + 10 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl 10 bundle/invariant/migrate/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl - 10 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN= - 10 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl/READPLAN=1 + 10 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=pipeline.yml.tmpl 5 bundle/invariant/continue_293/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl - 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= - 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 - 5 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= - 5 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 + 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl + 5 bundle/invariant/destroy_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl 5 bundle/invariant/migrate/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl - 5 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= - 5 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 + 5 bundle/invariant/no_drift/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl === The limit keeps the highest scoring tests >>> selection -limit 3 A:acceptance/selftest/basic/script M:acceptance/selftest/diff/script M:acceptance/bundle/invariant/configs/job.yml.tmpl Selected 3 changed tests (limit=3, 4 not selected) - 10 selftest/basic/DATABRICKS_BUNDLE_ENGINE=terraform - 10 selftest/basic/DATABRICKS_BUNDLE_ENGINE=direct + 10 selftest/basic 5 bundle/invariant/continue_293/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl - 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN= - 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl/READPLAN=1 + 5 bundle/invariant/delete_idempotent/DATABRICKS_BUNDLE_ENGINE=direct/INPUT_CONFIG=job.yml.tmpl