[WIP] Smartly select what tests to run on PR iterations

[janniklasrose]

Content-fingerprint result reuse for PR iterations

Status: design, for review before implementation
Depends on: Stage 1 (92d509b924, testmask docs-inert skip) — shipped
Author: drafted by Isaac for Jan

Problem

A PR is not one CI run — it's many. During review a branch gets pushed
repeatedly: address a comment, fix lint, add a changelog fragment, merge
origin/main. Each synchronize event re-runs the full test matrix and
re-dispatches the expensive cross-org integration suite
(push.yml → trigger-tests → cli-isolated-pr.yml), even when the new
iteration didn't touch anything the integration tests exercise.

That per-iteration re-run is the waste this stage removes. It is invisible in
main's history because the repo squash-merges (0 merge commits in the last
100 on main), so it can't be measured from main — but every reviewed PR
lives it.

Why Stage 1 does not already cover this

Stage 1 (testmask) diffs the cumulative merge-base range
base...head, so it answers "did this PR ever touch target T?" — not "did
this iteration touch T?". Concretely:

• iteration 1 edits experimental/ssh/foo.go → integration runs (correct)
• iteration 2 only tweaks a doc in response to review

On iteration 2, the cumulative diff still shows the ssh change, so testmask
still emits test-exp-ssh and integration re-runs — despite iteration 2 adding
nothing that ssh tests cover. Stage 1 cannot skip it; it has no memory of what
already passed.

Measured over the last 80 real commits on main: ~90% select test (and thus
integration). The trigger set is broad, so "this iteration changed nothing
integration-relevant" is common during a review cycle — exactly the gap.

Approach: content-address the test inputs

Instead of diffing commit-to-commit, hash the content of a target's
inputs and reuse a prior green result when the hash matches.

For target T:

fingerprint(T) = SHA256 over the sorted list of "(path, blob_sha)"
                 for every tracked file matching T's input set,
                 at the current commit.

git ls-tree -r --format='%(objectname) %(path)' <commit> -- <paths...> yields
path+blob-sha directly (verified on git 2.50). Blob shas are already computed by
git, so this is cheap and needs no file reads.

Protocol:

1. When target T passes at fingerprint F, record a marker keyed by F.
2. On any later run, compute F. If a pass marker for F exists → skip T
   and reuse the green result.

Why this shape is right:

• Covers iterations — iteration 2 that leaves T's inputs byte-identical has
  the same F ⇒ skip. This is the case Stage 1 structurally cannot handle.
• Force-push / rebase immune, for free — it's content-addressed, not
  history-addressed. This is the "snapshot" robustness the original ask hoped
  for, without depending on pull_request.synchronize's fragile before/after
  SHAs (which dangle under force-push).
• Subsumes the docs case — a docs-only iteration doesn't change any F.

Force-pushes (rebase / amend / squash / reset)

Force-pushes are covered by design — content-addressing is the reason we chose
this shape over a diff-from-last-iteration scheme. There are two layers:

Layer 1 — computation. F hashes (path, blob-sha) from git ls-tree at a
single commit. It reads no parent, commit SHA, author, or diff, so rewriting
history cannot change it. Verified: re-committing a commit's exact tree onto a
different parent (a simulated force-push) produces a new commit SHA but a
byte-identical F. This is why we do not key off
pull_request.synchronize's before/after SHAs — the before SHA dangles
after a force-push, breaking any history-relative scheme.

Layer 2 — cache readback. The marker key is pass-<T>-<F> (no SHA in it),
and GitHub Actions cache is scoped by git ref, not commit. A force-push keeps
the same ref, so iteration N+1 still reads the namespace iteration N wrote.

Edge cases, all correct:

Force-push kind	Effect on F	Outcome
Pure history rewrite (amend msg, reorder, squash; tree unchanged)	unchanged	reuse — correct, and the win
Rebase onto a moved main / conflict resolution (tree changes)	changes	miss → run — correct
Rebase that pulls in new tooling (Go bump, go.sum)	changes iff tooling is in the input set	run — only correct because the design mandates tooling ∈ F
Reset/rewrite back to earlier content	equals an earlier F	reuse of the older marker — correct bonus (a diff scheme would re-run)

The one real hazard is the third row: a rebase can silently change the
environment with no product-code diff. It is covered only because F
includes Go version, go.sum, and the harness/tool version (see safety point 3
below). Without that, a force-push-rebase could wrongly reuse.

Non-hazard: cache eviction (10 GB LRU / 7-day idle) only ever causes a miss →
safe re-run, force-push or not. Markers are written only by a genuinely
green run on the ref, never seeded speculatively, so nothing stale can be
resurrected by a rewrite-then-restore.

The safety asymmetry (the crux)

