Delete the document a failed save could not write (#250) #256

Merged
JMR-dev merged 4 commits from test/publish-delete-arm-real-provider into main 2026-09-06 21:11:14 +00:00
JMR-dev commented 2026-09-06 19:49:47 +00:00 (Migrated from github.com)

Closes #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.

The forcing condition

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. Nothing is simulated: URI, grant, provider and delete are all real.

Null rather than a throw because openDestination's own KDoc says a provider that is present and declines is the half no fake can produce on demand. That arm is taken here for the first time.

Mutation, measured. Remove if (destinationWasEmpty) deletePartialOutput(destination, failure) 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, for one reason

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() — added in #226 for exactly this test — 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 refusing a self-registered provider and ActivityScenario refusing a test-APK Activity.

The oracle is the document instead, which crosses the boundary because the app holds a URI grant for it. This is still the path rather than the artefact: the size assertion proves the document existed and was empty moments earlier, and a document that no longer answers a query is one something deleted. Nothing else in this app deletes SAF documents.

Cleanup is in teardown, and the mutation run is why

A failed save keeps its staged file — deliberately, since it may be the only copy of an hour of transcoding — 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: a 30 s timeout in a test that had nothing wrong with it.

The first fix tapped "Start over" at the end of the test body. That does not run when the test fails — so the mutation run turned one real failure into two, the second looking like an unrelated flake. Exactly the confusion corrected on #108 earlier this week. Cleanup is now headless and in @After, so one cause produces one red test.

A green run would never have shown this. Only the deliberate failure did.

Counts

Baseline 6 → 7, with the derived figures in CLAUDE.md, the marker KDoc and status_check.yml moved in the same diff. The new test carries @FailsOnEmulatorApi37 by inheritance — it opens the same picker plus a second DocumentsUI dialog — and that is recorded as unmeasured rather than dressed up.

Note 71 − 7 = 64, the same gating figure for the third consecutive time. That is precisely how the paragraph goes stale unnoticed, which #251 was about.

Verification

  • Three API 34 runs at expected: 71, received: 71, failed: 0, completed cleanly: yes
  • Mutation red on the intended assertion
  • assembleDebug + testDebugUnitTest + compileDebugAndroidTestKotlin + ktlintCheck + detekt + lintDebug green; pinned actionlint clean
  • git diff app/src/main empty — no production change in this PR
Closes #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. ## The forcing condition `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. Nothing is simulated: URI, grant, provider and delete are all real. Null rather than a throw because `openDestination`'s own KDoc says a provider that is present and declines is the half no fake can produce on demand. That arm is taken here for the first time. **Mutation, measured.** Remove `if (destinationWasEmpty) deletePartialOutput(destination, failure)` 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, for one reason `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()` — added in #226 for exactly this test — 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` refusing a self-registered provider and `ActivityScenario` refusing a test-APK Activity. The oracle is the **document** instead, which crosses the boundary because the app holds a URI grant for it. This is still the path rather than the artefact: the size assertion proves the document existed and was empty moments earlier, and a document that no longer answers a query is one something deleted. Nothing else in this app deletes SAF documents. ## Cleanup is in teardown, and the mutation run is why A failed save **keeps** its staged file — deliberately, since it may be the only copy of an hour of transcoding — 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: a 30 s timeout in a test that had nothing wrong with it. The first fix tapped "Start over" at the end of the test body. **That does not run when the test fails** — so the mutation run turned one real failure into two, the second looking like an unrelated flake. Exactly the confusion corrected on #108 earlier this week. Cleanup is now headless and in `@After`, so **one cause produces one red test**. A green run would never have shown this. Only the deliberate failure did. ## Counts Baseline **6 → 7**, with the derived figures in `CLAUDE.md`, the marker KDoc and `status_check.yml` moved in the same diff. The new test carries `@FailsOnEmulatorApi37` by **inheritance** — it opens the same picker plus a second DocumentsUI dialog — and that is recorded as unmeasured rather than dressed up. Note **71 − 7 = 64**, the same gating figure for the third consecutive time. That is precisely how the paragraph goes stale unnoticed, which #251 was about. ## Verification - Three API 34 runs at `expected: 71, received: 71, failed: 0, completed cleanly: yes` - Mutation red on the intended assertion - `assembleDebug` + `testDebugUnitTest` + `compileDebugAndroidTestKotlin` + `ktlintCheck` + `detekt` + `lintDebug` green; pinned `actionlint` clean - `git diff app/src/main` empty — no production change in this PR
Sign in to join this conversation.