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

W3: the screen wiring -- 17 bindings a transposition currently survives #156

New Issue
Closed
opened 2026-08-27 11:59:05 +00:00 by JMR-dev · 1 comment
JMR-dev commented 2026-08-27 11:59:05 +00:00 (Migrated from github.com)
Copy Link
Copy Source

ConverterScreen (60–113) and JoinScreen (40–68) are the stateful outer composables. Their stateless inners are tested thoroughly — ConverterScreenContentTest, ConverterStateAffordancesTest, JoinScreenContentTest — and cover 349 and 123 lines respectively. The outers are untested.

The failure mode, which is specific

The outer's job is almost entirely wiring: it collects state, remembers two activity-result launchers, and hands ConverterActions / JoinActions a set of callbacks. There are 17 bindings across the two files, 11 of them bare method references:

onPreset          = viewModel::setPreset,
onContainer       = viewModel::setContainer,
onVideoCodec      = viewModel::setVideoCodec,
onAudioCodec      = viewModel::setAudioCodec,
onSuggestion      = viewModel::applySuggestion,
onQuality         = viewModel::setQuality,
onEnginePreference= viewModel::setEnginePreference,
onCancel          = viewModel::cancel,
onReset           = viewModel::reset,

Because the content tests construct ConverterActions themselves, they never see this list. Transpose onVideoCodec and onAudioCodec and every test in the repo still passes; the app ships a codec picker that sets the other codec. The types are identical where it matters — setContainer/applySuggestion differ, but the six enum setters are mutually swappable in pairs and onCancel/onReset are both () -> Unit.

That is the thing to test. Not "does the screen render" — ConverterScreenContentTest answers that already.

Shape

Robolectric plus compose-ui-test-junit4 is in the JVM source set, so this is a unit test. Drive the real ConverterScreen against a ConversionViewModel whose method calls are observable, click each affordance by its TestTags entry, and assert which method arrived.

The activity-result launchers are the awkward part and are not the subject: pickInput, chooseDestination and requestNotifications need an activity result to do anything. Cover the nine method-reference bindings, and name the three launcher-backed ones (onPickInput, onConvert, onSave) as out of scope with that reason — SafPickerRoundTripTest already exercises the pick path end to end on a device.

destinationMime at 80 is worth one test of its own. It reads state.pendingSave()?.mimeType ?: settings.spec.mimeType, and the comment above it records that the fallback is wrong for a reattached job — a retry after a failed save must reuse the type its first attempt used. The JoinScreen twin at 52 falls back to ConcatWorker.DEFAULT_FORMAT.mimeType.

Done when

Each of the nine bindings is pinned by a test that fails when that binding alone is transposed to a neighbour of the same type. Run that mutation — swapping two bindings — rather than trusting a click test to imply it. A test that clicks the video-codec control and asserts "some setter ran" is exactly the vacuous shape CLAUDE.md warns about.

`ConverterScreen` (60–113) and `JoinScreen` (40–68) are the stateful outer composables. Their stateless inners are tested thoroughly — `ConverterScreenContentTest`, `ConverterStateAffordancesTest`, `JoinScreenContentTest` — and cover 349 and 123 lines respectively. **The outers are untested.** ## The failure mode, which is specific The outer's job is almost entirely wiring: it collects state, remembers two activity-result launchers, and hands `ConverterActions` / `JoinActions` a set of callbacks. There are **17 bindings** across the two files, 11 of them bare method references: ```kotlin onPreset = viewModel::setPreset, onContainer = viewModel::setContainer, onVideoCodec = viewModel::setVideoCodec, onAudioCodec = viewModel::setAudioCodec, onSuggestion = viewModel::applySuggestion, onQuality = viewModel::setQuality, onEnginePreference= viewModel::setEnginePreference, onCancel = viewModel::cancel, onReset = viewModel::reset, ``` Because the content tests construct `ConverterActions` themselves, **they never see this list.** Transpose `onVideoCodec` and `onAudioCodec` and every test in the repo still passes; the app ships a codec picker that sets the other codec. The types are identical where it matters — `setContainer`/`applySuggestion` differ, but the six enum setters are mutually swappable in pairs and `onCancel`/`onReset` are both `() -> Unit`. That is the thing to test. Not "does the screen render" — `ConverterScreenContentTest` answers that already. ## Shape Robolectric plus `compose-ui-test-junit4` is in the JVM source set, so this is a unit test. Drive the real `ConverterScreen` against a `ConversionViewModel` whose method calls are observable, click each affordance by its `TestTags` entry, and assert which method arrived. The activity-result launchers are the awkward part and are **not** the subject: `pickInput`, `chooseDestination` and `requestNotifications` need an activity result to do anything. Cover the nine method-reference bindings, and name the three launcher-backed ones (`onPickInput`, `onConvert`, `onSave`) as out of scope with that reason — `SafPickerRoundTripTest` already exercises the pick path end to end on a device. `destinationMime` at 80 is worth one test of its own. It reads `state.pendingSave()?.mimeType ?: settings.spec.mimeType`, and the comment above it records that the fallback is wrong for a reattached job — a retry after a failed save must reuse the type its first attempt used. The `JoinScreen` twin at 52 falls back to `ConcatWorker.DEFAULT_FORMAT.mimeType`. ## Done when Each of the nine bindings is pinned by a test that fails when that binding alone is transposed to a neighbour of the same type. **Run that mutation — swapping two bindings — rather than trusting a click test to imply it.** A test that clicks the video-codec control and asserts "some setter ran" is exactly the vacuous shape `CLAUDE.md` warns about.
JMR-dev commented 2026-08-29 15:57:04 +00:00 (Migrated from github.com)
Copy Link
Copy Source