testmask is a trigger: over-running wastes CI, under-running is safe because
a catch-all backstops it. A result-reuse cache is the opposite:
under-covering the fingerprint means skipping a run that would have caught a
regression. A missed input is a correctness bug, not a cost bug.

Consequences for the design:

1. The fingerprint must hash the true input set, not testmask's trigger
   globs. Those globs are deliberately conservative triggers; e.g. test-acc
   runs --packages ./acceptance/... with no -run filter, so the real
   acceptance coverage is broader than any single target's sources:. The
   fingerprint's input set must be a superset of what actually affects the
   outcome. When unsure, hash more (a superfluous input only causes a false
   cache-miss = a safe re-run).

2. Default to miss. Any error computing F (git failure, unknown path,
   tooling change) ⇒ run the tests. Never skip on uncertainty.

3. Include the tooling in F. Go version, go.sum, the test harness, the
   fingerprint tool's own version. A test-runner change can flip a result with
   zero product-code change; if it's not in F, reuse is unsound.

4. Bound blast radius on rollout — see below.

Scaling: fingerprint compute and cache size

A large PR does not stress this design, because nothing here scales with the
diff. Three distinct "sizes", only the first has any cost, and it answers to
repo size, not PR size:

Quantity	Scales with	Worst case (this repo)	Measured cost
Fingerprint compute	files in the target's input tree (not the diff)	whole tree, 9,663 files	~20 ms warm CPU (0.6 s cold ls-tree wall)
Fingerprint output	nothing — constant	always	65 bytes (sha256 hex + \n)
Cache marker	nothing — a presence key	always	empty value; all info in the key pass-<T>-<F>

Verified: the largest PR in the last 200 commits (375 files changed)
fingerprinted in the same ~20 ms as a one-file PR. ls-tree reads the target's
input tree at HEAD and never looks at the diff, so diff size is irrelevant to
compute.

Cache footprint is bounded by fingerprint churn, not files-per-PR:
cardinality is targets × distinct-fingerprints-ever-seen. A 375-file PR still
mints exactly one fingerprint per target per iteration, same as a one-file
PR. With ~5 targets and one new fingerprint per meaningful iteration, even a busy
week is single-digit MB against GitHub's 10 GB/repo budget — and GitHub already
GCs it (10 GB LRU + 7-day idle). An evicted or stale marker only ever causes a
safe cache miss (→ re-run), never a wrong skip.

The one repo-scale caveat: cold git ls-tree over the whole tree was ~0.6 s
wall (60 ms warm) at 9.6k files; it is O(files-in-input-set), so a
million-file monorepo would grow it. Non-issue at current scale. If it ever
mattered: fingerprint scoped targets (ssh's input tree is 5 KB of ls-tree vs
954 KB whole-repo) rather than the whole tree, and memoize ls-tree per
commit SHA.

