diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-09-26 10:04:46 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-09-26 10:04:46 +0000 |
| commit | 315fee32e64c75f641b7673092d1f3f42ea27e57 (patch) | |
| tree | b58f7171b9efe0681d66a0c1b189a7adb1265aa4 /CLAUDE.md | |
| parent | 12a0f7f07179f4d2b98a5db3c2a6bca33b57644c (diff) | |
Exclude framework adapters from coverage
Peter's call. The gate had been overridden eight times running, every one a
class whose whole body is a call into ExoPlayer, Media3, Room, WorkManager or
Chaquopy -- none of which execute on the JVM.
The last few overrides were the argument for doing this. Pulling completion
decisions out of PlaybackService into tested classes lowered the number,
because the tested part was already counted and the glue it left behind was
counted too. Writing an instrumented test for a migration lowered it again,
because jacocoTestReport measures unit tests only. A metric that punishes both
of those is measuring how much Android glue the app contains, not how well its
logic is tested.
Same code, 65.04% -> 89.93%: 629 uncovered lines were adapters, 131 are real.
What is left is honest -- page-fetch coroutines that need the actual host, a
few data classes, and Room's Converters, which are plain Kotlin and genuinely
untested. A drop from here will mean something.
The list sits next to its reasoning in the build file, with the bar written
down: "cannot execute off a device", not "awkward to test". QueueRepository,
PlaybackAdvancer, EpisodeDownloader and ChapterFetcher all touch
framework-adjacent things and all stay measured.
Also added jacocoFullReport, which folds in instrumented coverage for when you
want the fuller picture with a device attached. The commit gate deliberately
does not read it: device data only exists after a device run, so including it
would make the gate report different numbers for identical code depending on
whether one happened recently -- worse than a narrow gate.
One trap worth recording: Kotlin block comments nest. A KDoc mentioning a glob
with `/*` in it opened a nested comment, so the closing `*/` only closed the
inner one and the comment ran on until it hit a `*/` inside a later string
literal -- producing syntax errors on innocent lines well below the real cause.
No behaviour change. 425 unit tests green, detekt and lint clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
Diffstat (limited to 'CLAUDE.md')
| -rw-r--r-- | CLAUDE.md | 33 |
1 files changed, 23 insertions, 10 deletions
@@ -87,9 +87,11 @@ claims three separate times across these repos. 2. **Tests** — `./gradlew testDebugUnitTest` passes in full. 3. **Static analysis** — `./gradlew detekt lintDebug` clean: zero lint errors, zero new detekt findings beyond the baseline. -4. **Coverage must not decrease** against `.coverage-baseline` (line coverage over - `app/src/main/java` excluding `ui/**`). Set `COVERAGE_EXCEPTION="reason"` for justified drops, - e.g. deleting covered dead code. +4. **Coverage must not decrease** against `.coverage-baseline` — line coverage over + `app/src/main/java`, less Compose screens and the framework adapters listed in + `app/build.gradle.kts`. Set `COVERAGE_EXCEPTION="reason"` for justified drops, e.g. deleting + covered dead code. With the adapters excluded the number is about 90%, so a drop now means + something; before it hovered around 65% and mostly measured how much glue existed. 5. **Manifest permission diff** — if `<uses-permission>` changed, the commit message says why. 6. **Schema migrations** — if `AppDatabase.kt`'s `version` changed, a migration test must be touched in the same diff. Room tracks by version; a wrong migration corrupts a real, @@ -188,13 +190,24 @@ out of a possibly-partial MP3, and the JSON path covers the shows that mark chap ## Things a future session should know -- **The coverage gate has been overridden four times**, each recorded in the commit message. All - four are Android-framework glue that cannot execute off-device: the Chaquopy bridge, - PlaybackService, ChaquopyYouTubeSource, PlayerConnection and the two Workers. Each time the - gate fired it was a useful prompt — it drove PositionWriter onto a narrow Playhead interface - and pulled RetentionSweeper out of its Worker, both of which found or prevented real bugs. - If the exceptions keep accumulating, the honest fix is a jacoco exclusion for framework - adapters, which is a decision for Peter rather than something to slip in. +- **Framework adapters are excluded from coverage, as of 2026-09-26.** The gate had been + overridden eight times, every one a class whose whole body is a call into ExoPlayer, Media3, + Room, WorkManager or Chaquopy. The last few overrides were perverse: pulling decisions *out* + of a service into tested classes lowered the number, and writing an instrumented test for a + migration lowered it again, because `jacocoTestReport` measures unit tests only. A metric that + punishes both was measuring how much Android glue exists. + + Peter's call, taken 2026-09-26. The list lives in `app/build.gradle.kts` next to its reasoning, + and coverage went 65.04% → 89.93% on the same code. **Adding to that list needs a reason, not + just a name** — the bar is "cannot execute off a device", not "awkward to test". Earlier, the + gate firing was a useful prompt: it drove PositionWriter onto a narrow Playhead interface and + pulled RetentionSweeper out of its Worker, both of which found real bugs. That value only + survives if the list stays honest. + + `jacocoFullReport` adds instrumented coverage on top, for when you want the fuller picture + with a device attached. The commit gate deliberately does not read it: device data only exists + after a device run, so folding it in would make the gate report different numbers for + identical code. - **Detekt config has been relaxed five times**, all documented inline in `config/detekt/detekt.yml`. Twice the better answer was restructuring instead: `Screens.kt` was split per screen and |
