R38 — The Compose screens have ~0% coverage, and the Robolectric harness for it already exists unused #52

Closed
opened 2026-08-23 13:51:08 +00:00 by JMR-dev · 4 comments
JMR-dev commented 2026-08-23 13:51:08 +00:00 (Migrated from github.com)

Filed after the overnight run. Scoped against the project norm added in #51: unit-testable code gets unit tests, e2e-testable code gets e2e tests, both before done.

R38 — The Compose screens have ~0% coverage, and the harness to fix that already exists unused

severity: medium
verdict: CONFIRMED (measured)
where: app/src/main/java/org/libremediaconverter/convert/ConverterScreen.kt,
app/src/main/java/org/libremediaconverter/join/JoinScreen.kt,
app/src/main/java/org/libremediaconverter/MainActivity.kt

Measured on main @ 9c4f142 (jacoco, LINE):

file covered missed
ConverterScreenKt 0 293
JoinScreenKt 0 98
MainActivityKt 3 41

ConverterScreenKt is the single largest block of uncovered lines in the codebase.

The capability is already present and paid for. testImplementation(libs.compose.ui.test.junit4) is wired into the JVM source set specifically because ui-test-junit4 runs under Robolectric, and Robolectric is pinned at 4.16.1. Exactly two classes use it — AppRootRestorationTest and DestinationSaverTest, both from the D6 rotation fix. Nothing else touches the screens.

Evidence that the gap is invisible from inside: while fixing D5, a stream reported "ConverterScreen's 'Size unknown' has no test: the repo has no Compose test in either source set, so one would be new infrastructure." That was already false — D6's harness had landed on its base. A real testing capability went unused because its existence was not discoverable. Same class of error as the stale documentation R14/R15 corrected.

Scope

Unit (Robolectric + createComposeRule) — the state-to-UI contract, which is pure rendering logic:

  • each ConversionState renders its own affordances: Ready offers Convert only when validation.isValid; Converting offers Cancel; Converted offers Save + Start over; Failed shows the message
  • the same for JoinState
  • "Size unknown" — the D5 case that prompted this, currently untested
  • the validation error + suggestion chips, which are the only route to an invalid spec

E2E (tools/local-emulator/run-e2e.sh, API 33–36) — anything the JVM cannot honestly assert: real SAF picker round-trips, and rotation against a real Bundle rather than StateRestorationTester's in-memory map.

Known limit to design around

StateRestorationTester saves to an in-memory map, not a Bundle. It discriminates remember from rememberSaveable — which is the bite — but would pass equally with an ordinal or autoSaver. D6 handled this by pinning the saved representation in separate pure JVM tests. Any restoration test written here needs the same split, or it will assert less than it appears to.

Risk

Compose tests that assert on layout rather than behaviour are brittle and get deleted within months. Assert on what the user can do — which affordances exist in which state — not on structure or pixels. Prove each test bites by reverting the branch it covers.


Cut: backlog — deliberately not started unattended. This is a sizeable piece of new test surface, and the norm in #51 sets its bar.

🤖 Generated with Claude Code

