diff --git a/CLAUDE.md b/CLAUDE.md index 572081e..77bf98f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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. diff --git a/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt b/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt index f5af8d2..7121e33 100644 --- a/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt +++ b/app/src/androidTest/java/org/libremediaconverter/saf/SafPickerRoundTripTest.kt @@ -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) } } diff --git a/tools/git-hooks/local-gate.sh b/tools/git-hooks/local-gate.sh new file mode 100755 index 0000000..fcb1afd --- /dev/null +++ b/tools/git-hooks/local-gate.sh @@ -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 ` `, 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 diff --git a/tools/git-hooks/pre-commit b/tools/git-hooks/pre-commit new file mode 120000 index 0000000..572573a --- /dev/null +++ b/tools/git-hooks/pre-commit @@ -0,0 +1 @@ +local-gate.sh \ No newline at end of file diff --git a/tools/git-hooks/pre-push b/tools/git-hooks/pre-push new file mode 120000 index 0000000..572573a --- /dev/null +++ b/tools/git-hooks/pre-push @@ -0,0 +1 @@ +local-gate.sh \ No newline at end of file