Prior art: internal.ChangedTests (#5880)

PR #5880 (pietern, extract-changed-tests, open) extracts the
DATABRICKS_TEST_SKIPLOCAL=withchanged detection into a reusable
acceptance/internal.ChangedTests. It is orthogonal (it selects which
acceptance tests inside a run execute; testmask/this stage select whether the
job runs at all) and lives in the root module, so tools/testmask cannot
import it (dependency would invert). But its dependency model is exactly the
cross-directory correctness problem this stage shares, and should be mirrored:

• a shared parent script.prepare / script.cleanup / test.toml affects its
  whole subtree → those files must be in the fingerprint input set of every
  descendant target;
• a change under acceptance/bin/** affects every test → in the common input
  set;
• an unowned shared file (a sourced _script, a fixture read via $TESTDIR/..)
  affects its subtree.

If this stage ever moves detection into the root module, ChangedTests becomes
directly reusable. For now, mirror the rules and cite #5880.

Storage

GitHub Actions cache (already the substrate for the Go build cache in
setup-build-environment/action.yml):

• key pass-<T>-<F>; presence = "T passed at F".
• Scope works in our favour: a PR's iterations all push the same branch,
  and a branch reads caches it wrote plus the base branch's — so iteration N+1
  reads iteration N's marker, and any branch reads markers seeded by main.
  Sibling PR branches can't read each other's, which is correct (no
  cross-contamination).
• No new secrets/infra for the unit/acceptance test-* jobs.

The integration-test wrinkle (cross-org)

Unit/acceptance jobs run in databricks/cli and can record their own markers
inline. Integration is different: it's dispatched to
databricks-eng/eng-dev-ecosystem and the result comes back asynchronously
as the Integration Tests check written onto the head SHA. So the CLI repo
doesn't directly observe the pass.

Options (pick during review):

• (A) eng-dev records the marker. The integration workflow computes F for
  the SHA it tested and saves pass-integration-<F> to the CLI repo's cache on
  success. Most direct; needs a change in the other repo + cache-write scope.
• (B) CLI-side listener. A check_run.completed workflow in databricks/cli
  fires when Integration Tests concludes success, recomputes F for that SHA,
  and saves the marker. Keeps the fingerprint definition authoritative in this
  repo; no eng-dev change. Slightly more moving parts (one listener workflow).

Either way the skip decision stays in databricks/cli: trigger-tests
computes F and, on a marker hit, posts the synthetic green Integration Tests
check (reusing the exact path integration-trigger already uses for the
docs-skip) instead of dispatching. Recommendation: (B) — keeps the
fingerprint contract in one repo.

Rollout / guardrails

• Shadow mode first. Compute F, log hit/miss and what would have been
  skipped, but still run everything. Validate hit-rate and zero false-hits over
  a week before enforcing.
• Enforce unit/acceptance before integration. The inline test-* jobs are
  lower-stakes and self-contained; prove the mechanism there first, then extend
  to the cross-org integration path.
• log() every skip with T and F, and count skips — silent truncation
  reads as "covered" when it didn't (the acc: make changed-test detection reusable #5880 maxChangedLocalTests "log what
  was dropped" discipline).
• Kill switch. A repo variable / label forces full run (e.g.
  full-ci), for when a maintainer distrusts a reuse.
• Never skip on main / merge_group. Seed markers there, but always run, so
  the authoritative branch is never reused-into.

Verified end-to-end (fingerprint primitive)

tools/fingerprint/ implements the primitive: Fingerprint(entries) (pure,
order-independent, content- and rename-sensitive, collision-safe via
length-prefixed encoding) + TreeEntriesAt(commit, pathspecs) over
git ls-tree. Demonstrated on real main history:

Iteration	ssh input fingerprint	Decision
across an ssh commit (d6636f4d51)	changes	ssh integration runs ✓
across a README-only commit (9e81da898)	stable	ssh integration skipped ✓
across a postgres/bundle commit (681f8b098e)	stable	ssh integration skipped ✓

The stable cases are exactly what Stage 1 (cumulative diff) cannot skip once the
PR has touched ssh at all — the concrete Stage-2 win.

Design coupling this surfaced: a naive whole-tree fingerprint for the broad
test target flips on a README change (README is in the tree), which would
defeat the docs skip. So the test target's fingerprint input set must
exclude Stage 1's inert paths — Stage 1's inertFiles/inertPrefixes becomes
shared input to Stage 2. This answers open-question #2: per-target input sets,
with the broad target computed as "whole tree minus inert".

Open questions for review

1. Integration marker: (A) eng-dev writes vs (B) CLI-side listener?
   (Recommend B.)
2. Fingerprint granularity: per-target (5 markers) — confirmed sufficient, or do
   we want per-acceptance-dir (finer, closer to acc: make changed-test detection reusable #5880, but only helps if we also
   split the jobs)?
3. Enforce set: start with just integration (biggest cost), or all test-*?
4. Do we fold this on top of acc: make changed-test detection reusable #5880 landing, or keep fully independent?

Non-goals

• Changing which tests run inside a job (that's acc: make changed-test detection reusable #5880's layer).
• Caching test artifacts/outputs — we cache only the boolean "passed at F".
• Any change to main / merge_group behaviour beyond seeding markers.

[eng-dev-ecosystem-bot]

Integration test report

Commit: 04ad594

Run: 29830096425

	Env	💚​RECOVERED	🙈​SKIP	✅​pass	🙈​skip	Time
💚​	aws linux	4	4	227	1127	3:27
💚​	aws windows	4	4	229	1125	3:58
💚​	aws-ucws linux	4	4	314	1044	5:03
💚​	aws-ucws windows	4	4	316	1042	3:27
💚​	azure linux	4	4	227	1126	3:06
💚​	azure windows	4	4	229	1124	2:38
💚​	azure-ucws linux	4	4	316	1041	5:09
💚​	azure-ucws windows	4	4	318	1039	3:39
💚​	gcp linux	4	4	226	1128	2:52
💚​	gcp windows	4	4	228	1126	2:34

8 interesting tests: 4 RECOVERED, 4 SKIP

	Test Name	aws linux	aws windows	aws-ucws linux	aws-ucws windows	azure linux	azure windows	azure-ucws linux	azure-ucws windows	gcp linux	gcp windows
💚​	TestAccept	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R
🙈​	TestAccept/bundle/invariant/no_drift	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S
🙈​	TestAccept/bundle/resources/vector_search_endpoints/drift/recreated_same_name	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S
🙈​	TestAccept/bundle/resources/vector_search_indexes/recreate/embedding_dimension	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S
🙈​	TestAccept/ssh/connection	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S	🙈​S
💚​	TestFetchRepositoryInfoAPI_FromRepo	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R
💚​	TestFetchRepositoryInfoAPI_FromRepo/root	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R
💚​	TestFetchRepositoryInfoAPI_FromRepo/subdir	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R	💚​R