Wave 2 test gaps: the ViewModel state machines nothing chooses a branch of #153

New Issue

Closed

opened 2026-08-27 11:58:43 +00:00 by JMR-dev · 2 comments

JMR-dev commented 2026-08-27 11:58:43 +00:00 (Migrated from github.com)

Copy Link

Copy Source

Parent for the second wave of test gaps, found by a repo-wide coverage sweep rather than the file-scoped audit that closed out #132.

How this was measured, and what it is measured against

./gradlew :app:jacocoTestReport over a tree with all eight of #132/#133's branches merged together: 87.0% line (2040/2344), 69.1% branch (974/1410).

At the time of filing #144–#151 are open, not merged. That does not weaken any item below: every gap here lives in ConversionViewModel.kt, JoinViewModel.kt, ConverterScreen.kt or JoinScreen.kt, and the batch touches none of those four files. The measurement is quoted from the integrated tree because that is the state these tickets will be worked against.

What separates this wave from the last one

#132's children came from a read of three files. This came from ranking every class by uncovered lines and branches, then asking of each one whether something already accounts for it. That surfaced a different kind of gap.

The last wave's items were mostly cold code. This wave's biggest item is not cold at all — it executes on every test that runs a worker. What is missing is that no test ever chooses which branch it takes. That is the argument #141 made for MediaProbe's track walk:

androidTest reaches it only through whatever the committed fixtures happen to contain — so none of the rules below is chosen by any test there.

Here the same shape appears against WorkManager instead of against a media fixture, and the answer is the same one #141 preferred: cut the branch matrix out as a pure function, leave the thin edge behind.

Children

#	what	shape
W1	ConversionViewModel.observe's state mapping — seam + the six arms nothing chooses	seam + tests
W2	JoinViewModel.observe's state mapping, and the arity guard above it	seam + tests
W3	The screen wiring: 17 action bindings a transposition currently survives	tests
W4	Seven ViewModel methods with no coverage at all, and two else arms	tests
W5	Three user-facing messages duplicated across layers	production + tests

W5 is listed last but should be worked first. It is the only one that is not a coverage item — it is a defect shape the sweep exposed — and W2 wants to pin a literal that W5 removes. Doing W2 first freezes the duplication in place and makes the eventual fix a three-file change with two tests to rewrite.

Suggested order: W5 → W1 → W2, with W3 and W4 independent of all of them. W1 before W2 so the two seams come out the same shape.

Accounted for, and not in scope

Recorded so the next sweep does not re-derive them:

still cold	why it stays that way
MediaProbe 35L/72B	the FFprobe half and the thin setDataSource/release edge — #150's PR body
AndroidDeviceCodecs.probe()	#86's boundary: MediaCodecInfoBuilder has no setIsAlias/setCanonicalName
Media3Engine, FFmpegEngine, ConcatEngine	native / device
ConversionForegroundType	settled by #88
FFmpegCommandBuilder:188 (Vorbis)	F1 in docs/coverage-read-findings.md — a test would pin dead code
ConversionWorker / ConcatWorker residuals	the residual-gap audit on #132
MainActivity.onCreate	enableEdgeToEdge + setContent. There is no decision in it
MainActivity.Content()	a two-arm when over a two-entry enum

The Compose branch counts are not 170 missing tests. ConverterScreenKt shows 110 missed branches and JoinScreenKt 60; most are compiler-synthesized $changed/$dirty skip checks, exactly as docs/coverage-read-findings.md records under its look-alike non-gaps. W3 is scoped by what the bindings do, not by that number.

