acc: replace DATABRICKS_TEST_SKIPLOCAL with DATABRICKS_TEST_SELECT_CHANGED=N

.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:

Taskfile.yml

@@ -550,6 +550,15 @@ 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. -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 test ./acceptance -run '^TestAccept$' -v -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,12 +768,16 @@ tasks:
           --packages "./acceptance ./integration/..." \
           -- -parallel 4 -timeout=12h -short
 
-  integration-short-skiplocal:
-    desc: Run integration tests for PRs (skips CloudSlow and tests with testserver coverage)
+  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
+    # and falls back to integration-short, the full cloud suite, if it is missing).
+    aliases: [integration-short-skiplocal]
     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 \

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,6 +142,11 @@ func TestInprocessMode(t *testing.T) {
 	if os.Getenv("CLOUD_ENV") != "" {
 		t.Skip("No need to run this as integration test.")
 	}
+	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 " + selection.EnvVar)
+	}
 
 	// Uncomment to load  ~/.databricks/debug-env.json to debug integration tests
 	// testutil.LoadDebugEnvIfRunFromIDE(t, "workspace")
@@ -346,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)
@@ -445,25 +457,39 @@ 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, err := selection.ParseLimit(os.Getenv(selection.EnvVar))
+	require.NoError(t, err)
+	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 selection.EnvVar 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 {
+		// 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
 
+	// 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
@@ -518,7 +544,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)
 			if skipReason != "" {
 				skippedDirs += 1
 				t.Skip(skipReason)
@@ -572,9 +598,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 selection.EnvVar, 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)
 							}
@@ -625,23 +651,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
 }
 
@@ -654,15 +666,7 @@ 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.
-// 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 {
-		if _, ok := changedTests[dir]; !ok {
-			return "Disabled via DATABRICKS_TEST_SKIPLOCAL=" + SkipLocalWithChanged + " in " + configPath
-		}
-	}
-
+func getSkipReason(config *internal.TestConfig, configPath string) string {
 	if Forcerun {
 		return ""
 	}
@@ -1058,18 +1062,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 selection.MatchesFilters).
 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 !selection.MatchesFilters(testEnv, envFilters) {
+		t.Skipf("Skipping because test environment (%s) does not match filters (%s)",
+			strings.Join(testEnv, " "), strings.Join(envFilters, " "))
 	}
 }
 
@@ -1293,6 +1292,23 @@ 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}
+	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
+}
+
 // 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 {

acceptance/internal/selection/cmd/main.go

@@ -0,0 +1,143 @@
+// Command cmd prints the acceptance tests that DATABRICKS_TEST_SELECT_CHANGED would run,
+// 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
+//
+// 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:
+//
+//	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 (
+	"flag"
+	"fmt"
+	"os"
+	"path/filepath"
+	"slices"
+	"strings"
+
+	"github.com/BurntSushi/toml"
+
+	"github.com/databricks/cli/acceptance/internal"
+	"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()
+
+	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)
+	}
+	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 _, test := range result.Selected {
+		for _, name := range variantNames(*root, test) {
+			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")
+	}
+}
+
+// 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 {
+	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 !selection.MatchesFilters(envset, []string{test.Filter}) {
+			continue
+		}
+		// 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
+}
+
+// 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 {
+	lines := make([]string, 0, len(args))
+	for _, arg := range args {
+		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)
+}