PR #165. This ticket's central claim was half wrong, and it is worth correcting here rather than only in the PR.

I filed it saying a transposition of any two of the seventeen bindings would survive the suite. Most of them do not compile. Measured:

swap result
onVideoCodec ↔ onAudioCodec rejected — Inapplicable candidate(s): fun setAudioCodec(codec: AudioCodec)
onCancel ↔ onReset compiles

Every typed binding takes a distinct parameter type — Container, VideoCodec, AudioCodec, OutputFormat, OutputSpec, QualityTier, EnginePreference — so the compiler already rejects those swaps. Testing them would have been theatre, and this ticket would have bought it.

The real hazard is the () -> Unit bindings, which are interchangeable to the compiler: two on the converter screen and three on the join screen (onJoin, onCancel, onReset). That is five bindings rather than seventeen — narrower, but sharper, and the join trio is the worse of the two because a Join button bound to cancel reads as a dead button.

Two process notes, since #153 has more children than this:

  1. The claim came from reading the types, not from trying it. (VideoCodec) -> Unit and (AudioCodec) -> Unit look interchangeable when you are scanning a list of nine viewModel:: lines; they are not. One compileDebugKotlin settled it.
  2. My first attempt to check it also got it wrong, in the other direction — a grep-and-exit-code test reported the onCancel/onReset swap as rejected because it read a stale build. Running it again and looking at the actual output printed BUILD SUCCESSFUL. A check worth doing is worth reading the output of.
**PR #165.** This ticket's central claim was half wrong, and it is worth correcting here rather than only in the PR. I filed it saying a transposition of any two of the seventeen bindings would survive the suite. **Most of them do not compile.** Measured: | swap | result | |---|---| | `onVideoCodec` ↔ `onAudioCodec` | **rejected** — `Inapplicable candidate(s): fun setAudioCodec(codec: AudioCodec)` | | `onCancel` ↔ `onReset` | **compiles** | Every typed binding takes a distinct parameter type — `Container`, `VideoCodec`, `AudioCodec`, `OutputFormat`, `OutputSpec`, `QualityTier`, `EnginePreference` — so the compiler already rejects those swaps. Testing them would have been theatre, and this ticket would have bought it. **The real hazard is the `() -> Unit` bindings**, which are interchangeable to the compiler: two on the converter screen and *three* on the join screen (`onJoin`, `onCancel`, `onReset`). That is five bindings rather than seventeen — narrower, but sharper, and the join trio is the worse of the two because a Join button bound to `cancel` reads as a dead button. Two process notes, since #153 has more children than this: 1. **The claim came from reading the types, not from trying it.** `(VideoCodec) -> Unit` and `(AudioCodec) -> Unit` look interchangeable when you are scanning a list of nine `viewModel::` lines; they are not. One `compileDebugKotlin` settled it. 2. **My first attempt to check it also got it wrong**, in the other direction — a grep-and-exit-code test reported the `onCancel`/`onReset` swap as rejected because it read a stale build. Running it again and looking at the actual output printed `BUILD SUCCESSFUL`. A check worth doing is worth reading the output of.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
above-cut
Worked autonomously overnight: local, JVM-verifiable, no product decision
 accessibility
Barrier affecting people with disabilities
 backlog
Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here
 bug
Something isn't working
 confirmed
Reviewer demonstrated the defect
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 plausible
Reviewer could not fully demonstrate it; treat as unproven
 question
Further information is requested
 sev:high
High severity
 sev:low
Low severity
 sev:medium
Medium severity
 wontfix
This will not be worked on
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
JMR-dev
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#156