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

Six JVM test gaps from the 2026-08-26 coverage read, each with a sibling test that already models it #132

New Issue
Closed
opened 2026-08-27 02:20:52 +00:00 by JMR-dev · 4 comments
JMR-dev commented 2026-08-27 02:20:52 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Filed from a coverage read on main @ dc8b7c3, 2026-08-26. The number is not the reason — see the last section. Companion ticket: #133 holds the seam questions from the same read; the code findings are docs/coverage-read-findings.md (PR #131).

Re-measured with ./gradlew :app:jacocoTestReport: 84.9% line (1971/2321), 63.8% branch (900/1410), against 456 JVM tests in 68 classes. (CLAUDE.md quotes 454/67 from four hours earlier; percentages unchanged, so nothing there is stale.)

Read this first: what "gap" means here

Corrected 2026-08-27 — this ticket was filed with seven items and now has six. Item 6 was
ConversionNotifications.areEnabled(), and it was the cheapest-looking thing on the list: three
cold lines, a KDoc with real user-visible stakes, a permission Robolectric flips in one line.
grep -rn 'areEnabled' app/src returns the declaration and nothing else — both workers
construct ConversionNotifications and only ever call build(). The behaviour its KDoc describes
does not happen, so a test would assert that a function nobody calls returns what the platform
told it: green, vacuous, and misleading, because it would imply the disabled-notification case is
handled. It is now F5 in docs/coverage-read-findings.md, where the open question is whether
the app should act on it at all. Stated rather than quietly renumbered, because the near-miss is
the useful part.

JaCoCo in this repo measures testDebugUnitTest only. Every item below was checked against the androidTest suite by name before being listed, because that boundary is what made #52's premise an artifact and what #84, #85, #86 and #88 each had to correct for. These seven are untested, not merely unmeasured.

Three near-misses were checked and dropped rather than listed:

  • ConcatWorker.doWork's two input guards (:40, :42) — covered by UnopenableUriTest.aJoinWithNoInputArrayFailsWithAMessage and ConcatWorkerTest.aSingleInputFailsWithAnActionableMessage.

  • ConcatWorker.getForegroundInfo (:133-137) — #88 settled this as a named exemption. Not reopened here.

  • ConversionForegroundType.current()'s API 33/34 arms — looked like the sharpest item in the read. #88 established the CI matrix covers all three regimes, and the premise worth re-checking was whether #122's wedge still kills the 33/34 legs.

    Corrected after filing. This bullet first said "it does not", from one green run. #122 is not resolved — it fired on run 33033036857, a docs-only PR: wedged: yes — gradle was killed after 1200s, failed: unknown, 23m08s. It is intermittent (five of the last six completed legs passed in ~7 minutes), and what it costs is the verdict, not the execution: received: 60 means all sixty tests still reported, so the API 33 regime was exercised — the leg simply could not have said so if one had broken.

    Still not an item on this ticket. #88's reasoning holds. But a @Config(sdk = 33) / @Config(sdk = 34) JVM test is three lines of insurance against a leg that cannot be relied on to go red, and is worth folding in alongside item 1, which is in the same file's neighbourhood. Evidence is in docs/coverage-read-findings.md.

Children

Decomposed 2026-08-27. #134 first — it unblocks two of the others and changes nothing in production.

child item depends on
#134 — extract the fake SAF provider into shared scaffolding, give it a way to answer wrongly (new — the shared enabler) —
#135 — readSpec's three enum fallbacks 1 —
#136 — the audio half of validate 2 —
#137 — InputQuery has never been handed a cursor row 3 #134
#138 — ConcatWorker's cancellation and foreground-denied arms 4 —
#139 — ConversionWorker's missing-URI and invalid-spec refusals 5 —
#140 — four partial branches in OutputPublisher 6 #134, for the cursor third only

#134 is not in the six above. It came out of decomposition: items 3 and 6 are both cursor-shaped, and
the provider that can drive them exists but cannot answer wrongly. Filing it separately rather than
duplicating the work in two children follows #57, which was the same shape — an enabler with no
behaviour change, picked up first.

Four of the seven depend on nothing and can be taken in any order.

The six

Each has a sibling already in the suite that establishes the pattern, which is what makes these small diffs rather than new harness work.