Parent for the second wave of test gaps, found by a **repo-wide** coverage sweep rather than the file-scoped audit that closed out #132. ## How this was measured, and what it is measured against `./gradlew :app:jacocoTestReport` over a tree with all eight of #132/#133's branches merged together: **87.0% line (2040/2344), 69.1% branch (974/1410)**. **At the time of filing #144–#151 are open, not merged.** That does not weaken any item below: every gap here lives in `ConversionViewModel.kt`, `JoinViewModel.kt`, `ConverterScreen.kt` or `JoinScreen.kt`, and the batch touches none of those four files. The measurement is quoted from the integrated tree because that is the state these tickets will be worked against. ## What separates this wave from the last one #132's children came from a read of three files. This came from ranking *every* class by uncovered lines and branches, then asking of each one whether something already accounts for it. That surfaced a different kind of gap. The last wave's items were mostly **cold code**. This wave's biggest item is not cold at all — it executes on every test that runs a worker. What is missing is that **no test ever chooses which branch it takes**. That is the argument #141 made for `MediaProbe`'s track walk: > `androidTest` reaches it only through whatever the committed fixtures happen to contain — so none of the rules below is *chosen* by any test there. Here the same shape appears against `WorkManager` instead of against a media fixture, and the answer is the same one #141 preferred: cut the branch matrix out as a pure function, leave the thin edge behind. ## Children | # | what | shape | |---|---|---| | W1 | `ConversionViewModel.observe`'s state mapping — seam + the six arms nothing chooses | seam + tests | | W2 | `JoinViewModel.observe`'s state mapping, and the arity guard above it | seam + tests | | W3 | The screen wiring: 17 action bindings a transposition currently survives | tests | | W4 | Seven ViewModel methods with no coverage at all, and two `else` arms | tests | | W5 | Three user-facing messages duplicated across layers | production + tests | **W5 is listed last but should be worked first.** It is the only one that is not a coverage item — it is a defect shape the sweep exposed — and W2 wants to pin a literal that W5 removes. Doing W2 first freezes the duplication in place and makes the eventual fix a three-file change with two tests to rewrite. Suggested order: **W5 → W1 → W2**, with W3 and W4 independent of all of them. W1 before W2 so the two seams come out the same shape. ## Accounted for, and not in scope Recorded so the next sweep does not re-derive them: | still cold | why it stays that way | |---|---| | `MediaProbe` 35L/72B | the FFprobe half and the thin `setDataSource`/`release` edge — #150's PR body | | `AndroidDeviceCodecs.probe()` | #86's boundary: `MediaCodecInfoBuilder` has no `setIsAlias`/`setCanonicalName` | | `Media3Engine`, `FFmpegEngine`, `ConcatEngine` | native / device | | `ConversionForegroundType` | settled by #88 | | `FFmpegCommandBuilder:188` (Vorbis) | **F1** in `docs/coverage-read-findings.md` — a test would pin dead code | | `ConversionWorker` / `ConcatWorker` residuals | the residual-gap audit on #132 | | `MainActivity.onCreate` | `enableEdgeToEdge` + `setContent`. There is no decision in it | | `MainActivity.Content()` | a two-arm `when` over a two-entry enum | **The Compose branch counts are not 170 missing tests.** `ConverterScreenKt` shows 110 missed branches and `JoinScreenKt` 60; most are compiler-synthesized `$changed`/`$dirty` skip checks, exactly as `docs/coverage-read-findings.md` records under its look-alike non-gaps. W3 is scoped by what the bindings do, not by that number.

JMR-dev commented 2026-08-29 16:16:43 +00:00 (Migrated from github.com)

Copy Link

Copy Source

Wave 2 complete — all five children merged and verified on main

	PR	what it turned out to be
W5	#161	four duplicated messages, not three — a regex bound had hidden "Joining failed."
W1	#162	the six arms nothing chose; seam mirrors JobSnapshot
W2	#163	the join twin — plus a crash, see below
W4	#164	seven setters; three of my own tests were vacuous until the mutation pass said so
W3	#165	the wiring — this ticket's premise was half wrong, see #156

502 → 546 tests, 87.1% → 88.9% line, 69.1% → 75.4% branch. CLAUDE.md updated in #166.

What the wave produced beyond coverage

A real crash (W2). JoinViewModel read the join strategy with ConcatStrategy::valueOf, which throws inside a viewModelScope collect with no handler — so it took the process down rather than becoming a Failed state. Reachable on this wave's own premise: WorkManager keeps finished work a week, so a rollback hands this build a job naming a strategy it does not define. The repo had already made that exact fix one file over, in ConcatWorker.kt, with a comment explaining why — and nothing connected the two reads.

Three corrections to my own filings. W5's ticket named three duplicated strings and there were four; W3's ticket claimed seventeen transposable bindings and there are five, because every typed binding is already rejected by the compiler. Both are recorded on their tickets rather than only in the PRs.

The finding I would carry forward

Two of the five children found a defect rather than only a gap, and in both cases the defect had been sitting in plain sight inside a large block. valueOf was one line inside a 50-line when inside a collect; the transposable bindings were nine lines in an argument list. Neither was hidden — both were unreadable in place.

So the seam's value is not only that branches become testable. It is that a branch standing alone gets read. That is a better argument for the pure-seam pattern than "it raises coverage", and CLAUDE.md now carries the measurement half of it too: the branch denominator fell 1410 → 1340 as this landed, because extracting a when from a coroutine lambda deletes the state machine's synthesized branches around it.

Still open, filed during the wave

