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

Four wave-4 candidates that are real but may not be worth their cost #204

New Issue
Open
opened 2026-09-02 12:48:08 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-09-02 12:48:08 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Wave 4, filed from a coverage read on main @ 54ca2dd, 2026-09-02. The wave proper is #192-#203; the shared filter note is on #194.

Four candidates the read turned up that are real but not clearly worth their cost. They are on one ticket rather than four because the decision they each need is the same one — "is this worth it?" — and splitting that four ways invites answering it four times by momentum rather than once on purpose.

Any of these is a valid close with a written finding instead of a test. Doing none of them is also a valid close.

O1 — compositionFor(input, plan) out of Media3Engine.startExport

app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt:106-117

Four booleans decide what the export actually contains:

.setRemoveVideo(plan.video == VideoPlan.Drop)
.setRemoveAudio(plan.audio == AudioPlan.Drop)
.setTransmuxVideo(plan.video == VideoPlan.Copy)
.setTransmuxAudio(plan.audio == AudioPlan.Copy)

The comment above the first pair records the defect they fixed: "Without setRemoveVideo, asking for M4A produced an HEVC stream in a file named .m4a." Nothing on any source set asserts them except an end-to-end device transcode, which proves the outcome without pinning the decision.

EditedMediaItem and Composition are plain builders — no Transformer, no device — so a pure compositionFor(input, plan) returning the built Composition is assertable on the JVM. Prefer that to driving Transformer.start under Robolectric: the seam tests the decision, the experiment tests Media3.

Mutation: transpose setRemoveVideo and setRemoveAudio.

The cost: a seam in the middle of a device-only method, for four boolean assertions.

O2 — logIfRefused's onFailure

app/src/main/java/org/libremediaconverter/work/ConversionWorker.kt:291-292

runCatching { posted.get() } runs; the future has never failed. Behaviour worth having: a progress update WorkManager refuses does not fail the job — doWork() still returns Success.

The fixture is the awkward part. It needs a ForegroundUpdater that succeeds once and then fails, so the worker's initial setForeground is not what breaks — that case is DeniedForegroundStartTest's and is already covered. WorkerStubs.FailedFuture exists for the mechanism.

Mutation: drop the runCatching — the ExecutionException propagates out as Result.failure.

O3 — MainActivity reads the real window width, and nothing says so

app/src/main/java/org/libremediaconverter/MainActivity.kt:76-83

onCreate is device-covered by SafPickerRoundTripTest, so its 0% is the usual measurement boundary rather than a gap. What is genuinely unasserted anywhere is the one decision in it: calculateWindowSizeClass(this) feeding AppRoot. AdaptiveShellTest passes the width in as a parameter, which is what makes it a good test of the shell and no test of the Activity.

This is a spike, not a task. Robolectric.buildActivity(MainActivity::class.java).setup() under @Config(qualifiers = "w840dp") should show the rail rather than the bottom bar — but manifest theme resolution and WindowMetricsCalculator under Robolectric are unverified here. Record the answer either way; a written "this cannot be reached on the JVM, here is what was tried" is the successful outcome if it fails.

Mutation: hardcode WindowWidthSizeClass.Compact at :80.

Note that deleting enableEdgeToEdge() would not be caught by this, and should not be claimed.

O4 — InputQuery's unknown-size arm is device-only

app/src/main/java/org/libremediaconverter/convert/InputQuery.kt:102

statSize == -1 — a pipe or streaming provider — is real in production and load-bearing: it feeds hasSpaceFor. But Robolectric's ShadowParcelFileDescriptor.getStatSize() is getFile().length() over a RandomAccessFile, returning -1 only on IOException, and even its createPipe() is temp-file backed. So this belongs in androidTest, or needs a seam.

Worth recording either way: its cursor twin is tested — InputQueryCursorTest.kt:132, "a negative size is unknown rather than reported". Two paths to the same "size unknown" answer, one pinned. That asymmetry is the argument CLAUDE.md used for including ContainerCapabilities:94, and it applies here whichever way this is decided.

Not the acceptance

The coverage number, for any of the four.

