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
No Branch/Tag Specified
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
accessibility
backlog
bug
confirmed
documentation
duplicate
enhancement
good first issue
help wanted
invalid
plausible
question
sev:high
sev:low
sev:medium
wontfix
Worked autonomously overnight: local, JVM-verifiable, no product decision
Barrier affecting people with disabilities
Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here
Something isn't working
Reviewer demonstrated the defect
Improvements or additions to documentation
This issue or pull request already exists
New feature or request
Good for newcomers
Extra attention is needed
This doesn't seem right
Reviewer could not fully demonstrate it; treat as unproven
Further information is requested
High severity
Low severity
Medium severity
This will not be worked on
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: JMR-dev/LibreMediaConverter#52
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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.ktMeasured on
main@9c4f142(jacoco, LINE):ConverterScreenKtJoinScreenKtMainActivityKtConverterScreenKtis 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 becauseui-test-junit4runs under Robolectric, and Robolectric is pinned at 4.16.1. Exactly two classes use it —AppRootRestorationTestandDestinationSaverTest, 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:ConversionStaterenders its own affordances:Readyoffers Convert only whenvalidation.isValid;Convertingoffers Cancel;Convertedoffers Save + Start over;Failedshows the messageJoinStateE2E (
tools/local-emulator/run-e2e.sh, API 33–36) — anything the JVM cannot honestly assert: real SAF picker round-trips, and rotation against a realBundlerather thanStateRestorationTester's in-memory map.Known limit to design around
StateRestorationTestersaves to an in-memory map, not aBundle. It discriminatesrememberfromrememberSaveable— which is the bite — but would pass equally with an ordinal orautoSaver. 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
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:
private. Kotlinprivateon a top-level declaration isfile-scoped, so the eight helpers in
ConverterScreen.ktandFileRowinJoinScreen.ktareinvisible even to the JVM test source set, which is a friend of
main. The only threedeclarations
src/testcan name areConverterScreen,JoinScreenandAppRoot.when (state)inside thepublic entry point, so this issue's stated scope — "each
ConversionStaterenders its ownaffordances" — needs a real
ConversionViewModel, andWaiting(needs a foreground denial) andConverted(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
*ScreenContentcomposables restructures 300+ lines ofzero-coverage UI, and R38.2–R38.4 put the leaves it moves under test before it runs:
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.mdrecords (46 mutations,9 vacuous, five passing the whole suite over a completely unguarded path).
Two decisions taken while scoping
testTag, in a shared table in main (#57), rather than extracting the 61hardcoded
Text()literals tostrings.xml. Tests reference a symbol, so a reword cannot reddenfive sibling PRs. There are currently zero
testTag,semanticsorcontentDescriptioncallsanywhere in
app/src/main.name — its bite would be
rememberSaveable→remember, whichAppRootRestorationTestalreadycatches 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:128andJoinScreen.kt:94(is Idle -> Unitin the nestedwhen) arepermanently unreachable — the outer
whenalready peeledIdleoff. jacoco reports them missedforever. Do not "fix" them by deleting the outer branch.
ThemeKt— 23 lines, 0 covered, in the UI package but outside this issue's scope.Note on the line references in the children: they were captured against
1779f20(mainat the time of filing). R38.1 (#57) adds atestTagmodifier to every affordance and R38.5 (#61) moves 300+ lines wholesale, so by the time #62 and #63 are picked up,ConverterScreen.kt:149is notenabled = validation.isValidany 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.mdhit, which is why the audit's own verification step says to re-check each cited line before trusting it.Correcting the measurement this issue was filed on — see #75 and PR #76.
The headline number here was an artifact. "
ConverterScreenKt0 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:
ConverterScreenKtMainActivityKtJoinScreenKtWhat this does and does not change:
ValidationErrorrendered outside theAnimatedVisibility, and theJOIN_MOREtag 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.JoinScreenKtat 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".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
4375a37ConverterScreenKtJoinScreenKtMainActivityKtJVM suite: 373 tests in 53 classes. Plus a real SAF picker round-trip and a real-
Bundlerotation inandroidTest, 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 "
ConverterScreenKt0 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:
JOIN_MOREgivenJOIN's value was caught only by the tag-table uniqueness test; every per-leaf test stayed green.ValidationErrorinside theAnimatedVisibilityreddened three cases, whileConverterLeafTagsTeststayed green: it calls the leaf directly and cannot see where the call site sits.TestTags.*multiset againstmaininstead — identical, 26 refs converter / 11 join.ConversionViewModelholds the picked file in a plainMutableStateFlowwith noSavedStateHandle. Only the Activity's retainedViewModelStorecarries 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 andJoinScreen.kt'sis Idle -> Unitin the nestedwhen— permanently unreachable; the outerwhenpeelsIdleoff first. jacoco reports them missed forever. Do not "fix" them.Failed's error colour (#62) andHorizontalDivider'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.OpenMultipleDocumentsandCreateDocument(#64) — the Join picker and the save dialog. Different tickets.Follow-ups this opened
#66 (probe dispatcher seam), #68 (theme branches), #70 (actionlint), #74 (
describeAudioNUL), #75 (closed by #76), #81 (the advisory job's name no longer describes what it runs).