#159 — application-scope IO racing every Robolectric test that shares cacheDir. Found when #149's Unit tests leg failed once on CI and passed 500-odd times locally. Worked around in that PR's fixture with a retry loop; #159's done-when is that the loop can be deleted.

## Wave 2 complete — all five children merged and verified on `main` | | PR | what it turned out to be | |---|---|---| | W5 | #161 | **four** duplicated messages, not three — a regex bound had hidden `"Joining failed."` | | W1 | #162 | the six arms nothing chose; seam mirrors `JobSnapshot` | | W2 | #163 | the join twin — **plus a crash**, see below | | W4 | #164 | seven setters; three of my own tests were vacuous until the mutation pass said so | | W3 | #165 | the wiring — **this ticket's premise was half wrong**, see #156 | **502 → 546 tests, 87.1% → 88.9% line, 69.1% → 75.4% branch.** `CLAUDE.md` updated in #166. ### What the wave produced beyond coverage **A real crash (W2).** `JoinViewModel` read the join strategy with `ConcatStrategy::valueOf`, which throws inside a `viewModelScope` collect with no handler — so it took the process down rather than becoming a `Failed` state. Reachable on this wave's own premise: WorkManager keeps finished work a week, so a rollback hands this build a job naming a strategy it does not define. The repo had **already made that exact fix one file over**, in `ConcatWorker.kt`, with a comment explaining why — and nothing connected the two reads. **Three corrections to my own filings.** W5's ticket named three duplicated strings and there were four; W3's ticket claimed seventeen transposable bindings and there are five, because every typed binding is already rejected by the compiler. Both are recorded on their tickets rather than only in the PRs. ### The finding I would carry forward Two of the five children found a defect rather than only a gap, and in both cases **the defect had been sitting in plain sight inside a large block**. `valueOf` was one line inside a 50-line `when` inside a `collect`; the transposable bindings were nine lines in an argument list. Neither was hidden — both were unreadable in place. So the seam's value is not only that branches become testable. It is that a branch standing alone gets read. That is a better argument for the pure-seam pattern than "it raises coverage", and `CLAUDE.md` now carries the measurement half of it too: the branch denominator *fell* 1410 → 1340 as this landed, because extracting a `when` from a coroutine lambda deletes the state machine's synthesized branches around it. ### Still open, filed during the wave **#159** — application-scope IO racing every Robolectric test that shares `cacheDir`. Found when #149's `Unit tests` leg failed once on CI and passed 500-odd times locally. Worked around in that PR's fixture with a retry loop; #159's done-when is that the loop can be deleted.

JMR-dev commented 2026-09-02 01:41:44 +00:00 (Migrated from github.com)

Copy Link

Copy Source

All five children closed, and the wave is complete. Closing this parent.

Final state on main at d354f64: 502 → 546 tests, 87.1% → 88.9% line, 69.1% → 75.4% branch. CLAUDE.md re-measured in #166, including the note that the branch denominator fell 1410 → 1340 because extracting a when from a coroutine lambda deletes the state machine's synthesized branches — so the percentage moved for two reasons and only one of them is new tests.

What the wave produced beyond coverage, which is the part worth keeping:

