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

R38.8 — E2E: the SAF picker round-trip, and rotation against a real Bundle #64

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

Child 8 of 8 decomposing #52. Depends on R38.1 for the tag table only — independent of R38.5.

Why the picker and the rotation are one ticket

A rotation test on its own has no mutation of its own to name. Its bite would be
rememberSaveable -> remember, which AppRootRestorationTest already catches on the JVM. A child
whose acceptance criterion duplicates an existing test's is exactly the vacuous-test failure mode
this decomposition exists to prevent.

Driving the picker first fixes that. It leaves the app in Ready with a real input and a real
Activity-scoped ViewModel, and rotating from there is a bite nothing in the repo has:
AppRootRestorationTest injects a stub content lambda specifically to avoid standing up either
ViewModel, and StateRestorationTester saves to an in-memory map rather than a Bundle.

Scope

  • Add androidx.test.uiautomator to gradle/libs.versions.toml. Its androidx. group means the
    existing componentSelection prerelease guard in app/build.gradle.kts already covers it — no new
    pinning argument needed.
  • Drive OpenDocument / OpenMultipleDocuments / CreateDocument through the real system picker and
    assert the round trip lands in the ViewModel. Nothing in either source set exercises SAF as a
    picker     today; the only SAF coverage is the publish side, in OutputPublisherPublishTest against
    hand-written ContentProvider fakes.
  • Then rotate, and assert the picked input survives — real Bundle, real Activity recreation, real
    ViewModel.

Two environment facts that decide where this runs

  • This is the first Activity launched anywhere in androidTest. No test there currently uses
    ActivityScenario, createAndroidComposeRule or Espresso — though compose-ui-test-junit4 and
    espresso-core are both already on the classpath and ui-test-manifest is on debugImplementation,
    so the wiring exists.
  • Must run on an unlocked emulator, not the Pixel. The Pixel 10 Pro XL is secure-locked and cannot
    be unlocked from a shell, so the picker cannot be driven there. That is precisely why this gap
    survived. tools/local-emulator/run-e2e.sh covers API 33-36 locally.