# Where What has no test Sibling that models it
1 work/ConversionWorker.kt:332,335,338 all three ?: return fallback arms of readSpec() WorkerEnumFallbackTest
2 model/ContainerCapabilities.kt:227-229,232-234,241-243,247-250 + :101-102 the whole audio half of validate, six refusal messages ContainerCapabilitiesTest — has the video twin of each
3 convert/InputQuery.kt:90,104-105,107-108 everything that reads a real cursor row OutputPublisherPublishTest's FakeSafProvider — same package
4 work/ConcatWorker.kt:92,95-96,105-106 cancellation delete-and-rethrow; FOREGROUND_DENIED WorkerCancellationTest, DeniedForegroundStartTest
5 work/ConversionWorker.kt:62 and :124-126 missing KEY_INPUT_URI; the Validation.Invalid refusal WorkerEnumFallbackTest, SpaceCheckTest
6 convert/OutputPublisher.kt:197,235,258,271 three short-circuits in destinationIsKnownEmpty; two null guards OutputPublisherPublishTest, StagingSweepTest

1. readSpec()'s three fallbacks — the sharpest item here.

WorkerEnumFallbackTest exists for this exact defect class and its KDoc states the signature precisely: a name this build does not define, read above the try, threw out of doWork() entirely — FAILED with reschedule = false, empty output Data so the screen said "Conversion failed." with nothing else, and the staged file never deleted. It covers 2 of the 5 above-the-try reads (quality, engine preference). readSpec() is called at :69, also above the try, and its container / video / audio reads are all cold.

Done means each of the three names independently unresolvable, each falling back to OutputFormat.MP4_H265.spec without throwing.
Mutation: change one ?: return fallback to ?: error(...) — the test must go red naming that axis, not time out.

2. The audio half of validate.

Six user-visible refusal strings with no test: unidentifiable source audio on a COPY (:227), a container that cannot hold the copied source (:232), a codec the container cannot carry (:241), a codec this app cannot encode (:247), plus both arms of accepts(_, AudioCodec, _) at :101-102 — including the error("Resolve COPY to a concrete codec…") guard whose video twin is already tested at ContainerCapabilitiesTest:110.

Done means each refusal named by its message and its suggestions asserted valid, mirroring the video cases beside them.
Mutation: swap CARRIES_AUDIO for CARRIES_VIDEO in validateAudio — the container-cannot-hold tests must go red.

3. InputQuery's cursor half.

firstRow's body (:90), displayNameOrNull (:104-105) and sizeOrNull (:107-108) have never executed. UnknownInputSizeTest is the sibling for the file but not for this: its KDoc is explicit that it drives the case where no provider is registered, so the query returns null and measure answers instead. Nothing in the suite has ever handed InputQuery a row.

Untested as a result: display name resolution, size from OpenableColumns.SIZE, both isNull guards, the missing-column guard, and the negative-size rejection at :108 — which is one of the two places CLAUDE.md's zero-vs-unknown distinction is actually enforced.

The harness for this already exists and is in the same package. OutputPublisherPublishTest declares internal open class FakeSafProvider : ContentProvider() in org.libremediaconverter.convert, returning a MatrixCursor over exactly OpenableColumns.DISPLAY_NAME and OpenableColumns.SIZE — the two columns InputQuery reads — with Robolectric.buildContentProvider(...).create(info) beside it. It is open already. What it does not yet do is misbehave: its row is always (file.name, file.length()), so covering the null, missing-column and negative-size cases means one subclass or one knob on it, not a new harness.

Done means a row with a name and size, a row with the columns absent, a row with SIZE null, and a row with a negative SIZE falling through to measure.
Mutation: drop .takeIf { it >= 0 } from :108 — the negative-size test must go red.

4. ConcatWorker's failure branches.

ConversionWorker has WorkerCancellationTest and DeniedForegroundStartTest; its twin has neither. Untested: the CancellationException arm that deletes the staged file and rethrows (:92, 95-96) — the rule defect-audit.md D10 exists for — and FOREGROUND_DENIED (:105-106), of the three FailureOutcome arms the only one cold.

Done means both, asserted on the staged file's absence and on the KEY_ERROR value respectively.
Mutation: change the cancellation arm to return Result.failure() instead of rethrowing — the test must go red.

5. Two more in ConversionWorker.

:62 — a missing KEY_INPUT_URI returning "No input file." is untested everywhere: ForcedFailureTest.aMissingInputFailsRatherThanCrashing passes a URI to a nonexistent file, which is a different path.

:124-126 — the Validation.Invalid refusal, whose comment names its reason for existing: a job enqueued before the settings changed, or a hand-built ConversionWorker.request(...). That is a real arrival path (WorkManager keeps queued work for about a week, the same premise WorkerEnumFallbackTest is written on) and nothing exercises it.

Mutation: delete the if (validation is Validation.Invalid) block — the test must go red on the error message, not on a downstream conversion failure.

