JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

D4's delete arm has still never run against a real DocumentsProvider: #226 proved the premise, not the fix #250

New Issue
Closed
opened 2026-09-06 16:10:42 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-09-06 16:10:42 +00:00 (Migrated from github.com)
Copy Link
Copy Source

#226 confirmed the premise of docs/defect-audit.md D4 — stock DocumentsUI hands back a
document reporting OpenableColumns.SIZE == 0, so destinationIsKnownEmpty can return true and
the fix is live rather than inert. It did not exercise the arm that premise guards.

OutputPublisher.publish (OutputPublisher.kt:185-195) only reaches deletePartialOutput when
the copy fails after a positively-empty destination:

val destinationWasEmpty = destinationIsKnownEmpty(destination)
try { ... } catch (failure: Throwable) {
    if (destinationWasEmpty) deletePartialOutput(destination, failure)
    throw failure
}

aSaveWritesToTheDocumentTheSystemPickerCreated drives the success path, so the catch is
never entered. DocumentsContract.deleteDocument against a real DocumentsProvider therefore
still has no instrumented coverage; the only assertions on it are
OutputPublisherPublishTest's, against FakeSafProvider under Robolectric — which is the same
"asserted only against a fake built to match it" shape that made #226 worth doing.

The forcing condition already exists

openDestination is protected open and its KDoc says why: it is the seam for making the copy
refuse. A publisher that runs the real size probe and then throws from openDestination reaches
the catch with destinationWasEmpty == true on a genuine SAF document.

FixtureDocumentsProvider is already wired for the assertion — deleteDocument records into
DELETED and deletedDocumentIds() reads it back. That accessor has no callers today; it was
added in #226 for this test and left in place with a KDoc naming this ticket rather than removed
and re-added. Either finish it here or delete it — do not leave it unread a second time.

Acceptance criterion

Delete the if (destinationWasEmpty) deletePartialOutput(destination, failure) line. Today
nothing anywhere goes red. The new test must.

Assert the path, not just the artefact (the HardwareFallbackTest lesson from #223): that the
document the picker created appears in deletedDocumentIds(), and that the original failure — not
a cleanup failure — is what propagated.

Cost, stated up front

This is a second picker-driven test, so it inherits @FailsOnEmulatorApi37 for the same
TaskSnapshotPer reason its three siblings carry, and
FAILS_ON_EMULATOR_API37_BASELINE moves 6 -> 7 in the same diff. The derived carrier counts
in CLAUDE.md and .github/workflows/status_check.yml move with it — both were found stale
during the 2026-09-06 re-check for exactly this reason.

#226 confirmed the **premise** of `docs/defect-audit.md` **D4** — stock DocumentsUI hands back a document reporting `OpenableColumns.SIZE == 0`, so `destinationIsKnownEmpty` can return true and the fix is live rather than inert. It did **not** exercise the arm that premise guards. `OutputPublisher.publish` (`OutputPublisher.kt:185-195`) only reaches `deletePartialOutput` when the copy *fails* after a positively-empty destination: ```kotlin val destinationWasEmpty = destinationIsKnownEmpty(destination) try { ... } catch (failure: Throwable) { if (destinationWasEmpty) deletePartialOutput(destination, failure) throw failure } ``` `aSaveWritesToTheDocumentTheSystemPickerCreated` drives the **success** path, so the `catch` is never entered. `DocumentsContract.deleteDocument` against a real `DocumentsProvider` therefore still has no instrumented coverage; the only assertions on it are `OutputPublisherPublishTest`'s, against `FakeSafProvider` under Robolectric — which is the same "asserted only against a fake built to match it" shape that made #226 worth doing. ## The forcing condition already exists `openDestination` is `protected open` and its KDoc says why: it is the seam for making the copy refuse. A publisher that runs the real size probe and then throws from `openDestination` reaches the `catch` with `destinationWasEmpty == true` on a genuine SAF document. `FixtureDocumentsProvider` is already wired for the assertion — `deleteDocument` records into `DELETED` and `deletedDocumentIds()` reads it back. **That accessor has no callers today**; it was added in #226 for this test and left in place with a KDoc naming this ticket rather than removed and re-added. Either finish it here or delete it — do not leave it unread a second time. ## Acceptance criterion Delete the `if (destinationWasEmpty) deletePartialOutput(destination, failure)` line. Today nothing anywhere goes red. The new test must. Assert the *path*, not just the artefact (the `HardwareFallbackTest` lesson from #223): that the document the picker created appears in `deletedDocumentIds()`, and that the original failure — not a cleanup failure — is what propagated. ## Cost, stated up front This is a second picker-driven test, so it inherits `@FailsOnEmulatorApi37` for the same `TaskSnapshotPer` reason its three siblings carry, and `FAILS_ON_EMULATOR_API37_BASELINE` moves **6 -> 7** in the same diff. The derived carrier counts in `CLAUDE.md` and `.github/workflows/status_check.yml` move with it — both were found stale during the 2026-09-06 re-check for exactly this reason.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
above-cut
Worked autonomously overnight: local, JVM-verifiable, no product decision
 accessibility
Barrier affecting people with disabilities
 backlog
Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here
 bug
Something isn't working
 confirmed
Reviewer demonstrated the defect
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 plausible
Reviewer could not fully demonstrate it; treat as unproven
 question
Further information is requested
 sev:high
High severity
 sev:low
Low severity
 sev:medium
Medium severity
 wontfix
This will not be worked on
No labels bug
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
JMR-dev
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#250