_Filed after the overnight run. Scoped against the project norm added in #51: unit-testable code gets unit tests, e2e-testable code gets e2e tests, both before done._ ### R38 — The Compose screens have ~0% coverage, and the harness to fix that already exists unused severity: medium verdict: CONFIRMED (measured) where: `app/src/main/java/org/libremediaconverter/convert/ConverterScreen.kt`, `app/src/main/java/org/libremediaconverter/join/JoinScreen.kt`, `app/src/main/java/org/libremediaconverter/MainActivity.kt` **Measured on `main` @ `9c4f142` (jacoco, LINE):** | file | covered | missed | |---|---|---| | `ConverterScreenKt` | **0** | 293 | | `JoinScreenKt` | **0** | 98 | | `MainActivityKt` | 3 | 41 | `ConverterScreenKt` is the single largest block of uncovered lines in the codebase. **The capability is already present and paid for.** `testImplementation(libs.compose.ui.test.junit4)` is wired into the **JVM** source set specifically because `ui-test-junit4` runs under Robolectric, and Robolectric is pinned at 4.16.1. Exactly two classes use it — `AppRootRestorationTest` and `DestinationSaverTest`, both from the D6 rotation fix. Nothing else touches the screens. **Evidence that the gap is invisible from inside:** while fixing D5, a stream reported *"`ConverterScreen`'s 'Size unknown' has no test: the repo has no Compose test in either source set, so one would be new infrastructure."* That was already false — D6's harness had landed on its base. A real testing capability went unused because its existence was not discoverable. Same class of error as the stale documentation R14/R15 corrected. ### Scope **Unit (Robolectric + `createComposeRule`)** — the state-to-UI contract, which is pure rendering logic: - each `ConversionState` renders its own affordances: `Ready` offers Convert only when `validation.isValid`; `Converting` offers Cancel; `Converted` offers Save + Start over; `Failed` shows the message - the same for `JoinState` - **"Size unknown"** — the D5 case that prompted this, currently untested - the validation error + suggestion chips, which are the only route to an invalid spec **E2E (`tools/local-emulator/run-e2e.sh`, API 33–36)** — anything the JVM cannot honestly assert: real SAF picker round-trips, and rotation against a real `Bundle` rather than `StateRestorationTester`'s in-memory map. ### Known limit to design around `StateRestorationTester` saves to an in-memory map, **not a `Bundle`**. It discriminates `remember` from `rememberSaveable` — which is the bite — but would pass equally with an ordinal or `autoSaver`. D6 handled this by pinning the saved *representation* in separate pure JVM tests. Any restoration test written here needs the same split, or it will assert less than it appears to. ### Risk Compose tests that assert on layout rather than behaviour are brittle and get deleted within months. Assert on **what the user can do** — which affordances exist in which state — not on structure or pixels. Prove each test bites by reverting the branch it covers. --- **Cut:** `backlog` — deliberately not started unattended. This is a sizeable piece of new test surface, and the norm in #51 sets its bar. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
JMR-dev commented 2026-08-23 23:08:28 +00:00 (Migrated from github.com)

Decomposed into eight children — this issue is now a tracking issue

Two facts made this unstartable as filed, and they set the shape of the split:

  1. Every leaf composable is private. Kotlin private on a top-level declaration is
    file-scoped, so the eight helpers in ConverterScreen.kt and FileRow in JoinScreen.kt are
    invisible even to the JVM test source set, which is a friend of main. The only three
    declarations src/test can name are ConverterScreen, JoinScreen and AppRoot.
  2. There is no injection point for state. Both screens inline the whole when (state) inside the
    public entry point, so this issue's stated scope — "each ConversionState renders its own
    affordances" — needs a real ConversionViewModel, and Waiting (needs a foreground denial) and
    Converted (needs a full worker success) are not reachable that way at all.

So the first move is a seam, not a test. Strangler ordering, with the risky cut deliberately fifth
rather than first
— extracting the two *ScreenContent composables restructures 300+ lines of
zero-coverage UI, and R38.2–R38.4 put the leaves it moves under test before it runs:

#57  R38.1  internal leaves + tag table        (near-zero risk; blocks 58, 59, 60, 64)
  ├── #58  R38.2  FileCard + formatters        ← "Size unknown", the case that prompted this
  ├── #59  R38.3  Format/Quality/Engine        ─┐ parallel
  └── #60  R38.4  AdvancedPicker + errors      ─┘
        └── #61  R38.5  extract *ScreenContent (the risky cut — now under test)
              ├── #62  R38.6  Converter states ─┐ parallel
              └── #63  R38.7  Join states      ─┘
#64  R38.8  e2e: SAF round-trip + real-Bundle rotation   (needs #57's tags only)