6. OutputPublisher edges.

destinationIsKnownEmpty (:197) has three untested short-circuits — no SIZE column, no row, null value — and each one must answer false, because the KDoc is explicit that "I could not tell" must never authorise a delete. That is the guard standing between a failed save and deleting a file the user already had. Plus staged.parentFile == null (:235) and listFiles() returning null (:258).

These are cursor-shaped too, so they take the same FakeSafProvider extension item 3 needs. Doing 3 and 7 together is one piece of harness work and two sets of assertions.

Mutation: change :197's size >= 0 && to size >= -1 && — the missing-column test must go red.

What this ticket is not

  • Not the seams. Three sites need a seam cut before they can be tested at all — MediaProbe's extractor half and two OutputPublisher edges. That is #133, which also records why AndroidDeviceCodecs.probe() was considered and left out.
  • Not the code findings. Four things from the same read are code problems a test would document rather than fix (a dead Vorbis encoder arm, a write-only flag, two callerless accessors, two deliberate unreachable guards). They are docs/coverage-read-findings.md, PR #131.

Not the acceptance

The coverage number. These six are worth roughly 22 lines of 2321 and will barely move it. JaCoCo did not count a Robolectric test in this repo until #76, #52's entire premise was an artifact of that, and CLAUDE.md is explicit that the norm is behaviours having tests that bite, not a percentage rising. Every item above carries a mutation for that reason. A test that does not go red when its line is reverted has not closed its row.

