wtclaude
6305d86a01
Phase 8 was scoped as docs, coverage and the cutover. The AC-5 walk turned it into a phase with code in it, which is the outcome the walk existed to produce - so the record is longer than a phase-8 note would normally be. THEMING_AND_NAV.md gains "Phase 8 as landed": what the walk confirmed, the two defects it found and why they are phase 2's trap repeating, the role sweep that says which Material roles are still unmapped and which of them have a reader, the coverage-glob narrowing, and the adb-reverse rig note for the next walk. The phase 2 note is amended where it claimed the card shadow was the milestone's only visible change to an untouched instance - it is now one of three, and the third (an untouched instance drawing a hero, because brand.hero carries the env default) is a correction to the record with no code behind it. PLAN.md's M12 entry stopped at phase 2; it now runs through 6, records phase 8 including the defects, and drops the same "one deliberate change" claim. COVERAGE_PLAN.md gains an amendment section explaining why the ui/theme/** directory glob became wrong the moment M12 put three pure resolvers behind it, and the general rule it suggests: prefer file globs to directory globs once a directory is mixed. Its own §5 already forbade excluding testable code. Pairs with Android-app phase 8. Co-Authored-By: Claude <noreply@anthropic.com>
230 lines
12 KiB
Markdown
230 lines
12 KiB
Markdown
# Android App — Test Coverage Plan
|
||
|
||
**Goal:** clear the SonarQube coverage quality gate (`new_coverage ≥ 50%`) for
|
||
`Runic-Gateway-Android-app`, and leave a durable unit-test culture behind it. Companion to
|
||
[`PLAN.md`](./PLAN.md) §12.1 (the JaCoCo wiring that made coverage measurable).
|
||
|
||
## 1. Current state (2026-07-22, post `Android-app#26`)
|
||
|
||
The JaCoCo→Sonar wiring is live on `main`, so coverage is now real — and the gate is **failing**:
|
||
|
||
| Metric | Value |
|
||
|---|---|
|
||
| Quality gate | **ERROR** (one condition) |
|
||
| `new_coverage` | **16.4%** (threshold ≥ 50%) |
|
||
| overall `coverage` | 16.3% |
|
||
| lines to cover | 3,188 |
|
||
| covered | 542 |
|
||
|
||
Everything else on the gate is green (reliability/security/maintainability **A**, duplication 0%).
|
||
Because this is the *first* measured version, Sonar's "new code" window is essentially the whole
|
||
codebase, so `new_coverage ≈ overall coverage` — to pass we need to roughly **triple** covered
|
||
lines, from 542 to ~1,600.
|
||
|
||
### Where the uncovered lines are
|
||
|
||
Bucketed from Sonar's per-file `uncovered_lines` (exclusions from #26 already applied, so `*Screen.kt`
|
||
is absent):
|
||
|
||
| Bucket | Files | Lines to cover | Covered % | Verdict |
|
||
|---|--:|--:|--:|---|
|
||
| **ViewModels** | 28 | 1,153 | **0.1%** | **Test** — the dominant lever; no ViewModel has any test |
|
||
| **DTOs** | 12 | 648 | 26.7% | **Test** — trivial (serialization); a pattern already exists |
|
||
| **Repositories** | 11 | 274 | 11.3% | **Test** — fake the API interface |
|
||
| **core/\* (logic)** | 21 | 365 | 56.2% | **Test** — top up the partially-covered ones |
|
||
| UI composables (`*Components.kt`, `BlockRenderer`, …) | 7 | 255 | 3.9% | **Exclude** — not JVM-unit-testable, and `*Screen.kt`'s exclusion missed them |
|
||
| core/push (Android services) | 4 | ~191 | ~0% | **Exclude** (or Robolectric later) — foreground service / notifications |
|
||
| core/auth `Encrypted*` stores | 3 | 76 | 0% | **Exclude** — Android Keystore / EncryptedSharedPreferences |
|
||
| framework glue (`di/`, `RunicGatewayApp`, `LocalAssetResolver`) | 3 | ~17 | 0% | **Exclude** |
|
||
|
||
**Two levers, applied together:** (a) stop *counting* code a JVM unit test physically cannot execute,
|
||
and (b) actually *test* the logic — ViewModels, DTOs, repositories, core utilities.
|
||
|
||
## 2. Strategy & projected math
|
||
|
||
Numbers below are line-coverage projections against Sonar's `lines_to_cover`. They are estimates, but
|
||
grounded in the current per-bucket totals.
|
||
|
||
### Phase 0 — Broaden coverage exclusions (no tests; ~½ day)
|
||
|
||
Move non-unit-testable code out of the **coverage** denominator (it stays in *analysis* — bugs and
|
||
smells are still reported). Extend `sonar.coverage.exclusions` in `sonar-project.properties`:
|
||
|
||
```properties
|
||
sonar.coverage.exclusions=\
|
||
app/src/main/java/**/ui/**/*Screen.kt,\
|
||
app/src/main/java/**/ui/**/*Screen*.kt,\
|
||
app/src/main/java/**/ui/**/*Components.kt,\
|
||
app/src/main/java/**/ui/page/BlockRenderer.kt,\
|
||
app/src/main/java/**/ui/components/**,\
|
||
app/src/main/java/**/ui/shard/FrameFields.kt,\
|
||
app/src/main/java/**/ui/theme/**,\
|
||
app/src/main/java/**/ui/LocalAssetResolver.kt,\
|
||
app/src/main/java/**/RunicApp.kt,\
|
||
app/src/main/java/**/MainActivity.kt,\
|
||
app/src/main/java/**/*Application.kt,\
|
||
app/src/main/java/**/RunicGatewayApp.kt,\
|
||
app/src/main/java/**/di/**,\
|
||
app/src/main/java/**/core/push/PushService.kt,\
|
||
app/src/main/java/**/core/push/PushManager.kt,\
|
||
app/src/main/java/**/core/push/PushNotifier.kt,\
|
||
app/src/main/java/**/core/push/NtfyStreamClient.kt,\
|
||
app/src/main/java/**/core/auth/Encrypted*.kt
|
||
```
|
||
|
||
> Verify each glob targets composable-only / framework-only files before committing (e.g. confirm
|
||
> `FrameFields.kt` holds no testable logic). Keep the *pure-logic* push files in coverage
|
||
> (`PushPreferences`, `PushTickle`, `NtfyTopic`, `PushStreams`) — they already have tests.
|
||
|
||
Effect: denominator ~3,188 → ~2,650; covered ~542 → ~532. **Coverage ≈ 20%.** (Removing ~540 lines
|
||
that were ~2% covered.)
|
||
|
||
### Phase 1 — DTO serialization tests (highest ROI; ~1 day) → ~34%
|
||
|
||
DTOs are `@Serializable` data classes; test them with kotlinx-serialization round-trips against
|
||
representative backend JSON. The pattern already exists (`AccountDtoTest`, `AuthDtoTest`,
|
||
`NotificationsDtoTest`, `PlayerShardDtoTest`, `ShardDtoTest`). Add/extend:
|
||
|
||
- **New:** `AdminDto` (111 uncov — biggest single file), `WikiDto` (51), `PublicDto` (32),
|
||
`PageDto` (16), `PostDto` (12), `ContactDto` (11), `SsoDto`, `PageDto`.
|
||
- **Extend to ~85%:** `PlayerShardDto` (30.8%), `ShardDto` (35.6%), `AccountDto` (50.7%),
|
||
`AuthDto` (47.4%).
|
||
|
||
Target DTOs to ~85%: **+~380 covered lines** → covered ~912 / ~2,650 ≈ **34%**.
|
||
|
||
### Phase 2 — ViewModel tests (the big one; ~3–4 days) → clears the gate
|
||
|
||
28 ViewModels, ~1,153 lines, currently 0%. This is where the gate is won. Requires a small test
|
||
harness (§3). Each test drives the VM with fake collaborators and asserts `UiState` transitions
|
||
(loading → success/error, form validation, actions).
|
||
|
||
Priority by uncovered lines:
|
||
|
||
1. `LoginViewModel` (116), `AccountViewModel` (104), `CharactersViewModel` (77),
|
||
`AdminContentViewModel` (74), `NotificationsViewModel` (66), `TrustedDevicesViewModel` (63),
|
||
`ShardViewModel` (59)
|
||
2. `HousesViewModel` (51), `GovernorsViewModel` (47), `AdminDashboardViewModel` (43),
|
||
`AdminSupportViewModel` (41), `ChampsViewModel` (40), `AdminModerationViewModel` (40),
|
||
`GuildsViewModel` (39), `RecoveryCodesViewModel` (38), `ConnectViewModel` (37),
|
||
`ContactViewModel` (36), `VendorsViewModel` (32), `AppViewModel` (29)
|
||
3. The small ones (`PostViewModel`, `NewsViewModel`, `WikiViewModel`, `CharacterViewModel`,
|
||
`MyHousesViewModel`, `WikiPageViewModel`, `PageViewModel`, `HomeViewModel`, `SessionViewModel`)
|
||
|
||
Target ViewModels to ~70%: **+~800 covered lines** → covered ~1,712 / ~2,650 ≈ **65%. ✅ Gate passes.**
|
||
|
||
> Phases 0 + 2 alone (skipping DTOs) already reach ~50.5% — but DTOs are cheap insurance and Phase 1
|
||
> lands first because it de-risks the harness work.
|
||
|
||
### Phase 3 — Repository tests (~1–2 days) → margin
|
||
|
||
Repositories map API `Response`/exceptions to `ApiResult`; test with a fake `*Api` interface (or
|
||
OkHttp `MockWebServer`). Priority: `AuthRepository` (89), `ConnectionRepository` (43),
|
||
`ShardRepository` (27), `AdminRepository` (21), then the small content/wiki/player repos. Target ~70%:
|
||
**+~160 lines** → buffer well above 50% and resilience as the new-code window narrows.
|
||
|
||
### Phase 4 — core/net + core/auth top-up (~½–1 day) → durability
|
||
|
||
Fill the partially-covered utilities: `TokenAuthenticator` (31), `ShardStreamClient` (36),
|
||
`HostSelectionInterceptor` (9), `ApiResult` (6), `WebHandoff`, `WebsiteUrls`, `DeviceNameProvider`,
|
||
`ServerPreferences`, `AppConfig`.
|
||
|
||
### Trajectory
|
||
|
||
| After | Denominator | Covered | Coverage |
|
||
|---|--:|--:|--:|
|
||
| Today | 3,188 | 542 | 16.3% |
|
||
| Phase 0 (exclusions) | ~2,650 | ~532 | ~20% |
|
||
| Phase 1 (DTOs) | ~2,650 | ~912 | ~34% |
|
||
| **Phase 2 (ViewModels)** | ~2,650 | ~1,712 | **~65% ✅** |
|
||
| Phase 3 (repos) | ~2,650 | ~1,872 | ~71% |
|
||
| Phase 4 (core) | ~2,650 | ~2,000+ | ~75%+ |
|
||
|
||
## 3. Test infrastructure to add
|
||
|
||
The existing suite tests pure-logic classes only; ViewModel/coroutine testing needs a little scaffold.
|
||
`kotlinx-coroutines-test` is already a `testImplementation` dependency.
|
||
|
||
**`MainDispatcherRule`** (JUnit4) — swaps `Dispatchers.Main` (used by `viewModelScope`) for a test
|
||
dispatcher:
|
||
|
||
```kotlin
|
||
// app/src/test/java/com/runicgateway/app/util/MainDispatcherRule.kt
|
||
@OptIn(ExperimentalCoroutinesApi::class)
|
||
class MainDispatcherRule(
|
||
private val dispatcher: TestDispatcher = StandardTestDispatcher(),
|
||
) : TestWatcher() {
|
||
override fun starting(d: Description) = Dispatchers.setMain(dispatcher)
|
||
override fun finished(d: Description) = Dispatchers.resetMain()
|
||
}
|
||
```
|
||
|
||
**ViewModel test pattern** — hand-written fakes (matches the repo's existing no-mock convention; no new
|
||
dependency):
|
||
|
||
```kotlin
|
||
class LoginViewModelTest {
|
||
@get:Rule val mainDispatcher = MainDispatcherRule()
|
||
|
||
private class FakeAuthRepository(var result: LoginResult) : AuthRepository { /* stub the seam */ }
|
||
|
||
@Test fun `blank credentials surface INVALID_CREDENTIALS without a network call`() = runTest {
|
||
val vm = LoginViewModel(FakeAuthRepository(LoginResult.Success), /* … */)
|
||
vm.submit()
|
||
assertEquals(LoginError.INVALID_CREDENTIALS, vm.state.value.error)
|
||
}
|
||
}
|
||
```
|
||
|
||
- Assert on `viewModel.state.value` after `advanceUntilIdle()`; or collect the `StateFlow` in a
|
||
background `launch` when you need to see intermediate (loading) states.
|
||
- **Optional deps (decide once):** `mockk` would cut fake-writing for wide interfaces, and `turbine`
|
||
simplifies Flow assertions. Recommendation: **stay with hand fakes** to match convention; revisit
|
||
only if VM tests get boilerplate-heavy.
|
||
|
||
## 4. Execution notes
|
||
|
||
- CI already runs `./gradlew testDebugUnitTest jacocoTestReport` before the scan (`sonarqube.yml`),
|
||
so new tests count automatically on merge to `main`. Locally on this machine: JDK 21 needs
|
||
`-Pksp.incremental=false`.
|
||
- Sonar recomputes the gate on the post-merge scan; there's no way to fully confirm the number
|
||
pre-merge. Land phases as separate PRs (0, 1, 2, …) so coverage climbs visibly and reviews stay
|
||
small.
|
||
- `sonar.coverage.exclusions` removes files from **coverage only** — analysis still flags bugs/smells
|
||
in them, so excluding UI/framework code is safe.
|
||
- **Android-framework code deferred, not abandoned:** push services and `Encrypted*` stores are
|
||
excluded now; if we want them covered later, add Robolectric (`testImplementation`) and a
|
||
`RobolectricTestRunner` suite rather than instrumented tests, to keep it in the fast JVM `test`
|
||
source set the scan already consumes.
|
||
|
||
## 5. Definition of done
|
||
|
||
- `new_coverage ≥ 50%` and the SonarQube quality gate is **green**.
|
||
- `MainDispatcherRule` + a documented ViewModel test pattern exist and are reused.
|
||
- Coverage exclusions list only genuinely non-unit-testable files (UI composables, Android-framework
|
||
glue) — no ViewModel, repository, DTO, or pure core-logic file is excluded.
|
||
|
||
## 6. Amendment (2026-08-08, M12 phase 8): `ui/theme/**` narrowed to one file
|
||
|
||
**A directory glob in the exclusion list went stale as soon as another milestone put testable code
|
||
in that directory.** §2 phase 0 excluded `app/src/main/java/**/ui/theme/**` because at the time the
|
||
directory held `Color.kt`, `Type.kt` and the composables — constants and composable bodies, nothing
|
||
a JVM test could execute. M12 then added three *pure* resolvers to it: `ShardPalette`,
|
||
`ShardStructure` and `ShardTypeface`, which exist precisely so the theming milestone's no-op
|
||
invariants could be plain JVM assertions. JaCoCo measures them at **98%, 100% and 100%**, and the
|
||
glob was discarding every line.
|
||
|
||
The exclusion is now the single file it was really about, `ui/theme/Theme.kt` (the composable, 52%).
|
||
Everything else in `ui/theme/` is measured and all of it covers at 93% or better.
|
||
|
||
Two things worth carrying forward:
|
||
|
||
- **This did not rescue the gate and was not meant to.** M12's already-measured code
|
||
(`data/appearance/` at 100%, `ui/navigation/` at 93–100%) clears `new_coverage ≥ 50` on its own.
|
||
The point is that a future change deleting those resolvers' tests would now move the number, where
|
||
before it would not have — the exclusion was hiding well-tested code, which is the opposite of what
|
||
§1 built the list for and what §5's third bullet asks for.
|
||
- **Prefer file globs to directory globs when a directory is mixed.** `ui/components/**` stays a
|
||
directory glob and correctly so: `BrandAssets.kt` sits at 11% because only `brandAssetUrl` is pure,
|
||
and the rest is composable bodies. The distinction is whether the directory is *uniformly*
|
||
untestable, not whether it is under `ui/`.
|