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

R38.4 — AdvancedPicker's expand gate, and the suggestion chips that render outside it #60

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

Child 4 of 8 decomposing #52. Depends on R38.1. The subtlest of the leaf children.

Scope

AdvancedPicker is not stateless. It holds var expanded by rememberSaveable { mutableStateOf(false) }
at :278, and the Container/Video/Audio chip rows sit inside AnimatedVisibility(visible = expanded)
at :284. A test must click "Advanced" before any of those chips is findable — this is the
thing that will waste an afternoon if it is not written down.

ValidationError renders even while the section is collapsed. It is invoked at :327-329,
outside the AnimatedVisibility block. So the suggestion chips — which #52 calls "the only route
to an invalid spec" — are reachable without expanding anything. That is deliberate and load-bearing:
it is how a user with an invalid spec is offered a way out without first discovering the Advanced
section. Completely untested today.

Cover:

  • collapsed -> Container/Video/Audio chips absent; expand -> present; collapse -> absent again
  • an Validation.Invalid renders its message and its suggestion chips while collapsed
  • clicking a suggestion chip emits the right OutputSpec through onSuggestion
  • the messages come from ContainerCapabilities.validate — e.g. "MP4 cannot hold VP9 video.",
    "This would produce an empty file — keep at least one track.". Build them through validate
    rather than constructing Validation.Invalid by hand where practical, so the test bites on the
    real pairing.

This child owns the StateRestorationTester caveat

#52 records the limit as a general note, which is how notes get skipped. It is assigned here,
because expanded at :278 is the only rememberSaveable any of these eight children touch.

StateRestorationTester saves into an in-memory map, not a Bundle. It discriminates remember
from rememberSaveable — that is the bite — but would pass equally with an ordinal or autoSaver.
Follow the split DestinationSaverTest established: pin the saved representation in a separate
pure JVM test, rather than assuming the restoration test covers it.

Acceptance — the mutation

Move the ValidationError call at :327 inside the AnimatedVisibility block -> the
"suggestions are reachable while collapsed" test goes red.

Second bite: rememberSaveable -> remember at :278 -> the restoration test goes red.

Size

~170 lines across two files. Independent of R38.2 and R38.3.

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 4 of 8 decomposing #52. **Depends on R38.1.** The subtlest of the leaf children._ ### Scope **`AdvancedPicker` is not stateless.** It holds `var expanded by rememberSaveable { mutableStateOf(false) }` at `:278`, and the Container/Video/Audio chip rows sit inside `AnimatedVisibility(visible = expanded)` at `:284`. **A test must click `"Advanced"` before any of those chips is findable** — this is the thing that will waste an afternoon if it is not written down. **`ValidationError` renders even while the section is collapsed.** It is invoked at `:327-329`, *outside* the `AnimatedVisibility` block. So the suggestion chips — which #52 calls "the only route to an invalid spec" — are reachable without expanding anything. That is deliberate and load-bearing: it is how a user with an invalid spec is offered a way out without first discovering the Advanced section. Completely untested today. Cover: - collapsed -> Container/Video/Audio chips absent; expand -> present; collapse -> absent again - an `Validation.Invalid` renders its message and its suggestion chips **while collapsed** - clicking a suggestion chip emits the right `OutputSpec` through `onSuggestion` - the messages come from `ContainerCapabilities.validate` — e.g. `"MP4 cannot hold VP9 video."`, `"This would produce an empty file — keep at least one track."`. Build them through `validate` rather than constructing `Validation.Invalid` by hand where practical, so the test bites on the real pairing. ### This child owns the `StateRestorationTester` caveat #52 records the limit as a general note, which is how notes get skipped. **It is assigned here**, because `expanded` at `:278` is the only `rememberSaveable` any of these eight children touch. `StateRestorationTester` saves into an **in-memory map, not a `Bundle`**. It discriminates `remember` from `rememberSaveable` — that is the bite — but would pass equally with an ordinal or `autoSaver`. Follow the split `DestinationSaverTest` established: pin the saved *representation* in a separate pure JVM test, rather than assuming the restoration test covers it. ### Acceptance — the mutation Move the `ValidationError` call at `:327` inside the `AnimatedVisibility` block -> the "suggestions are reachable while collapsed" test goes red. Second bite: `rememberSaveable` -> `remember` at `:278` -> the restoration test goes red. ### Size ~170 lines across two files. Independent of R38.2 and R38.3. --- ### 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#60