• A crash (#155). JoinViewModel read the join strategy with ConcatStrategy::valueOf, which throws inside a viewModelScope collect with no handler. The repo had already made that exact fix one file over, in ConcatWorker.kt, with a comment explaining why — and nothing connected the two reads.
• Three corrections to my own filings. #158 named three duplicated strings and there were four (a regex bound hid "Joining failed."); #156 claimed seventeen transposable bindings and there are five, because every typed binding is already rejected by the compiler.

The through-line: two of five children found a defect rather than only a gap, and both defects were in plain sight inside a large block. valueOf was one line inside a 50-line when; the transposable bindings were nine lines in an argument list. Neither was hidden — both were unreadable in place. That is a better argument for the pure-seam pattern than coverage is.

Still open from this wave: #159, the application-scope IO race against every Robolectric test sharing cacheDir. Worked around in #149's fixture; its done-when is that the retry loop can be deleted.

**All five children closed, and the wave is complete.** Closing this parent. Final state on `main` at `d354f64`: **502 → 546 tests, 87.1% → 88.9% line, 69.1% → 75.4% branch.** `CLAUDE.md` re-measured in #166, including the note that the branch denominator *fell* 1410 → 1340 because extracting a `when` from a coroutine lambda deletes the state machine's synthesized branches — so the percentage moved for two reasons and only one of them is new tests. **What the wave produced beyond coverage**, which is the part worth keeping: - **A crash** (#155). `JoinViewModel` read the join strategy with `ConcatStrategy::valueOf`, which throws inside a `viewModelScope` collect with no handler. The repo had already made that exact fix one file over, in `ConcatWorker.kt`, with a comment explaining why — and nothing connected the two reads. - **Three corrections to my own filings.** #158 named three duplicated strings and there were four (a regex bound hid `"Joining failed."`); #156 claimed seventeen transposable bindings and there are five, because every typed binding is already rejected by the compiler. The through-line: **two of five children found a defect rather than only a gap, and both defects were in plain sight inside a large block.** `valueOf` was one line inside a 50-line `when`; the transposable bindings were nine lines in an argument list. Neither was hidden — both were unreadable in place. That is a better argument for the pure-seam pattern than coverage is. Still open from this wave: **#159**, the application-scope IO race against every Robolectric test sharing `cacheDir`. Worked around in #149's fixture; its done-when is that the retry loop can be deleted.

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

fix/102-picker-back-press-overshoot

fix/268-saf-picker-determinism

feat/ogg-vorbis-libvorbis

feat/expedited-conversion-work

fix/gate-cache-in-worktrees

chore/gate-runs-shellcheck

test/publish-delete-arm-real-provider

docs/e8-instrumented-coverage

docs/api37-carrier-count-drift

docs/e7-second-constraint

test/publish-to-a-real-saf-destination

fix/api37-report-match-line

fix/api37-task-snapshot-crash

test/join-failure-message-on-device

docs/e2e-read-findings-e7

test/cancelling-a-running-export

test/reattach-to-a-running-job

test/content-uri-reaches-ffmpeg

fix/launcher-wiring-waits-for-the-pick

fix/cancel-tests-need-a-slower-encode

test/cancelling-a-running-join

test/cancelling-a-running-session

test/notification-cancel-action

test/ffmpeg-progress-is-observed

test/fallback-asserts-the-path

test/flac-and-opus-assert-their-format

docs/e2e-read-findings

docs/wave4-coverage-numbers

fix/injectable-startup-sweep-scope

test/session-outcome-seam

test/launcher-callback-identity

test/theme-follows-system-dark

test/audio-drop-arm

fix/rotation-waits-for-recreation

fix/convert-guards-on-ready

test/retry-save-mime

test/hardware-progress-reaches-workmanager

test/ffprobe-mapping-seam

test/device-codec-enumeration-seam

test/unknown-container-row

test/null-message-fallbacks

test/cancel-reaches-workmanager

docs/coverage-wave3-recovery

test/concat-engine-seam

docs/coverage-wave3

test/mediaprobe-merge-seam

test/adaptive-shell-wiring

test/aac-audio-args

test/notification-progress-text

test/media3-muxer-guard

test/hardware-fallback-and-cancellation

test/unprobeable-join-clip

test/one-branch-outcomes

test/foreground-type-regimes

fix/bound-wedge-diagnostics

docs/coverage-wave2

test/screen-wiring

test/viewmodel-setters

test/join-state-mapping

test/conversion-state-mapping

test/dedupe-user-messages

fix/restore-stack-merges

test/refused-jobs

test/concatworker-failure-arms

test/container-capabilities-audio

test/readspec-enum-fallbacks

test/outputpublisher-seams

test/mediaprobe-track-seam

test/outputpublisher-partial-branches

test/fake-provider-scaffolding

docs/coverage-read-findings

chore/gitignore-kotlin

test/bound-the-hangs

docs/coverage-remeasure

ci/baseline-counter-precision

fix/invalid-suggestion-chip

ci/wedged-leg-report

fix/reattachment-overwrites-pick

test/theme-live-branches

fix/failed-save-retry

fix/empty-composition-crash

ci/advisory-failure-report

docs/seven-run-counts

test/release-permission-guard

ci/build-workflow-permissions

docs/api37-point-release

docs/benchmark-populate-path

fix/dead-assertion-probe-test

ci/actionlint

test/device-codecs-encode-consequence

fix/sdkmanager-pipefail

docs/readme-restart-claim

fix/saf-picker-root-discovery

fix/probe-dispatcher-seam

test/media3engine-mime-tables

test/mediaprobe-pure-helpers

fix/codec-vocabulary-drift

docs/robolectric-rationale-correction

docs/api37-advisory-counts

test/r38-8-saf-e2e

test/r38-7-join-states

test/r38-6-conversion-states

test/r38-5-state-seam

fix/jacoco-robolectric-coverage

docs/instrumented-tests-correction

test/r38-2-filecard

test/r38-4-advanced-picker

test/r38-3-pickers

tools/file-issue-script

tools/api-37-emulator

fix/review-app-gaps

docs/review-corrections

No results found.

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

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#153