_Filed from a coverage read on `main` @ `dc8b7c3`, 2026-08-26. **The number is not the reason** — see the last section. Companion ticket: #133 holds the seam questions from the same read; the code findings are `docs/coverage-read-findings.md` (PR #131)._ Re-measured with `./gradlew :app:jacocoTestReport`: **84.9% line (1971/2321), 63.8% branch (900/1410)**, against **456 JVM tests in 68 classes**. (`CLAUDE.md` quotes 454/67 from four hours earlier; percentages unchanged, so nothing there is stale.) ### Read this first: what "gap" means here > **Corrected 2026-08-27 — this ticket was filed with seven items and now has six.** Item 6 was > `ConversionNotifications.areEnabled()`, and it was the cheapest-looking thing on the list: three > cold lines, a KDoc with real user-visible stakes, a permission Robolectric flips in one line. > `grep -rn 'areEnabled' app/src` returns **the declaration and nothing else** — both workers > construct `ConversionNotifications` and only ever call `build()`. The behaviour its KDoc describes > does not happen, so a test would assert that a function nobody calls returns what the platform > told it: green, vacuous, and misleading, because it would imply the disabled-notification case is > handled. It is now **F5** in `docs/coverage-read-findings.md`, where the open question is whether > the app should act on it at all. Stated rather than quietly renumbered, because the near-miss is > the useful part. JaCoCo in this repo measures `testDebugUnitTest` **only**. Every item below was checked against the `androidTest` suite by name before being listed, because that boundary is what made #52's premise an artifact and what #84, #85, #86 and #88 each had to correct for. **These seven are untested, not merely unmeasured.** Three near-misses were checked and dropped rather than listed: - `ConcatWorker.doWork`'s two input guards (`:40`, `:42`) — covered by `UnopenableUriTest.aJoinWithNoInputArrayFailsWithAMessage` and `ConcatWorkerTest.aSingleInputFailsWithAnActionableMessage`. - `ConcatWorker.getForegroundInfo` (`:133-137`) — **#88 settled this** as a named exemption. Not reopened here. - `ConversionForegroundType.current()`'s API 33/34 arms — looked like the sharpest item in the read. #88 established the CI matrix covers all three regimes, and the premise worth re-checking was whether #122's wedge still kills the 33/34 legs. **Corrected after filing.** This bullet first said "it does not", from one green run. #122 is *not* resolved — it fired on run `33033036857`, a docs-only PR: `wedged: yes — gradle was killed after 1200s`, `failed: unknown`, 23m08s. It is intermittent (five of the last six completed legs passed in ~7 minutes), and **what it costs is the verdict, not the execution**: `received: 60` means all sixty tests still reported, so the API 33 regime *was* exercised — the leg simply could not have said so if one had broken. Still not an item on this ticket. #88's reasoning holds. But a `@Config(sdk = 33)` / `@Config(sdk = 34)` JVM test is three lines of insurance against a leg that cannot be relied on to go red, and is worth folding in alongside item 1, which is in the same file's neighbourhood. Evidence is in `docs/coverage-read-findings.md`. ### Children Decomposed 2026-08-27. **#134 first — it unblocks two of the others and changes nothing in production.** | child | item | depends on | |---|---|---| | **#134** — extract the fake SAF provider into shared scaffolding, give it a way to answer wrongly | *(new — the shared enabler)* | — | | #135 — `readSpec`'s three enum fallbacks | 1 | — | | #136 — the audio half of `validate` | 2 | — | | #137 — `InputQuery` has never been handed a cursor row | 3 | **#134** | | #138 — `ConcatWorker`'s cancellation and foreground-denied arms | 4 | — | | #139 — `ConversionWorker`'s missing-URI and invalid-spec refusals | 5 | — | | #140 — four partial branches in `OutputPublisher` | 6 | **#134**, for the cursor third only | #134 is not in the six above. It came out of decomposition: items 3 and 6 are both cursor-shaped, and the provider that can drive them exists but cannot answer *wrongly*. Filing it separately rather than duplicating the work in two children follows #57, which was the same shape — an enabler with no behaviour change, picked up first. Four of the seven depend on nothing and can be taken in any order. ### The six Each has a **sibling already in the suite** that establishes the pattern, which is what makes these small diffs rather than new harness work. | # | Where | What has no test | Sibling that models it | |---|---|---|---| | 1 | `work/ConversionWorker.kt:332,335,338` | all three `?: return fallback` arms of `readSpec()` | `WorkerEnumFallbackTest` | | 2 | `model/ContainerCapabilities.kt:227-229,232-234,241-243,247-250` + `:101-102` | the whole audio half of `validate`, six refusal messages | `ContainerCapabilitiesTest` — has the video twin of each | | 3 | `convert/InputQuery.kt:90,104-105,107-108` | everything that reads a real cursor row | `OutputPublisherPublishTest`'s `FakeSafProvider` — same package | | 4 | `work/ConcatWorker.kt:92,95-96,105-106` | cancellation delete-and-rethrow; `FOREGROUND_DENIED` | `WorkerCancellationTest`, `DeniedForegroundStartTest` | | 5 | `work/ConversionWorker.kt:62` and `:124-126` | missing `KEY_INPUT_URI`; the `Validation.Invalid` refusal | `WorkerEnumFallbackTest`, `SpaceCheckTest` | | 6 | `convert/OutputPublisher.kt:197,235,258,271` | three short-circuits in `destinationIsKnownEmpty`; two null guards | `OutputPublisherPublishTest`, `StagingSweepTest` | --- **1. `readSpec()`'s three fallbacks — the sharpest item here.** `WorkerEnumFallbackTest` exists *for this exact defect class* and its KDoc states the signature precisely: a name this build does not define, read **above the `try`**, threw out of `doWork()` entirely — FAILED with `reschedule = false`, empty output `Data` so the screen said "Conversion failed." with nothing else, and the staged file never deleted. It covers 2 of the 5 above-the-`try` reads (quality, engine preference). `readSpec()` is called at `:69`, also above the `try`, and its container / video / audio reads are all cold. *Done means* each of the three names independently unresolvable, each falling back to `OutputFormat.MP4_H265.spec` without throwing. *Mutation:* change one `?: return fallback` to `?: error(...)` — the test must go red naming that axis, not time out. **2. The audio half of `validate`.** Six user-visible refusal strings with no test: unidentifiable source audio on a COPY (`:227`), a container that cannot hold the copied source (`:232`), a codec the container cannot carry (`:241`), a codec this app cannot encode (`:247`), plus both arms of `accepts(_, AudioCodec, _)` at `:101-102` — including the `error("Resolve COPY to a concrete codec…")` guard whose **video twin is already tested** at `ContainerCapabilitiesTest:110`. *Done means* each refusal named by its message and its suggestions asserted valid, mirroring the video cases beside them. *Mutation:* swap `CARRIES_AUDIO` for `CARRIES_VIDEO` in `validateAudio` — the container-cannot-hold tests must go red. **3. `InputQuery`'s cursor half.** `firstRow`'s body (`:90`), `displayNameOrNull` (`:104-105`) and `sizeOrNull` (`:107-108`) have never executed. `UnknownInputSizeTest` is the sibling for the *file* but not for this: its KDoc is explicit that it drives the case where **no provider is registered**, so the query returns null and `measure` answers instead. Nothing in the suite has ever handed `InputQuery` a row. Untested as a result: display name resolution, size from `OpenableColumns.SIZE`, both `isNull` guards, the missing-column guard, and the **negative-size rejection at `:108`** — which is one of the two places `CLAUDE.md`'s zero-vs-unknown distinction is actually enforced. **The harness for this already exists and is in the same package.** `OutputPublisherPublishTest` declares `internal open class FakeSafProvider : ContentProvider()` in `org.libremediaconverter.convert`, returning a `MatrixCursor` over exactly `OpenableColumns.DISPLAY_NAME` and `OpenableColumns.SIZE` — the two columns `InputQuery` reads — with `Robolectric.buildContentProvider(...).create(info)` beside it. It is `open` already. What it does not yet do is *misbehave*: its row is always `(file.name, file.length())`, so covering the null, missing-column and negative-size cases means one subclass or one knob on it, not a new harness. *Done means* a row with a name and size, a row with the columns absent, a row with `SIZE` null, and a row with a negative `SIZE` falling through to `measure`. *Mutation:* drop `.takeIf { it >= 0 }` from `:108` — the negative-size test must go red. **4. `ConcatWorker`'s failure branches.** `ConversionWorker` has `WorkerCancellationTest` and `DeniedForegroundStartTest`; its twin has neither. Untested: the `CancellationException` arm that deletes the staged file and rethrows (`:92, 95-96`) — the rule `defect-audit.md` D10 exists for — and `FOREGROUND_DENIED` (`:105-106`), of the three `FailureOutcome` arms the only one cold. *Done means* both, asserted on the staged file's absence and on the `KEY_ERROR` value respectively. *Mutation:* change the cancellation arm to return `Result.failure()` instead of rethrowing — the test must go red. **5. Two more in `ConversionWorker`.** `:62` — a missing `KEY_INPUT_URI` returning `"No input file."` is untested **everywhere**: `ForcedFailureTest.aMissingInputFailsRatherThanCrashing` passes a URI to a *nonexistent file*, which is a different path. `:124-126` — the `Validation.Invalid` refusal, whose comment names its reason for existing: a job enqueued before the settings changed, or a hand-built `ConversionWorker.request(...)`. That is a real arrival path (WorkManager keeps queued work for about a week, the same premise `WorkerEnumFallbackTest` is written on) and nothing exercises it. *Mutation:* delete the `if (validation is Validation.Invalid)` block — the test must go red on the error message, not on a downstream conversion failure. **6. `OutputPublisher` edges.** `destinationIsKnownEmpty` (`:197`) has three untested short-circuits — no `SIZE` column, no row, null value — and each one must answer **false**, because the KDoc is explicit that "I could not tell" must never authorise a delete. That is the guard standing between a failed save and deleting a file the user already had. Plus `staged.parentFile == null` (`:235`) and `listFiles()` returning null (`:258`). These are cursor-shaped too, so they take the same `FakeSafProvider` extension item 3 needs. Doing 3 and 7 together is one piece of harness work and two sets of assertions. *Mutation:* change `:197`'s `size >= 0 &&` to `size >= -1 &&` — the missing-column test must go red. ### What this ticket is not - **Not the seams.** Three sites need a seam cut before they can be tested at all — `MediaProbe`'s extractor half and two `OutputPublisher` edges. That is #133, which also records why `AndroidDeviceCodecs.probe()` was considered and left out. - **Not the code findings.** Four things from the same read are code problems a test would document rather than fix (a dead Vorbis encoder arm, a write-only flag, two callerless accessors, two deliberate unreachable guards). They are `docs/coverage-read-findings.md`, PR #131. ### Not the acceptance The coverage number. These six are worth roughly 22 lines of 2321 and will barely move it. JaCoCo did not count a Robolectric test in this repo until #76, #52's entire premise was an artifact of that, and `CLAUDE.md` is explicit that the norm is behaviours having tests that **bite**, not a percentage rising. Every item above carries a mutation for that reason. **A test that does not go red when its line is reverted has not closed its row.**
JMR-dev commented 2026-08-27 04:03:14 +00:00 (Migrated from github.com)
Copy Link
Copy Source