May need @FailsOnEmulatorApi37 — the marker is on main as of 1779f20 (PR #56), in the root
org.libremediaconverter package, and CI reads it twice (notAnnotation for the gating leg,
annotation for the advisory one).

Acceptance — the mutation

Change the MIME filter at ConverterScreen.kt:116 from arrayOf("*/*") to something the fixture
does not match -> the picker test goes red.

Second bite: make the input ViewModel composition-scoped rather than Activity-scoped -> the rotation
test goes red while every JVM test stays green. That divergence is this ticket's whole reason to
exist; if it does not hold, say so on the ticket rather than shipping a test that proves nothing.

Size

~250 lines plus a build change. The one child that could reasonably be split again if it runs long.

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 8 of 8 decomposing #52. **Depends on R38.1** for the tag table only — independent of R38.5._ ### Why the picker and the rotation are one ticket A rotation test on its own **has no mutation of its own to name.** Its bite would be `rememberSaveable` -> `remember`, which `AppRootRestorationTest` already catches on the JVM. A child whose acceptance criterion duplicates an existing test's is exactly the vacuous-test failure mode this decomposition exists to prevent. Driving the picker first fixes that. It leaves the app in `Ready` with a **real input and a real Activity-scoped ViewModel**, and rotating from there is a bite nothing in the repo has: `AppRootRestorationTest` injects a stub `content` lambda *specifically* to avoid standing up either ViewModel, and `StateRestorationTester` saves to an in-memory map rather than a `Bundle`. ### Scope - **Add `androidx.test.uiautomator`** to `gradle/libs.versions.toml`. Its `androidx.` group means the existing `componentSelection` prerelease guard in `app/build.gradle.kts` already covers it — no new pinning argument needed. - Drive `OpenDocument` / `OpenMultipleDocuments` / `CreateDocument` through the real system picker and assert the round trip lands in the ViewModel. Nothing in either source set exercises SAF **as a picker** today; the only SAF coverage is the publish side, in `OutputPublisherPublishTest` against hand-written `ContentProvider` fakes. - Then rotate, and assert the picked input survives — real `Bundle`, real Activity recreation, real ViewModel. ### Two environment facts that decide where this runs - **This is the first Activity launched anywhere in `androidTest`.** No test there currently uses `ActivityScenario`, `createAndroidComposeRule` or Espresso — though `compose-ui-test-junit4` and `espresso-core` are both already on the classpath and `ui-test-manifest` is on `debugImplementation`, so the wiring exists. - **Must run on an unlocked emulator, not the Pixel.** The Pixel 10 Pro XL is secure-locked and cannot be unlocked from a shell, so the picker cannot be driven there. That is precisely why this gap survived. `tools/local-emulator/run-e2e.sh` covers API 33-36 locally. May need `@FailsOnEmulatorApi37` — the marker is on `main` as of `1779f20` (PR #56), in the root `org.libremediaconverter` package, and CI reads it twice (`notAnnotation` for the gating leg, `annotation` for the advisory one). ### Acceptance — the mutation Change the MIME filter at `ConverterScreen.kt:116` from `arrayOf("*/*")` to something the fixture does not match -> the picker test goes red. Second bite: make the input ViewModel composition-scoped rather than Activity-scoped -> the rotation test goes red **while every JVM test stays green**. That divergence is this ticket's whole reason to exist; if it does not hold, say so on the ticket rather than shipping a test that proves nothing. ### Size ~250 lines plus a build change. The one child that could reasonably be split again if it runs long. --- ### 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
JMR-dev commented 2026-08-23 23:09:04 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Flagging one thing before anyone starts this: the second mutation above is asserted, not verified.

"Make the input ViewModel composition-scoped rather than Activity-scoped" is not a one-line revert. viewModel() inside a Composable resolves through LocalViewModelStoreOwner, which is the Activity — changing that scope may not be expressible without restructuring the call site. And if the ViewModel is recreated on rotation, ConversionState returns to Idle and the assertion becomes trivially different rather than a genuine mutation.

That matters because it is this ticket's whole stated reason for existing: the rotation half was folded in here precisely so it would have a bite that AppRootRestorationTest does not already catch on the JVM.

Establish that the mutation is constructible before writing the test. If it is not, the honest outcomes are either a different mutation that only a real Bundle + real Activity recreation can catch, or dropping the rotation half and keeping this ticket to the SAF round-trip alone — which has its own solid bite (the MIME filter at ConverterScreen.kt:116). Do not ship a rotation test that proves nothing; say which way it went on this ticket.

Nothing here blocks the SAF half, and nothing blocks #57–#63.

Flagging one thing before anyone starts this: **the second mutation above is asserted, not verified.** "Make the input ViewModel composition-scoped rather than Activity-scoped" is not a one-line revert. `viewModel()` inside a Composable resolves through `LocalViewModelStoreOwner`, which is the Activity — changing that scope may not be expressible without restructuring the call site. And if the ViewModel is recreated on rotation, `ConversionState` returns to `Idle` and the assertion becomes trivially different rather than a genuine mutation. That matters because **it is this ticket's whole stated reason for existing**: the rotation half was folded in here precisely so it would have a bite that `AppRootRestorationTest` does not already catch on the JVM. **Establish that the mutation is constructible before writing the test.** If it is not, the honest outcomes are either a different mutation that only a real `Bundle` + real Activity recreation can catch, or dropping the rotation half and keeping this ticket to the SAF round-trip alone — which has its own solid bite (the MIME filter at `ConverterScreen.kt:116`). Do not ship a rotation test that proves nothing; say which way it went on this ticket. Nothing here blocks the SAF half, and nothing blocks #57–#63.
JMR-dev commented 2026-08-24 21:15:49 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Finding from #60 (PR #71) that lands directly on this ticket's premise.

What crosses the Bundle is not a Boolean.

rememberSaveable { mutableStateOf(false) } passes no stateSaver, so autoSaver saves the MutableState object itself — the registry hands back an androidx.compose.runtime.ParcelableSnapshotMutableState. It survives a rotation only because mutableStateOf on Android returns a Parcelable implementation; the plain-JVM one is not.

This ticket is scoped as "rotation against a real Bundle, not StateRestorationTester's in-memory map". Whoever picks it up will otherwise write that test assuming a primitive goes through and be confused when the round trip produces a state holder. It is pinned in AdvancedPanelSavedStateTest, which drives a real SaveableStateRegistry with canBeSaved = { true } — deliberately, so a dropped entry cannot masquerade as the remember regression — plus a real Bundle/Parcel round trip. Read that test before writing this one; the pattern is already there.

Also relevant to the second mutation on this ticket, which I flagged earlier as asserted-not-verified: the same test class shows the shape of a saved-state assertion that genuinely bites (expected:<1> but was:<0> when rememberSaveable is downgraded to remember). If the ViewModel-scoping mutation turns out not to be constructible, a registry-level assertion of this kind may be the honest substitute — but only if it asserts something the JVM test cannot, which is the bar this ticket has to clear.

Finding from #60 (PR #71) that lands directly on this ticket's premise. **What crosses the `Bundle` is not a `Boolean`.** `rememberSaveable { mutableStateOf(false) }` passes no `stateSaver`, so `autoSaver` saves the **`MutableState` object itself** — the registry hands back an `androidx.compose.runtime.ParcelableSnapshotMutableState`. It survives a rotation only because `mutableStateOf` on Android returns a `Parcelable` implementation; the plain-JVM one is not. This ticket is scoped as "rotation against a real `Bundle`, not `StateRestorationTester`'s in-memory map". Whoever picks it up will otherwise write that test assuming a primitive goes through and be confused when the round trip produces a state holder. It is pinned in `AdvancedPanelSavedStateTest`, which drives a real `SaveableStateRegistry` with `canBeSaved = { true }` — deliberately, so a dropped entry cannot masquerade as the `remember` regression — plus a real `Bundle`/`Parcel` round trip. Read that test before writing this one; the pattern is already there. Also relevant to the second mutation on this ticket, which I flagged earlier as asserted-not-verified: the same test class shows the shape of a saved-state assertion that genuinely bites (`expected:<1> but was:<0>` when `rememberSaveable` is downgraded to `remember`). If the ViewModel-scoping mutation turns out not to be constructible, a registry-level assertion of this kind may be the honest substitute — but only if it asserts something the JVM test cannot, which is the bar this ticket has to clear.
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#64