Every child names the line to revert and the assertion that must go red. No child's acceptance
criterion is a coverage delta. If R38.1–R38.7 land, ~400 of the 432 missed lines become reachable and
total line coverage moves 29.8% → roughly 48% — but that number materialises from rendering the
screens, so it appears whether or not a single assertion bites. On this ticket coverage is the signal
most likely to read green over vacuous tests, which is the failure CLAUDE.md records (46 mutations,
9 vacuous, five passing the whole suite over a completely unguarded path).

Two decisions taken while scoping

  • Node location is testTag, in a shared table in main (#57), rather than extracting the 61
    hardcoded Text() literals to strings.xml. Tests reference a symbol, so a reword cannot redden
    five sibling PRs. There are currently zero testTag, semantics or contentDescription calls
    anywhere in app/src/main.
  • The e2e half is one ticket, not two. A rotation test alone has no mutation of its own to
    name — its bite would be rememberSaveable → remember, which AppRootRestorationTest already
    catches on the JVM. #64 drives the picker first so the rotation runs against a real input and a real
    Activity-scoped ViewModel, which is a bite nothing in the repo has.

Explicitly not covered, so it is a decision rather than an oversight

  • ConverterScreen.kt:128 and JoinScreen.kt:94 (is Idle -> Unit in the nested when) are
    permanently unreachable — the outer when already peeled Idle off. jacoco reports them missed
    forever. Do not "fix" them by deleting the outer branch.
  • ThemeKt — 23 lines, 0 covered, in the UI package but outside this issue's scope.
  • The ~30 residual lines of launcher wiring in each entry point after #61. Reachable only by #64.
## Decomposed into eight children — this issue is now a tracking issue Two facts made this unstartable as filed, and they set the shape of the split: 1. **Every leaf composable is `private`.** Kotlin `private` on a top-level declaration is *file*-scoped, so the eight helpers in `ConverterScreen.kt` and `FileRow` in `JoinScreen.kt` are invisible even to the JVM test source set, which is a friend of `main`. The only three declarations `src/test` can name are `ConverterScreen`, `JoinScreen` and `AppRoot`. 2. **There is no injection point for state.** Both screens inline the whole `when (state)` inside the public entry point, so this issue's stated scope — "each `ConversionState` renders its own affordances" — needs a real `ConversionViewModel`, and `Waiting` (needs a foreground denial) and `Converted` (needs a full worker success) are not reachable that way at all. So the first move is a seam, not a test. **Strangler ordering, with the risky cut deliberately fifth rather than first** — extracting the two `*ScreenContent` composables restructures 300+ lines of zero-coverage UI, and R38.2–R38.4 put the leaves it moves under test before it runs: ``` #57 R38.1 internal leaves + tag table (near-zero risk; blocks 58, 59, 60, 64) ├── #58 R38.2 FileCard + formatters ← "Size unknown", the case that prompted this ├── #59 R38.3 Format/Quality/Engine ─┐ parallel └── #60 R38.4 AdvancedPicker + errors ─┘ └── #61 R38.5 extract *ScreenContent (the risky cut — now under test) ├── #62 R38.6 Converter states ─┐ parallel └── #63 R38.7 Join states ─┘ #64 R38.8 e2e: SAF round-trip + real-Bundle rotation (needs #57's tags only) ``` **Every child names the line to revert and the assertion that must go red.** No child's acceptance criterion is a coverage delta. If R38.1–R38.7 land, ~400 of the 432 missed lines become reachable and total line coverage moves 29.8% → roughly 48% — but that number materialises from *rendering* the screens, so it appears whether or not a single assertion bites. On this ticket coverage is the signal most likely to read green over vacuous tests, which is the failure `CLAUDE.md` records (46 mutations, 9 vacuous, five passing the whole suite over a completely unguarded path). ### Two decisions taken while scoping - **Node location is `testTag`**, in a shared table in main (#57), rather than extracting the 61 hardcoded `Text()` literals to `strings.xml`. Tests reference a symbol, so a reword cannot redden five sibling PRs. There are currently **zero** `testTag`, `semantics` or `contentDescription` calls anywhere in `app/src/main`. - **The e2e half is one ticket, not two.** A rotation test alone has no mutation of its own to name — its bite would be `rememberSaveable` → `remember`, which `AppRootRestorationTest` already catches on the JVM. #64 drives the picker first so the rotation runs against a real input and a real Activity-scoped ViewModel, which is a bite nothing in the repo has. ### Explicitly not covered, so it is a decision rather than an oversight - `ConverterScreen.kt:128` and `JoinScreen.kt:94` (`is Idle -> Unit` in the nested `when`) are **permanently unreachable** — the outer `when` already peeled `Idle` off. jacoco reports them missed forever. Do not "fix" them by deleting the outer branch. - `ThemeKt` — 23 lines, 0 covered, in the UI package but outside this issue's scope. - The ~30 residual lines of launcher wiring in each entry point after #61. Reachable only by #64.
JMR-dev commented 2026-08-23 23:08:52 +00:00 (Migrated from github.com)

Note on the line references in the children: they were captured against 1779f20 (main at the time of filing). R38.1 (#57) adds a testTag modifier to every affordance and R38.5 (#61) moves 300+ lines wholesale, so by the time #62 and #63 are picked up, ConverterScreen.kt:149 is not enabled = validation.isValid any more.

Re-locate by symbol, not by number. The line numbers are there to make the first read fast, not to be followed literally — the same trap docs/defect-audit.md hit, which is why the audit's own verification step says to re-check each cited line before trusting it.

Note on the line references in the children: they were captured against `1779f20` (`main` at the time of filing). **R38.1 (#57) adds a `testTag` modifier to every affordance and R38.5 (#61) moves 300+ lines wholesale**, so by the time #62 and #63 are picked up, `ConverterScreen.kt:149` is not `enabled = validation.isValid` any more. **Re-locate by symbol, not by number.** The line numbers are there to make the first read fast, not to be followed literally — the same trap `docs/defect-audit.md` hit, which is why the audit's own verification step says to re-check each cited line before trusting it.
JMR-dev commented 2026-08-24 23:46:01 +00:00 (Migrated from github.com)

Correcting the measurement this issue was filed on — see #75 and PR #76.

The headline number here was an artifact. "ConverterScreenKt 0 covered / 293 missed" was never evidence the screens were untested by the JVM suite; it is what JaCoCo reports for any Robolectric-exercised class in this repo. Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo skips no-location classes by default, and nothing in the build said otherwise — so no Robolectric test has ever counted here.

With that fixed, same commit and same tests:

as this issue measured actually
repo LINE 29.8% 69.2%
ConverterScreenKt 0.0% 62.8%
MainActivityKt 6.8% 86.4%
JoinScreenKt 0.0% 9.2%

What this does and does not change:

  • The work was still worth doing, and the children still bit. #57, #58, #59 and #60 each named a mutation and each went red on it — including two cases (ValidationError rendered outside the AnimatedVisibility, and the JOIN_MORE tag collision) that no other test in the suite caught. Coverage was the wrong reason to do it; the reasons in the children were the right ones.
  • The "~0% coverage" framing was wrong, and the decomposition plan's projection of "29.8% → roughly 48%" was wrong in both directions — the real starting point was higher, and the route there was not the one described.
  • JoinScreenKt at 9.2% is now the honest remaining gap, and #63 is the child that closes it. That is a real number rather than an artifact for the first time.

Recording this here so nobody re-derives the original conclusion from the archived issue. The instruction that caught it was already written in CLAUDE.md — "re-measure before quoting it".

Correcting the measurement this issue was filed on — see #75 and PR #76. **The headline number here was an artifact.** "`ConverterScreenKt` 0 covered / 293 missed" was never evidence the screens were untested by the JVM suite; it is what JaCoCo reports for any Robolectric-exercised class in this repo. Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo skips no-location classes by default, and nothing in the build said otherwise — so **no Robolectric test has ever counted here.** With that fixed, same commit and same tests: | | as this issue measured | actually | |---|---|---| | repo LINE | 29.8% | **69.2%** | | `ConverterScreenKt` | 0.0% | **62.8%** | | `MainActivityKt` | 6.8% | **86.4%** | | `JoinScreenKt` | 0.0% | 9.2% | **What this does and does not change:** - The work was still worth doing, and the children still bit. #57, #58, #59 and #60 each named a mutation and each went red on it — including two cases (`ValidationError` rendered outside the `AnimatedVisibility`, and the `JOIN_MORE` tag collision) that no other test in the suite caught. Coverage was the wrong reason to do it; the reasons in the children were the right ones. - **The "~0% coverage" framing was wrong**, and the decomposition plan's projection of "29.8% → roughly 48%" was wrong in both directions — the real starting point was higher, and the route there was not the one described. - `JoinScreenKt` at **9.2%** is now the honest remaining gap, and #63 is the child that closes it. That is a real number rather than an artifact for the first time. Recording this here so nobody re-derives the original conclusion from the archived issue. The instruction that caught it was already written in `CLAUDE.md` — "re-measure before quoting it".
JMR-dev commented 2026-08-25 02:25:31 +00:00 (Migrated from github.com)

All eight children are merged and closed — #57, #58, #59, #60, #61, #62, #63, #64. Closing this as complete.

What the screens look like now, measured on 4375a37

at filing now
ConverterScreenKt 0 / 293 90.8%
JoinScreenKt 0 / 98 84.7%
MainActivityKt 3 / 41 86.4%
repo LINE / BRANCH 29.8% / 28.7% 78.6% / 56.7%

JVM suite: 373 tests in 53 classes. Plus a real SAF picker round-trip and a real-Bundle rotation in androidTest, neither of which existed in any form before.

This issue's headline measurement was wrong, and that is worth recording

JaCoCo had never counted a single Robolectric test in this repo (#75, fixed in #76). Robolectric's sandbox classloader gives classes no source location and JaCoCo skips those by default. So "ConverterScreenKt 0 covered / 293 missed" was never evidence the screens were untested — it is what JaCoCo reported for any Robolectric-exercised class, no matter what.

The 29.8% -> 48% projection in the decomposition plan was wrong in both directions: the real starting point was higher, and the route was not the one described.

The work was still right, for reasons that had nothing to do with the number. Every child named a mutation and went red on it, and four of those caught things no other test in the suite could:

  • #57 — JOIN_MORE given JOIN's value was caught only by the tag-table uniqueness test; every per-leaf test stayed green.
  • #60 — moving ValidationError inside the AnimatedVisibility reddened three cases, while ConverterLeafTagsTest stayed green: it calls the leaf directly and cannot see where the call site sits.
  • #61 — the safety net had a structural hole its agent found and closed: every net test composes a leaf, none touches an entry point, so a tag dropped from a whole state branch would have passed everything. It diffed the TestTags.* multiset against main instead — identical, 26 refs converter / 11 join.
  • #64 — the rotation mutation this issue called unverifiable is constructible, and it bites because ConversionViewModel holds the picked file in a plain MutableStateFlow with no SavedStateHandle. Only the Activity's retained ViewModelStore carries it across a rotation, and nothing asserted that.

Named exemptions, so their absence is a decision

  • ThemeKt — 23 lines, still 0%. Out of scope here and owned by #68, which found something better than missing coverage: two of its four colour-scheme branches are unreachable and its KDoc promises a switch with no caller.
  • ConverterScreen.kt's and JoinScreen.kt's is Idle -> Unit in the nested when — permanently unreachable; the outer when peels Idle off first. jacoco reports them missed forever. Do not "fix" them.
  • Failed's error colour (#62) and HorizontalDivider's presence (#58) — Compose publishes neither to the semantics tree, so no JVM test can observe them. The text is asserted; the styling is not.
  • OpenMultipleDocuments and CreateDocument (#64) — the Join picker and the save dialog. Different tickets.

Follow-ups this opened

#66 (probe dispatcher seam), #68 (theme branches), #70 (actionlint), #74 (describeAudio NUL), #75 (closed by #76), #81 (the advisory job's name no longer describes what it runs).

All eight children are merged and closed — #57, #58, #59, #60, #61, #62, #63, #64. Closing this as complete. ## What the screens look like now, measured on `4375a37` | | at filing | now | |---|---|---| | `ConverterScreenKt` | 0 / 293 | **90.8%** | | `JoinScreenKt` | 0 / 98 | **84.7%** | | `MainActivityKt` | 3 / 41 | **86.4%** | | repo LINE / BRANCH | 29.8% / 28.7% | **78.6% / 56.7%** | JVM suite: **373 tests in 53 classes.** Plus a real SAF picker round-trip and a real-`Bundle` rotation in `androidTest`, neither of which existed in any form before. ## This issue's headline measurement was wrong, and that is worth recording **JaCoCo had never counted a single Robolectric test in this repo** (#75, fixed in #76). Robolectric's sandbox classloader gives classes no source location and JaCoCo skips those by default. So "`ConverterScreenKt` 0 covered / 293 missed" was never evidence the screens were untested — it is what JaCoCo reported for *any* Robolectric-exercised class, no matter what. The `29.8% -> 48%` projection in the decomposition plan was wrong in both directions: the real starting point was higher, and the route was not the one described. **The work was still right, for reasons that had nothing to do with the number.** Every child named a mutation and went red on it, and four of those caught things no other test in the suite could: - **#57** — `JOIN_MORE` given `JOIN`'s value was caught **only** by the tag-table uniqueness test; every per-leaf test stayed green. - **#60** — moving `ValidationError` inside the `AnimatedVisibility` reddened three cases, while `ConverterLeafTagsTest` stayed green: it calls the leaf directly and cannot see where the call site sits. - **#61** — the safety net had a structural hole its agent found and closed: every net test composes a **leaf**, none touches an entry point, so a tag dropped from a whole state branch would have passed everything. It diffed the `TestTags.*` multiset against `main` instead — identical, 26 refs converter / 11 join. - **#64** — the rotation mutation this issue called unverifiable **is** constructible, and it bites because `ConversionViewModel` holds the picked file in a plain `MutableStateFlow` with no `SavedStateHandle`. Only the Activity's retained `ViewModelStore` carries it across a rotation, and nothing asserted that. ## Named exemptions, so their absence is a decision - **`ThemeKt`** — 23 lines, still 0%. Out of scope here and owned by **#68**, which found something better than missing coverage: two of its four colour-scheme branches are unreachable and its KDoc promises a switch with no caller. - **`ConverterScreen.kt`'s and `JoinScreen.kt`'s `is Idle -> Unit`** in the nested `when` — permanently unreachable; the outer `when` peels `Idle` off first. jacoco reports them missed forever. Do not "fix" them. - **`Failed`'s error colour** (#62) and **`HorizontalDivider`'s presence** (#58) — Compose publishes neither to the semantics tree, so no JVM test can observe them. The text is asserted; the styling is not. - **`OpenMultipleDocuments`** and **`CreateDocument`** (#64) — the Join picker and the save dialog. Different tickets. ## Follow-ups this opened #66 (probe dispatcher seam), #68 (theme branches), #70 (actionlint), #74 (`describeAudio` NUL), #75 (closed by #76), #81 (the advisory job's name no longer describes what it runs).
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#52