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

R38.1 — Make the screen leaves nameable from a test, and give tests a tag vocabulary #57

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

Child 1 of 8 decomposing #52. Blocks R38.2, R38.3, R38.4 and R38.8 — pick this up first.

Why this exists

#52 cannot be started as filed. Kotlin private on a top-level declaration is file-scoped,
so every leaf composable in the two screens is invisible even to the JVM test source set, which is a
friend of main. The only three declarations src/test can name today are ConverterScreen,
JoinScreen and AppRoot. There is nothing to write a test against.

This child makes the leaves nameable and gives every later child a node-location vocabulary. No
behaviour change, no restructuring. The when (state) extraction is deliberately not here — it
is R38.5, and it lands after the leaves are under test.

Scope

1. private -> internal, one word per declaration:

file declarations
convert/ConverterScreen.kt FormatPicker :238, AdvancedPicker :268, ValidationError :332, QualityPicker :375, EnginePicker :391, FileCard :420, DetailRow :483
convert/ConverterScreen.kt (pure) describe :362, EnginePreference.label :406, formatDuration :492, formatBytes :499
join/JoinScreen.kt FileRow :182

In-tree precedent, with the reasoning already written down at MainActivity.kt:36-38:

internal rather than private so the unit tests can name a tab. The JVM test source set is a
friend of main, so this stays invisible to anything outside the module.

2. A tag table in main. One object of tag constants, applied via Modifier.testTag(...) to each
affordance, so tests reference a symbol and never a literal. This is what keeps the sibling children
independent: "Cancel", "Start over" and "Save file" each appear in both screens and in several
state branches, so a reword would otherwise redden five PRs at once.

Make it public, not internal, and say why in its KDoc. R38.8 needs these constants from
androidTest, and whether androidTest is a friend source set of main under AGP 9 is
unverified — no main declaration is currently referenced from androidTest, so the repo carries
no evidence either way. Settle it with compileDebugAndroidTestKotlin in this child and write the
answer in the KDoc. Public is the safe default.

Lands with

A smoke test per leaf: it renders, and its tag resolves to exactly one node. Not the state
matrix — that is R38.6/R38.7.

Acceptance — the mutation

Delete one testTag modifier -> that leaf's smoke test goes red. Restore.

A visibility-and-tags PR with no test would fail the norm in #51 on its own terms.

Size

~120 lines. Near-zero risk by design; the risky change is R38.5.

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 1 of 8 decomposing #52. **Blocks R38.2, R38.3, R38.4 and R38.8** — pick this up first._ ### Why this exists `#52` cannot be started as filed. **Kotlin `private` on a top-level declaration is *file*-scoped**, so every leaf composable in the two screens is invisible even to the JVM test source set, which is a friend of `main`. The only three declarations `src/test` can name today are `ConverterScreen`, `JoinScreen` and `AppRoot`. There is nothing to write a test against. This child makes the leaves nameable and gives every later child a node-location vocabulary. **No behaviour change, no restructuring.** The `when (state)` extraction is deliberately *not* here — it is R38.5, and it lands after the leaves are under test. ### Scope **1. `private` -> `internal`**, one word per declaration: | file | declarations | |---|---| | `convert/ConverterScreen.kt` | `FormatPicker` :238, `AdvancedPicker` :268, `ValidationError` :332, `QualityPicker` :375, `EnginePicker` :391, `FileCard` :420, `DetailRow` :483 | | `convert/ConverterScreen.kt` (pure) | `describe` :362, `EnginePreference.label` :406, `formatDuration` :492, `formatBytes` :499 | | `join/JoinScreen.kt` | `FileRow` :182 | In-tree precedent, with the reasoning already written down at `MainActivity.kt:36-38`: > `internal` rather than `private` so the unit tests can name a tab. The JVM test source set is a > friend of `main`, so this stays invisible to anything outside the module. **2. A tag table in main.** One object of tag constants, applied via `Modifier.testTag(...)` to each affordance, so tests reference a symbol and never a literal. This is what keeps the sibling children independent: `"Cancel"`, `"Start over"` and `"Save file"` each appear in both screens and in several state branches, so a reword would otherwise redden five PRs at once. > **Make it public, not `internal`, and say why in its KDoc.** R38.8 needs these constants from > `androidTest`, and **whether `androidTest` is a friend source set of `main` under AGP 9 is > unverified** — no main declaration is currently referenced from `androidTest`, so the repo carries > no evidence either way. Settle it with `compileDebugAndroidTestKotlin` in this child and write the > answer in the KDoc. Public is the safe default. ### Lands with A smoke test per leaf: it renders, and its tag resolves to **exactly one** node. Not the state matrix — that is R38.6/R38.7. ### Acceptance — the mutation Delete one `testTag` modifier -> that leaf's smoke test goes red. Restore. A visibility-and-tags PR with no test would fail the norm in #51 on its own terms. ### Size ~120 lines. Near-zero risk by design; the risky change is R38.5. --- ### 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#57