_Wave 4, filed from a coverage read on `main` @ `54ca2dd`, 2026-09-02. The wave proper is #192-#203; the shared filter note is on #194._ Four candidates the read turned up that are **real but not clearly worth their cost**. They are on one ticket rather than four because the decision they each need is the same one — "is this worth it?" — and splitting that four ways invites answering it four times by momentum rather than once on purpose. **Any of these is a valid close with a written finding instead of a test.** Doing none of them is also a valid close. --- ## O1 — `compositionFor(input, plan)` out of `Media3Engine.startExport` ``` app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt:106-117 ``` Four booleans decide what the export actually contains: ```kotlin .setRemoveVideo(plan.video == VideoPlan.Drop) .setRemoveAudio(plan.audio == AudioPlan.Drop) .setTransmuxVideo(plan.video == VideoPlan.Copy) .setTransmuxAudio(plan.audio == AudioPlan.Copy) ``` The comment above the first pair records the defect they fixed: "Without `setRemoveVideo`, asking for M4A produced an HEVC stream in a file named .m4a." Nothing on **any** source set asserts them except an end-to-end device transcode, which proves the outcome without pinning the decision. `EditedMediaItem` and `Composition` are plain builders — no `Transformer`, no device — so a pure `compositionFor(input, plan)` returning the built `Composition` is assertable on the JVM. **Prefer that to driving `Transformer.start` under Robolectric**: the seam tests the decision, the experiment tests Media3. *Mutation:* transpose `setRemoveVideo` and `setRemoveAudio`. *The cost:* a seam in the middle of a device-only method, for four boolean assertions. ## O2 — `logIfRefused`'s `onFailure` ``` app/src/main/java/org/libremediaconverter/work/ConversionWorker.kt:291-292 ``` `runCatching { posted.get() }` runs; the future has never failed. Behaviour worth having: **a progress update WorkManager refuses does not fail the job** — `doWork()` still returns `Success`. *The fixture is the awkward part.* It needs a `ForegroundUpdater` that succeeds once and then fails, so the worker's *initial* `setForeground` is not what breaks — that case is `DeniedForegroundStartTest`'s and is already covered. `WorkerStubs.FailedFuture` exists for the mechanism. *Mutation:* drop the `runCatching` — the `ExecutionException` propagates out as `Result.failure`. ## O3 — `MainActivity` reads the real window width, and nothing says so ``` app/src/main/java/org/libremediaconverter/MainActivity.kt:76-83 ``` `onCreate` is **device-covered** by `SafPickerRoundTripTest`, so its 0% is the usual measurement boundary rather than a gap. What is genuinely unasserted anywhere is the one decision in it: `calculateWindowSizeClass(this)` feeding `AppRoot`. `AdaptiveShellTest` passes the width in as a parameter, which is what makes it a good test of the shell and no test of the Activity. *This is a spike, not a task.* `Robolectric.buildActivity(MainActivity::class.java).setup()` under `@Config(qualifiers = "w840dp")` should show the rail rather than the bottom bar — but manifest theme resolution and `WindowMetricsCalculator` under Robolectric are unverified here. **Record the answer either way**; a written "this cannot be reached on the JVM, here is what was tried" is the successful outcome if it fails. *Mutation:* hardcode `WindowWidthSizeClass.Compact` at `:80`. Note that deleting `enableEdgeToEdge()` would **not** be caught by this, and should not be claimed. ## O4 — `InputQuery`'s unknown-size arm is device-only ``` app/src/main/java/org/libremediaconverter/convert/InputQuery.kt:102 ``` `statSize == -1` — a pipe or streaming provider — is real in production and load-bearing: it feeds `hasSpaceFor`. But Robolectric's `ShadowParcelFileDescriptor.getStatSize()` is `getFile().length()` over a `RandomAccessFile`, returning `-1` only on IOException, and even its `createPipe()` is temp-file backed. So this belongs in `androidTest`, or needs a seam. **Worth recording either way:** its cursor twin *is* tested — `InputQueryCursorTest.kt:132`, "a negative size is unknown rather than reported". Two paths to the same "size unknown" answer, one pinned. That asymmetry is the argument `CLAUDE.md` used for including `ContainerCapabilities:94`, and it applies here whichever way this is decided. --- ## Not the acceptance The coverage number, for any of the four.
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
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#204