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

R38.5 — Extract the state seam both screens lack, with the leaf tests as the net #61

New Issue
Closed
opened 2026-08-23 23:06:59 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-08-23 23:06:59 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Child 5 of 8 decomposing #52. Depends on R38.2, R38.3 and R38.4 — deliberately. Blocks R38.6 and R38.7.

The risky cut, sequenced so it is not taken blind

#52's stated scope — "each ConversionState renders its own affordances" — has no injection
point. Both screens inline the whole when (state) inside the public entry point
(ConverterScreen.kt:103-234, JoinScreen.kt:69-178), and state arrives only via viewModel.state.
Driving it through a real ConversionViewModel means a WorkManager and a media probe in the
constructor, and Waiting (needs a foreground denial) and Converted (needs a full worker success)
are not reachable that way at all.

So the seam has to be cut. This restructures 300+ lines of UI, and it is the one change in the whole
of #52 that can silently break rendering. That is why it is fifth and not first: R38.2-R38.4 put
the leaves this extraction moves under test, so a regression surfaces here instead of in production.

Scope

Extract ConverterScreenContent and JoinScreenContent — internal @Composable, taking state /
settings / validation plus an actions holder, containing the when. The entry points keep only
the launchers and collectAsStateWithLifecycle and pass callbacks down. Untested residue in each
entry point drops to ~30 lines of launcher wiring, which only R38.8 can reach.

The actions must be a holder, not loose parameters. ConverterScreenContent needs ~12 callbacks
and detekt's LongParameterList sits at its default threshold of 6. config/detekt/detekt.yml
relaxes LongMethod, CyclomaticComplexMethod and TooManyFunctions for Composable but not
this rule, and AdvancedPicker already sits exactly at 6. A flat signature turns a currently-clean
detekt run red. Use internal data class ConverterActions(val onPickInput: () -> Unit, ...).

Acceptance — the mutation

This child's own bite is that the seam actually injects: pass a ConversionState.Converted to
ConverterScreenContent and assert Save renders. That assertion is impossible before this
extraction and cannot be reached through a ViewModel.

Then the regression gate: the full R38.2-R38.4 suite must stay green, unchanged. If any of those
tests needs editing to keep passing, the extraction changed behaviour — that is the signal to stop,
not to update the test.

Size

~180 lines, mostly moved rather than new.

Traps (shared across the R38 children)

  • The import pair is mixed: androidx.compose.ui.test.junit4.v2.createComposeRule (v2) but
    androidx.compose.ui.test.junit4.StateRestorationTester (non-v2). Every tutorial shows the
    non-v2 rule. Copy both lines from AppRootRestorationTest.
  • @UnstableApi propagates. Both screens carry it, so a test class touching them needs it or the
    build fails on UnsafeOptInUsageError.
  • @RunWith(RobolectricTestRunner::class) and nothing else. There is no @Config in this repo;
    sdk=36 is set once in app/src/test/resources/robolectric.properties because Robolectric 4.16.1
    has no android-all jar for API 37.
  • DetailRow renders "$label: $value" as one node (ConverterScreen.kt:486).
    onNodeWithText("Container") will not match — use the full string or substring = true.
  • Typographic characters retyped as ASCII fail silently: … U+2026, — U+2014, × U+00D7,
    · U+00B7. Copy them out of the source.
  • Assertions are org.junit.Assert.*, statically imported one per symbol. No kotlin.test, no
    Truth, no mockk — test doubles are hand-written subclasses.
  • Open the test class with a KDoc naming the defect it bites on. House style, and it is what makes
    the mutation check reviewable by someone else.

Done means

./gradlew :app:assembleDebug :app:testDebugUnitTest :app:compileDebugAndroidTestKotlin :app:ktlintCheck :app:detekt :app:lintDebug --continue

then the mutation above: revert the named line, watch the named test go red, restore, and quote what
the failure said. Per CLAUDE.md, own branch, own PR, never on main.

Parent: #52

