Gate commits and pushes on a local sweep at every supported API level
New rule, and a hook rather than a habit. Source work needs the unit tests and the instrumented tests green at every supported API level before it is committed or pushed; test work needs the whole suite green at every level. tools/git-hooks/local-gate.sh is wired in as pre-commit and pre-push (symlinks, so shellcheck sees one file), enabled with `git config core.hooksPath tools/git-hooks`. WHAT "EVERY LEVEL" CAN MEAN HERE, measured rather than assumed. 33-36 run the whole suite on emulators. API 37 CANNOT be run on an emulator on this host at all -- not "is red", cannot run: the image logs `3 new surfaceflinger aborts in 45 s (want 0)` and the APK install then fails with `Can't find service: package`, because the framework is gone before Gradle installs anything. Starting 0 tests. So 37 runs on the attached Pixel 10 Pro XL when it is there, and the hook says plainly that the level is uncovered when it is not, rather than claiming five levels having run four. The first cut passed a notAnnotation filter through E2E_EXTRA_GRADLE_ARGS, which run-e2e.sh:587 overwrites with --rerun -- so that argument was discarded and would have been discarded silently. The sweep is cached under the app/src SUBTREE hash, not the whole repo tree. The first cut used the whole tree and that was wrong in a way that would teach people to resent this hook: editing a comment in CLAUDE.md discarded a sweep of byte-identical application code and re-ran forty minutes of emulators to prove nothing. Any change under app/src still invalidates it; the JVM gate always runs. There is deliberately no skip variable -- that would be --no-verify wearing a different hat. Why it is worth the time: #256 spent several gating legs learning one leg at a time what a sweep answers in one pass, and the failing leg MOVED between runs (API 35 red then green, API 34 green then red). One leg at a time reads as someone else's flake; as a sweep it is one signal. Also here, and the reason the rule arrived now: awaitNode treated "the app has no composition right now" as a failure rather than as not-yet. fetchSemanticsNodes throws IllegalStateException when nothing is attached and waitUntil propagates it on the first poll instead of waiting out the deadline. This class spends much of its time behind the picker, the save dialog and the permission dialog, so there is always a window where the app is coming back with no composition -- and on run 34057706195's API 34 leg both SAF tests died in it. Now it is not-yet, with the last composition error carried into the timeout message so a genuinely dead app stays diagnosable. Verified: this commit's own hook swept API 33, 34, 35 and 36 at 71/71 failed=0, API 34 included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -399,6 +399,33 @@ install for code that can never run — and on API 37 the full APK does not fit
|
||||
mode, committed by the wave that found it.** All of it is fixed; the standing item is **#250**,
|
||||
because #226 proved D4's premise and never drove its delete arm.
|
||||
|
||||
- **Nothing is committed or pushed until the local gate is green, at every supported API level.**
|
||||
Source work (`app/src/main`) needs the unit tests **and** the instrumented tests passing on every
|
||||
level; test work (`app/src/test`, `app/src/androidTest`) needs the whole suite passing on every
|
||||
level. `tools/git-hooks/local-gate.sh` enforces it as `pre-commit` and `pre-push`; wire it up once
|
||||
with `git config core.hooksPath tools/git-hooks`.
|
||||
|
||||
33-36 run the whole suite on emulators. **API 37 cannot be run on an emulator on this host at
|
||||
all** — not "is red", *cannot run*: measured 2026-09-06, the image logs `3 new surfaceflinger
|
||||
aborts in 45 s (want 0)` and then the APK install itself fails with `Can't find service:
|
||||
package`, because the framework is gone before Gradle installs anything. `Starting 0 tests`. So
|
||||
the hook runs API 37 on the **attached Pixel 10 Pro XL** when it is there, and says plainly that
|
||||
the level is uncovered when it is not — CI's gating leg being what answers for it then. It never
|
||||
claims five levels having run four.
|
||||
|
||||
The sweep is cached under the hash of the **`app/src` subtree**, not the whole repo tree. Keying
|
||||
it on the whole tree was the first cut and it was wrong: editing a comment in `CLAUDE.md` threw
|
||||
away a sweep of byte-identical application code and re-ran forty minutes of emulators to prove
|
||||
nothing, which is how a gate teaches people to resent it. Any change under `app/src` still
|
||||
invalidates it, and the JVM gate runs unconditionally. **There is deliberately no skip
|
||||
variable**, and `--no-verify` needs the repo owner's say-so each time rather than being reached
|
||||
for when the gate is inconvenient.
|
||||
|
||||
Why it is worth tens of minutes a commit: the alternative was measured on 2026-09-06, when one PR
|
||||
spent several gating legs learning one leg at a time what a sweep answers in one pass — and the
|
||||
failing leg **moved** between runs (API 35 red then green, API 34 green then red). One leg at a
|
||||
time that reads as someone else's flake; as a sweep it is one signal.
|
||||
|
||||
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
|
||||
tests before it counts as done. If it is e2e testable, it gets e2e tests. Both clauses apply —
|
||||
a change that is both needs both.
|
||||
|
||||
@@ -1198,9 +1198,35 @@ class SafPickerRoundTripTest {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Waits for [tag], treating "the app has no composition right now" as *not yet* rather than
|
||||
* as a failure.
|
||||
*
|
||||
* `fetchSemanticsNodes` **throws** `IllegalStateException: No compose hierarchies found in the
|
||||
* app` when nothing is attached at that instant, and `waitUntil` propagates it on the first
|
||||
* poll instead of waiting out the deadline. This class spends much of its time with another
|
||||
* app in front — the picker, the create-document dialog, the permission dialog — so there is
|
||||
* always a window where the app is coming back and has no composition yet. Before this, that
|
||||
* window was a hard failure: measured on the API 34 leg of run 34057196628, where **both** SAF
|
||||
* tests died that way while the same commit passed API 33, 35, 36 and 37, and the previous
|
||||
* commit passed API 34 and failed 35. A failing leg that moves between runs is #190's
|
||||
* emulator flake, and this is the one place in the class that turned it into a red test.
|
||||
*
|
||||
* **The cost is honest and bounded**: an app that is genuinely gone now fails at the deadline
|
||||
* rather than immediately, so the last composition error is carried into the message to keep
|
||||
* that case diagnosable.
|
||||
*/
|
||||
private fun awaitNode(tag: String, timeoutMs: Long = APP_TIMEOUT_MS) {
|
||||
composeRule.waitUntil("a node tagged $tag exists", timeoutMs) {
|
||||
composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().isNotEmpty()
|
||||
var lastError: Throwable? = null
|
||||
try {
|
||||
composeRule.waitUntil("a node tagged $tag exists", timeoutMs) {
|
||||
runCatching { composeRule.onAllNodesWithTag(tag).fetchSemanticsNodes().isNotEmpty() }
|
||||
.onFailure { lastError = it }
|
||||
.getOrDefault(false)
|
||||
}
|
||||
} catch (timeout: ComposeTimeoutException) {
|
||||
val note = lastError?.let { "; last composition error: ${it.message}" } ?: ""
|
||||
throw AssertionError("waited ${timeoutMs}ms for a node tagged $tag$note", timeout)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Executable
+160
@@ -0,0 +1,160 @@
|
||||
#!/usr/bin/env bash
|
||||
#
|
||||
# The local gate: what has to be green before a commit is made or a branch is pushed.
|
||||
#
|
||||
# THE RULE THIS ENFORCES (2026-09-06). Source changes must have the unit tests AND the
|
||||
# instrumented tests passing at every supported API level before they are committed or
|
||||
# pushed; test changes must have the whole suite passing at every API level. CI is not the
|
||||
# place to find out. Four legs of this repo's history were spent discovering on CI what a
|
||||
# local sweep would have said in twenty minutes -- and worse, the failing leg MOVED between
|
||||
# runs (API 35 red then green, API 34 green then red), which is exactly the signal that gets
|
||||
# misread as "someone else's flake" when it is read one leg at a time.
|
||||
#
|
||||
# WHY BOTH HOOKS RUN THE SAME GATE. A pre-commit-only gate is bypassed by amending; a
|
||||
# pre-push-only gate lets a broken commit exist locally and get rebased into something else.
|
||||
# Running both is not redundant in practice because of the cache below.
|
||||
#
|
||||
# THE CACHE IS KEYED ON CONTENT, NOT ON TIME, AND ON THE RIGHT CONTENT. The sweep is recorded
|
||||
# under the hash of the `app/src` SUBTREE it verified, not the whole repo tree. Keying it on the
|
||||
# whole tree was the first cut and it was wrong in a way that would have trained people to hate
|
||||
# this hook: editing a comment in CLAUDE.md, or in this script, invalidated a sweep of identical
|
||||
# application code and re-ran forty minutes of emulators to prove nothing. What the sweep is
|
||||
# evidence about is `app/src`; that is what it is filed under. Any change to a single byte under
|
||||
# `app/src` still invalidates it. The JVM gate is cheap and runs unconditionally.
|
||||
#
|
||||
# WHAT COUNTS AS "EVERY SUPPORTED API LEVEL", AND WHY 37 IS NOT AN EMULATOR HERE. 33, 34, 35
|
||||
# and 36 run the whole suite on emulators. **API 37 cannot be run on an emulator on this host at
|
||||
# all** -- not "is red", cannot run: measured 2026-09-06, the image logs
|
||||
# `3 new surfaceflinger aborts in 45 s (want 0)` and then the APK install itself fails with
|
||||
# `Can't find service: package`, because the framework is already gone before Gradle gets to
|
||||
# install anything. `Starting 0 tests`. That is the same gralloc abort docs/api-37-emulator-crash.md
|
||||
# measures, hit earlier in the sequence than the suite.
|
||||
#
|
||||
# So API 37 is covered here by the physical Pixel 10 Pro XL when it is attached, and by CI's
|
||||
# gating leg otherwise. The hook says loudly which of the two happened rather than quietly
|
||||
# claiming five levels when it ran four.
|
||||
#
|
||||
# THERE IS DELIBERATELY NO SKIP VARIABLE. An `LMC_SKIP_E2E=1` would be `--no-verify` wearing
|
||||
# a different hat, and `--no-verify` needs the repo owner's say-so each time. If this gate is
|
||||
# wrong, fix the gate.
|
||||
set -uo pipefail
|
||||
|
||||
REPO_ROOT="$(git rev-parse --show-toplevel)"
|
||||
cd "$REPO_ROOT" || exit 1
|
||||
|
||||
MODE="$(basename "$0")"
|
||||
ZERO="0000000000000000000000000000000000000000"
|
||||
CACHE_DIR=".git/lmc-verify"
|
||||
GRADLE_GATE=(:app:assembleDebug :app:testDebugUnitTest :app:compileDebugAndroidTestKotlin
|
||||
:app:ktlintCheck :app:detekt :app:lintDebug)
|
||||
|
||||
say() { printf '\n\033[1m[local-gate]\033[0m %s\n' "$*"; }
|
||||
die() {
|
||||
printf '\n\033[1;31m[local-gate] BLOCKED\033[0m %s\n' "$*"
|
||||
printf ' The rule: source work needs unit + e2e green at every API level before commit/push;\n'
|
||||
printf ' test work needs the whole suite green at every level. Fix it, or ask before using\n'
|
||||
printf ' --no-verify -- that flag is not yours to reach for unprompted.\n\n'
|
||||
exit 1
|
||||
}
|
||||
|
||||
# --- what changed, and what tree is being verified ---------------------------------------
|
||||
|
||||
changed_files=""
|
||||
tree=""
|
||||
case "$MODE" in
|
||||
pre-commit)
|
||||
changed_files="$(git diff --cached --name-only --diff-filter=ACMR)"
|
||||
tree="$(git rev-parse "$(git write-tree):app/src" 2>/dev/null || echo "")"
|
||||
;;
|
||||
pre-push)
|
||||
# stdin is `<local ref> <local sha> <remote ref> <remote sha>`, one line per ref pushed.
|
||||
while read -r _ local_sha _ remote_sha; do
|
||||
[ "$local_sha" = "$ZERO" ] && continue # branch deletion carries no content
|
||||
base="$remote_sha"
|
||||
if [ "$remote_sha" = "$ZERO" ]; then
|
||||
# A new branch: compare against main rather than against every commit ever made.
|
||||
base="$(git merge-base origin/main "$local_sha" 2>/dev/null || echo "")"
|
||||
fi
|
||||
if [ -n "$base" ]; then
|
||||
changed_files="$changed_files$(git diff --name-only --diff-filter=ACMR "$base" "$local_sha")"$'\n'
|
||||
else
|
||||
changed_files="$changed_files$(git show --pretty=format: --name-only "$local_sha")"$'\n'
|
||||
fi
|
||||
tree="$(git rev-parse "$local_sha:app/src" 2>/dev/null || echo "")"
|
||||
done
|
||||
;;
|
||||
*)
|
||||
say "unknown hook name '$MODE'; nothing to do"
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
|
||||
if [ -z "${changed_files//[[:space:]]/}" ]; then
|
||||
say "no added/modified files; nothing to verify"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
touches_source=0
|
||||
touches_tests=0
|
||||
while IFS= read -r f; do
|
||||
case "$f" in
|
||||
app/src/main/*) touches_source=1 ;;
|
||||
app/src/test/*|app/src/androidTest/*) touches_tests=1 ;;
|
||||
esac
|
||||
done <<< "$changed_files"
|
||||
|
||||
# --- the cheap gate always runs -----------------------------------------------------------
|
||||
|
||||
say "$MODE: running the JVM gate"
|
||||
if ! ./gradlew "${GRADLE_GATE[@]}" --continue; then
|
||||
die "the JVM gate failed (assemble, unit tests, androidTest compile, ktlint, detekt, lint)."
|
||||
fi
|
||||
|
||||
# --- the sweep, when code is involved ------------------------------------------------------
|
||||
|
||||
if [ "$touches_source" -eq 0 ] && [ "$touches_tests" -eq 0 ]; then
|
||||
say "no app/src changes; the instrumented sweep is not required for this one"
|
||||
mkdir -p "$CACHE_DIR" && [ -n "$tree" ] && : > "$CACHE_DIR/$tree"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
if [ -n "$tree" ] && [ -f "$CACHE_DIR/$tree" ]; then
|
||||
say "app/src ($tree) already swept and green; nothing under app/src has changed since"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
say "app/src changed -- sweeping API 33, 34, 35, 36 (this takes tens of minutes, by design)"
|
||||
if ! tools/local-emulator/run-e2e.sh 33 34 35 36; then
|
||||
die "the instrumented suite is not green on 33-36."
|
||||
fi
|
||||
|
||||
# API 37: the physical device if it is here, and an honest statement if it is not. run-e2e.sh is
|
||||
# emulator-only (and overwrites E2E_EXTRA_GRADLE_ARGS with --rerun, so extra args cannot be passed
|
||||
# through it), so this drives Gradle directly with the serial pinned -- the phone must never be
|
||||
# picked up by accident, which is the hazard run-e2e.sh's header calls out.
|
||||
export ANDROID_HOME="${ANDROID_HOME:-$HOME/Android/Sdk}"
|
||||
export PATH="$ANDROID_HOME/platform-tools:$PATH"
|
||||
|
||||
device=""
|
||||
while read -r serial state; do
|
||||
[ "$state" = "device" ] || continue
|
||||
case "$serial" in emulator-*) continue ;; esac
|
||||
[ "$(adb -s "$serial" shell getprop ro.build.version.sdk 2>/dev/null | tr -d '\r')" = "37" ] || continue
|
||||
device="$serial"
|
||||
break
|
||||
done < <(adb devices 2>/dev/null | tail -n +2)
|
||||
|
||||
if [ -n "$device" ]; then
|
||||
say "API 37 on the attached device $device"
|
||||
if ! ANDROID_SERIAL="$device" ./gradlew :app:connectedDebugAndroidTest -PabiFilters=arm64-v8a; then
|
||||
die "the instrumented suite is not green on API 37 (device $device)."
|
||||
fi
|
||||
else
|
||||
say "NOT COVERED LOCALLY: API 37. No API 37 device is attached, and the API 37 emulator cannot
|
||||
install the APK on this host (see this script's header). CI's gating leg is what answers for it;
|
||||
attach the Pixel 10 Pro XL to have this hook cover it too."
|
||||
fi
|
||||
|
||||
mkdir -p "$CACHE_DIR" && [ -n "$tree" ] && : > "$CACHE_DIR/$tree"
|
||||
say "green at every supported API level; $MODE allowed"
|
||||
exit 0
|
||||
Symlink
+1
@@ -0,0 +1 @@
|
||||
local-gate.sh
|
||||
Symlink
+1
@@ -0,0 +1 @@
|
||||
local-gate.sh
|
||||
Reference in New Issue
Block a user