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

S2 — publish()'s null-return branch needs a seam before any test can reach it #142

New Issue
Closed
opened 2026-08-27 03:07:07 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-08-27 03:07:07 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Child 2 of 3 decomposing #133. Independent of S1 (#141). Touches the same file as S3 (#143) — take them in either order, but not in parallel.

Why this exists

app/src/main/java/org/libremediaconverter/convert/OutputPublisher.kt:170-180

val out = context.contentResolver.openOutputStream(destination)
    ?: error("Could not open destination for writing: $destination")   // :174 — never executed

The only never-executed line in OutputPublisher, and it is not reachable by any test today.

UnopenableUriTest.publishingToAnUnwritableDestinationThrowsRatherThanSilentlySucceeding looks like
it covers this and does not:

val failure = runCatching { publisher.publish(staged, bogus) }.exceptionOrNull()
assertTrue("publishing to a dead provider must not appear to succeed, got $failure", failure != null)

A dead provider throws FileNotFoundException inside openOutputStream; it does not return null.
The assertion cannot tell the two apart — and they are not interchangeable, because :177 decides
whether to delete a partially-written destination, and only one of the two has written anything.

Scope

One seam, as small as it can be:

protected open fun openDestination(uri: Uri): OutputStream? =
    context.contentResolver.openOutputStream(uri)

publish is already open, and OutputPublisher is already subclassed in tests —
WorkerStubs.kt's AlwaysRoomPublisher and NamingPublisher are the precedent, both overriding one
method to force one condition. This adds a third of the same shape.

Do not reach for ShadowContentResolver's registered-stream map. OutputPublisherPublishTest's
KDoc explains that it is consulted before any provider, which is what lets a test hand out a stream
that fails mid-write — a different condition, deliberately, and it cannot produce a null return.

Done means

A publisher whose openDestination returns null, asserting: the failure propagates, it names the
destination, and the destination is not deleted — because nothing was written to it, which is the
distinction this seam exists to make testable.

Mutation: change ?: error(...) to ?: return — the test must go red on a save that silently
reported success, which is the defect defect-audit.md D4 is about.

_Child 2 of 3 decomposing #133. Independent of S1 (#141). **Touches the same file as S3 (#143)** — take them in either order, but not in parallel._ ### Why this exists ``` app/src/main/java/org/libremediaconverter/convert/OutputPublisher.kt:170-180 ``` ```kotlin val out = context.contentResolver.openOutputStream(destination) ?: error("Could not open destination for writing: $destination") // :174 — never executed ``` The only never-executed *line* in `OutputPublisher`, and it is not reachable by any test today. `UnopenableUriTest.publishingToAnUnwritableDestinationThrowsRatherThanSilentlySucceeding` looks like it covers this and does not: ```kotlin val failure = runCatching { publisher.publish(staged, bogus) }.exceptionOrNull() assertTrue("publishing to a dead provider must not appear to succeed, got $failure", failure != null) ``` A dead provider throws `FileNotFoundException` *inside* `openOutputStream`; it does not return null. The assertion cannot tell the two apart — and they are not interchangeable, because `:177` decides whether to delete a partially-written destination, and only one of the two has written anything. ### Scope One seam, as small as it can be: ```kotlin protected open fun openDestination(uri: Uri): OutputStream? = context.contentResolver.openOutputStream(uri) ``` `publish` is already `open`, and `OutputPublisher` is already subclassed in tests — `WorkerStubs.kt`'s `AlwaysRoomPublisher` and `NamingPublisher` are the precedent, both overriding one method to force one condition. This adds a third of the same shape. **Do not reach for `ShadowContentResolver`'s registered-stream map.** `OutputPublisherPublishTest`'s KDoc explains that it is consulted *before* any provider, which is what lets a test hand out a stream that fails mid-write — a different condition, deliberately, and it cannot produce a null return. ### Done means A publisher whose `openDestination` returns null, asserting: the failure propagates, it names the destination, and **the destination is not deleted** — because nothing was written to it, which is the distinction this seam exists to make testable. **Mutation:** change `?: error(...)` to `?: return` — the test must go red on a save that silently reported success, which is the defect `defect-audit.md` **D4** is about.
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 enhancement sev:low
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#142