_Child 5 of 8 decomposing #52. **Depends on R38.2, R38.3 and R38.4 — deliberately.** Blocks R38.6 and R38.7._ ### The risky cut, sequenced so it is not taken blind `#52`'s stated scope — "each `ConversionState` renders its own affordances" — has **no injection point**. Both screens inline the whole `when (state)` inside the public entry point (`ConverterScreen.kt:103-234`, `JoinScreen.kt:69-178`), and state arrives only via `viewModel.state`. Driving it through a real `ConversionViewModel` means a `WorkManager` and a media probe in the constructor, and **`Waiting` (needs a foreground denial) and `Converted` (needs a full worker success) are not reachable that way at all**. So the seam has to be cut. This restructures 300+ lines of UI, and it is the one change in the whole of #52 that can silently break rendering. **That is why it is fifth and not first**: R38.2-R38.4 put the leaves this extraction moves under test, so a regression surfaces here instead of in production. ### Scope Extract `ConverterScreenContent` and `JoinScreenContent` — `internal @Composable`, taking `state` / `settings` / `validation` plus an actions holder, containing the `when`. The entry points keep only the launchers and `collectAsStateWithLifecycle` and pass callbacks down. Untested residue in each entry point drops to ~30 lines of launcher wiring, which only R38.8 can reach. **The actions must be a holder, not loose parameters.** `ConverterScreenContent` needs ~12 callbacks and **detekt's `LongParameterList` sits at its default threshold of 6**. `config/detekt/detekt.yml` relaxes `LongMethod`, `CyclomaticComplexMethod` and `TooManyFunctions` for `Composable` but **not** this rule, and `AdvancedPicker` already sits exactly at 6. A flat signature turns a currently-clean detekt run red. Use `internal data class ConverterActions(val onPickInput: () -> Unit, ...)`. ### Acceptance — the mutation This child's own bite is that the seam actually injects: pass a `ConversionState.Converted` to `ConverterScreenContent` and assert Save renders. **That assertion is impossible before this extraction** and cannot be reached through a ViewModel. Then the regression gate: **the full R38.2-R38.4 suite must stay green, unchanged.** If any of those tests needs editing to keep passing, the extraction changed behaviour — that is the signal to stop, not to update the test. ### Size ~180 lines, mostly moved rather than new. --- ### Traps (shared across the R38 children) - **The import pair is mixed**: `androidx.compose.ui.test.junit4.v2.createComposeRule` (**v2**) but `androidx.compose.ui.test.junit4.StateRestorationTester` (**non-v2**). Every tutorial shows the non-v2 rule. Copy both lines from `AppRootRestorationTest`. - **`@UnstableApi` propagates.** Both screens carry it, so a test class touching them needs it or the build fails on `UnsafeOptInUsageError`. - **`@RunWith(RobolectricTestRunner::class)` and nothing else.** There is no `@Config` in this repo; `sdk=36` is set once in `app/src/test/resources/robolectric.properties` because Robolectric 4.16.1 has no `android-all` jar for API 37. - **`DetailRow` renders `"$label: $value"` as one node** (`ConverterScreen.kt:486`). `onNodeWithText("Container")` will not match — use the full string or `substring = true`. - **Typographic characters retyped as ASCII fail silently**: `…` U+2026, `—` U+2014, `×` U+00D7, `·` U+00B7. Copy them out of the source. - **Assertions are `org.junit.Assert.*`, statically imported one per symbol.** No `kotlin.test`, no Truth, no mockk — test doubles are hand-written subclasses. - **Open the test class with a KDoc naming the defect it bites on.** House style, and it is what makes the mutation check reviewable by someone else. ### Done means `./gradlew :app:assembleDebug :app:testDebugUnitTest :app:compileDebugAndroidTestKotlin :app:ktlintCheck :app:detekt :app:lintDebug --continue` then the mutation above: revert the named line, watch the named test go red, restore, and quote what the failure said. Per `CLAUDE.md`, own branch, own PR, never on `main`. Parent: #52
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:medium
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#61