All children are implemented, and the batch integrates. Verified locally rather than assumed, because eight PRs touching overlapping files is exactly where a clean-per-PR result stops meaning much.

Merged all eight branches onto current main in a throwaway branch:

  • no conflicts — the stack (#144 → #149 → #151) and the five independent branches merge cleanly in any order
  • 500 tests in 71 classes, 0 failures, 0 errors (baseline was 456 in 68)
  • full gate green on the integrated result: ktlintCheck, detekt, lintDebug, testDebugUnitTest, compileDebugAndroidTestKotlin
metric before integrated
line 84.9% (1971/2321) 87.1% (2024/2324)
branch 63.8% (900/1410) 69.0% (973/1410)

Branch moved more than line, which is what this batch was aimed at — nearly every test here targets a guard rather than a new code path.

Files this batch was about, after integration:

file missed lines missed branches
InputQuery.kt 0 3
OutputPublisher.kt 0 2
ContainerCapabilities.kt 0 12
MediaProbe.kt 43 72 (was 91)
ConcatWorker.kt 15 3 (was 4)
ConversionWorker.kt 22 14

What remains in those files is the native/device half and the named exemptions recorded in each PR — not unclaimed gaps.

Follow-up worth its own ticket, not folded in here: CLAUDE.md's coverage entry quotes 84.9%/63.8% and instructs re-measuring before quoting. It goes stale the moment this batch lands. Updating it now would be quoting a number that is not true of main yet, which is the exact failure that entry documents about itself.

**All children are implemented, and the batch integrates.** Verified locally rather than assumed, because eight PRs touching overlapping files is exactly where a clean-per-PR result stops meaning much. Merged all eight branches onto current `main` in a throwaway branch: - **no conflicts** — the stack (#144 → #149 → #151) and the five independent branches merge cleanly in any order - **500 tests in 71 classes, 0 failures, 0 errors** (baseline was 456 in 68) - full gate green on the integrated result: `ktlintCheck`, `detekt`, `lintDebug`, `testDebugUnitTest`, `compileDebugAndroidTestKotlin` | metric | before | integrated | |---|---|---| | line | 84.9% (1971/2321) | **87.1%** (2024/2324) | | branch | 63.8% (900/1410) | **69.0%** (973/1410) | Branch moved more than line, which is what this batch was aimed at — nearly every test here targets a guard rather than a new code path. Files this batch was about, after integration: | file | missed lines | missed branches | |---|---|---| | `InputQuery.kt` | **0** | 3 | | `OutputPublisher.kt` | **0** | 2 | | `ContainerCapabilities.kt` | **0** | 12 | | `MediaProbe.kt` | 43 | 72 (was 91) | | `ConcatWorker.kt` | 15 | 3 (was 4) | | `ConversionWorker.kt` | 22 | 14 | What remains in those files is the native/device half and the named exemptions recorded in each PR — not unclaimed gaps. **Follow-up worth its own ticket, not folded in here:** `CLAUDE.md`'s coverage entry quotes 84.9%/63.8% and instructs re-measuring before quoting. It goes stale the moment this batch lands. Updating it now would be quoting a number that is not true of `main` yet, which is the exact failure that entry documents about itself.
JMR-dev commented 2026-08-27 04:16:47 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Residual-gap audit: what is still cold after all ten children, and why

The children are done, so the honest closing question is not "did coverage go up" but "of the lines still never executed, does each one map to something already named?" I rebuilt the integration branch (all eight PR heads are ancestors of it — #151 is stacked on #149 on #144, so the tip carries the stack), ran jacocoTestReport, and took every line with ci == 0.

Everything mapped except one.

ConversionWorker.kt — 15 lines, all named

lines what accounted for by
133, 206–221, 295 the Media3 arm, runMedia3OrFallBack, isCancellation #84 — device-bound hardware path; isCancellation is only called from line 213
228, 231 FFmpegKitConfig.getSafParameterForRead and its elvis native, JVM-unreachable
342–346 the getForegroundInfo override #88's exemption shape — WorkManager calls it, TestListenableWorkerBuilder does not

ConcatWorker.kt — one line did not map

lines what accounted for by
38 the suspend fun doWork signature Kotlin state-machine artifact — 39 and 41 are covered
80–84, 88–89 ConcatEngine.join success path native; NamingPublisher's KDoc already says no JVM test gets past it
133–137 getForegroundInfo #88's named exemption, cited by name in CLAUDE.md
40 "No input files." androidTest — UnopenableUriTest.aJoinWithNoInputArrayFailsWithAMessage
42 "Pick at least two files to join." nothing, in either source set

The one gap, and why it hid

Lines 40 and 42 are two arms of the same guard, four lines apart. JaCoCo shows both cold and cannot tell them apart, because it measures testDebugUnitTest only and line 40's test lives in androidTest. Reading the report alone, either both look like gaps or — if you know the join flow is e2e-covered — both look accounted for. Only grep over the androidTest source separates them: "No input files." appears there, "Pick at least two files to join." appears nowhere.

That is the generalisable finding from this audit, and it is worth more than the line itself: on this repo, "cold in JaCoCo" and "untested" differ by whatever androidTest happens to cover, so a residual-line audit has to read both source sets or it will mis-sort adjacent branches.

Closed in test/refused-jobs (#148), where the file's existing charter — "jobs the worker refuses before it converts anything" — already covers it. Two tests, each killed by exactly one mutation:

mutation red
guard deleted outright a join of a single file is refused with a message rather than joined
uris.size < 2 → < 3 a join of two files is not refused for its count

The control refuses the space rather than running the job, so it proves execution cleared line 42 without touching the native engine.

Also cold, and correctly so

ConversionNotifications.areEnabled() (60–62) — F5 in docs/coverage-read-findings.md, already merged. No callers; a test would pin dead code. Line 30 is the build$default bridge, an artifact.

MediaProbe.kt's residual lines are accounted for in #150's PR body (the FFprobe half and the two catch arms) and are not re-audited here.

Running mutation total across the batch: 50 run, 45 red. The five green are all written down — three unfalsifiable guards recorded as named exemptions in test KDoc, one seam I had put in the wrong place (#143), and one bad mutation of mine that reached the same return by a different route.

## Residual-gap audit: what is still cold after all ten children, and why The children are done, so the honest closing question is not "did coverage go up" but **"of the lines still never executed, does each one map to something already named?"** I rebuilt the integration branch (all eight PR heads are ancestors of it — #151 is stacked on #149 on #144, so the tip carries the stack), ran `jacocoTestReport`, and took every line with `ci == 0`. **Everything mapped except one.** ### ConversionWorker.kt — 15 lines, all named | lines | what | accounted for by | |---|---|---| | 133, 206–221, 295 | the Media3 arm, `runMedia3OrFallBack`, `isCancellation` | #84 — device-bound hardware path; `isCancellation` is only called from line 213 | | 228, 231 | `FFmpegKitConfig.getSafParameterForRead` and its elvis | native, JVM-unreachable | | 342–346 | the `getForegroundInfo` override | #88's exemption shape — WorkManager calls it, `TestListenableWorkerBuilder` does not | ### ConcatWorker.kt — one line did not map | lines | what | accounted for by | |---|---|---| | 38 | the `suspend fun doWork` signature | Kotlin state-machine artifact — 39 and 41 are covered | | 80–84, 88–89 | `ConcatEngine.join` success path | native; `NamingPublisher`'s KDoc already says no JVM test gets past it | | 133–137 | `getForegroundInfo` | #88's named exemption, cited by name in `CLAUDE.md` | | 40 | `"No input files."` | **androidTest** — `UnopenableUriTest.aJoinWithNoInputArrayFailsWithAMessage` | | **42** | **`"Pick at least two files to join."`** | **nothing, in either source set** | ### The one gap, and why it hid Lines 40 and 42 are two arms of the same guard, four lines apart. JaCoCo shows both cold and **cannot tell them apart**, because it measures `testDebugUnitTest` only and line 40's test lives in `androidTest`. Reading the report alone, either both look like gaps or — if you know the join flow is e2e-covered — both look accounted for. Only `grep` over the androidTest source separates them: `"No input files."` appears there, `"Pick at least two files to join."` appears nowhere. That is the generalisable finding from this audit, and it is worth more than the line itself: **on this repo, "cold in JaCoCo" and "untested" differ by whatever androidTest happens to cover, so a residual-line audit has to read both source sets or it will mis-sort adjacent branches.** Closed in `test/refused-jobs` (#148), where the file's existing charter — "jobs the worker refuses before it converts anything" — already covers it. Two tests, each killed by exactly one mutation: | mutation | red | |---|---| | guard deleted outright | `a join of a single file is refused with a message rather than joined` | | `uris.size < 2` → `< 3` | `a join of two files is not refused for its count` | The control refuses the *space* rather than running the job, so it proves execution cleared line 42 without touching the native engine. ### Also cold, and correctly so `ConversionNotifications.areEnabled()` (60–62) — **F5** in `docs/coverage-read-findings.md`, already merged. No callers; a test would pin dead code. Line 30 is the `build$default` bridge, an artifact. `MediaProbe.kt`'s residual lines are accounted for in #150's PR body (the FFprobe half and the two catch arms) and are not re-audited here. **Running mutation total across the batch: 50 run, 45 red.** The five green are all written down — three unfalsifiable guards recorded as named exemptions in test KDoc, one seam I had put in the wrong place (#143), and one bad mutation of mine that reached the same return by a different route.
JMR-dev commented 2026-08-27 12:00:08 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Follow-up wave filed as #153.

After this ticket's children were done I ran a repo-wide sweep rather than the three-file residual audit that closed this one out, and it found a coherent second wave — chiefly that neither ViewModel's WorkInfo → UI state machine has any test that chooses which branch it takes. Both observe blocks execute on every test that drives a real worker, so they never read as cold; what a real worker cannot produce is a RUNNING progress read, an ENQUEUED with a retry count, a SUCCEEDED with no output path, or a blank failure message.

The detail that made it worth filing: ReattachmentTest already drives all six WorkInfo.State constants — into Reattachment.choose, a different function that encodes the same enqueued-means-retry rule. So that rule is tested in one of its two homes, and the untested one is the copy the user's screen reads.

#153 carries the full accounting, including a table of what stays cold on purpose so the next sweep does not re-derive it.

**Follow-up wave filed as #153.** After this ticket's children were done I ran a **repo-wide** sweep rather than the three-file residual audit that closed this one out, and it found a coherent second wave — chiefly that neither ViewModel's `WorkInfo` → UI state machine has any test that *chooses* which branch it takes. Both `observe` blocks execute on every test that drives a real worker, so they never read as cold; what a real worker cannot produce is a `RUNNING` progress read, an `ENQUEUED` with a retry count, a `SUCCEEDED` with no output path, or a blank failure message. The detail that made it worth filing: `ReattachmentTest` already drives all six `WorkInfo.State` constants — into `Reattachment.choose`, a *different* function that encodes the same enqueued-means-retry rule. So that rule is tested in one of its two homes, and the untested one is the copy the user's screen reads. #153 carries the full accounting, including a table of what stays cold on purpose so the next sweep does not re-derive it.
JMR-dev commented 2026-09-02 01:41:10 +00:00 (Migrated from github.com)
Copy Link
Copy Source

All seven children closed. Five of them were finished on 2026-08-27 and stayed open for a bookkeeping reason worth recording.

#135, #136, #138, #139 and #141 each had a PR carrying Closes #NNN. GitHub only fires a closing keyword when the PR merges into the default branch — those five merged into their stack bases during the async-retarget race (#160, now in CLAUDE.md), so the keywords never ran. #160 restored the content to main but did not carry the keywords, so the work landed and the tickets stayed open.

That is the same failure as #160 wearing a different hat: MERGED was not evidence the work reached main, and a closed keyword is not evidence either. Worth knowing that the bookkeeping fails silently in exactly the same way the content did.

Verified before closing, against main at d354f64 — by re-running each ticket's own named mutation, not by checking files exist:

child mutation result
#135 each of three ?: return fallback → error(...) each reddens only its own test
#136 audio container guard disabled; COPY -> error arm removed both red
#138 cancellation arm throw e → Result.failure() red
#139 invalid-spec refusal block deleted red on both halves
#141 first-track-wins guards dropped, video and audio both red

Two of those needed adapting and the adaptation is on the tickets: #136's literal mutation cannot compile (codec is an AudioCodec, so CARRIES_VIDEO is a type error, not a behaviour change), and #141's line had moved when the seam was cut.

59 tests across the five classes, 0 failures. Full suite on main: 546 tests in 76 classes, 88.9% line, 75.4% branch.

**All seven children closed. Five of them were finished on 2026-08-27 and stayed open for a bookkeeping reason worth recording.** #135, #136, #138, #139 and #141 each had a PR carrying `Closes #NNN`. **GitHub only fires a closing keyword when the PR merges into the default branch** — those five merged into their stack bases during the async-retarget race (#160, now in `CLAUDE.md`), so the keywords never ran. #160 restored the content to `main` but did not carry the keywords, so the work landed and the tickets stayed open. That is the same failure as #160 wearing a different hat: **`MERGED` was not evidence the work reached `main`, and a closed keyword is not evidence either.** Worth knowing that the bookkeeping fails silently in exactly the same way the content did. Verified before closing, against `main` at `d354f64` — by re-running **each ticket's own named mutation**, not by checking files exist: | child | mutation | result | |---|---|---| | #135 | each of three `?: return fallback` → `error(...)` | each reddens **only its own** test | | #136 | audio container guard disabled; `COPY -> error` arm removed | both red | | #138 | cancellation arm `throw e` → `Result.failure()` | red | | #139 | invalid-spec refusal block deleted | red on **both** halves | | #141 | first-track-wins guards dropped, video and audio | both red | Two of those needed adapting and the adaptation is on the tickets: #136's literal mutation cannot compile (`codec` is an `AudioCodec`, so `CARRIES_VIDEO` is a type error, not a behaviour change), and #141's line had moved when the seam was cut. **59 tests across the five classes, 0 failures.** Full suite on `main`: 546 tests in 76 classes, 88.9% line, 75.4% branch.
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#132