Delete the document a failed save could not write (#250) #256
Merged
JMR-dev
merged 4 commits from 2026-09-06 21:11:14 +00:00
test/publish-delete-arm-real-provider into main
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  JMR-devandClaude Opus 5
|
8105291f6a |
Make the gate name the levels it ran instead of claiming all of them
The closing line was `green at every supported API level`, printed on both paths -- including the one that had just said `NOT COVERED LOCALLY: API 37` two lines above. A false claim, printed by the tool whose entire purpose is to stop false claims reaching CI, on its first run. It now names them: `green on API 33, 34, 35, 36` when the Pixel is absent, and `green on API 33, 34, 35, 36, 37` when it is attached and passed. Nothing else changes. The app/src subtree is untouched, so this exercises the cache scoping from the previous commit: the sweep is skipped as already green and only the JVM gate runs -- which is the whole reason that key was moved off the repo tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
68bd24e54a |
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> |
||
|
JMR-devandClaude Opus 5
|
19e35394e1 |
Bound the conversion against API 35's encode, not API 34's
The API 35 leg of #256 went red on aSaveWritesToTheDocumentTheSystemPickerCreated with a 120 s ComposeTimeoutException on action.saveFile. It was not a cancelled job and not the new teardown: run 34056545386's logcat has 20:05:26.897 FFmpegEngine: ffmpeg ... -c:v libx265 -crf 24 -preset veryfast 20:07:41.693 ConversionWorker: Routing worker_sample.mp4 ... 134.8 s between the encode starting and the next job in the suite, with no cancel between them. The conversion was healthy and still running when the bound fired. CONVERSION_TIMEOUT_MS was 120_000, and its KDoc justified that with "the whole test takes 11.8 s on the API 34 CI leg" -- a real measurement generalised to an API level it was never taken on. Adding a second picker test made this class encode twice, so the second one runs on a more contended emulator and crossed a line that was already marginal. Now 300_000, justified against the 134.8 s, with a note not to re-tighten it from a fast leg's timing. cancelAllWork was SUSPECTED of causing this and did not. A local API 35 run with it passed, which is what sent me to the logcat. pruneWork is kept because it is the narrower call -- only finished records need to go, and cancelling live work is a wider blast radius than teardown in a shared process needs -- and its KDoc now says it fixed nothing rather than claiming a cause it does not have. That correction is the point: the first version of that KDoc asserted a cause from one red CI leg and one green local run on a different machine. A test carrying a confident wrong explanation is the failure mode this whole read has been about. Verified: API 35 at 71/71 failed=0 with the raised bound, and the full gate green. Production is untouched -- git diff origin/main -- app/src/main is empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
JMR-devandClaude Opus 5
|
cbbaf74285 |
Delete the document a failed save could not write (#250)
#226 proved D4's premise -- SAF hands back a document reporting exactly zero bytes, so destinationIsKnownEmpty can answer true -- and then drove the success path, where publish's catch is never entered. So deletePartialOutput had still never run against a real DocumentsProvider; its only assertions were OutputPublisherPublishTest's, against FakeSafProvider under Robolectric. That is the same "asserted only against a fake built to match it" shape #226 was filed to break, one layer down. RecordingPublisher.failOpen makes openDestination return null, which publish turns into error("Could not open destination for writing") AFTER its size probe has run -- so the catch is reached with destinationWasEmpty true on a document DocumentsUI created seconds earlier. Null rather than a throw because openDestination's KDoc says a provider that is present and declines is the half no fake can produce on demand, so that arm is also taken for the first time. Mutation, measured: delete the deletePartialOutput call and this test fails with "publish did not delete the document it could not write". Nothing anywhere went red for that line before. TWO DEAD ACCESSORS #226 LEFT, and the reason is the same one: FixtureDocumentsProvider is declared by the test APK and runs in org.libremediaconverter.test; instrumentation runs in the app's process. A static in the provider is a different object from the one a test can see, so deletedDocumentIds() would have read empty forever, and reset(File) deletes under a filesDir that is not the provider's. Both are removed rather than worked around. That is E7's process wall from a third side, after ACTION_OPEN_DOCUMENT and ActivityScenario. The oracle is the document instead, which crosses the boundary because the app holds a URI grant for it. Still the path rather than the artefact: the size query proves the document existed and was empty moments earlier, and one that no longer answers a query is one something deleted. CLEANUP IS IN TEARDOWN, and the mutation run is why. A failed save keeps its staged file deliberately, so this test ends with a finished job for the next launch to reattach to; its sibling then opened on Converted with no "Choose file" to tap. The first fix tapped Start over at the end of the test body, which does not run when the test fails -- so the mutation run turned one real failure into two, the second looking like an unrelated flake. One cause must produce one red test. Baseline 6 -> 7, with the derived counts in CLAUDE.md, the marker KDoc and status_check.yml moved in the same diff. 71 - 7 is 64, the same gating figure for the third consecutive time, which is how that paragraph goes stale unnoticed. Verified: three API 34 runs at 71/71 failed=0, the mutation red on the right assertion, and the full gate plus pinned actionlint green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |