Compare commits

..
Author SHA1 Message Date
JMR-dev 3d55004286 Count the Robolectric tests, which JaCoCo has never counted
The three #52 test PRs landed 56 new tests and the coverage figure moved 29.8% -> 29.7%.
That looked like the tests being worthless. It was the measurement.

Robolectric loads every class it touches through its own sandbox classloader, and those
classes arrive with no source location. JaCoCo skips no-location classes unless told
otherwise, and nothing here told it. So not one Robolectric test has ever contributed
coverage in this repo -- and Robolectric is what exercises the framework edge: both
workers, the publisher, both ViewModels, every Compose screen.

Same commit, same 335 tests, same 0 failures, only the block below added:

  LINE    652/2194  29.7%  ->  1519/2194  69.2%
  BRANCH  425/1424  29.8%  ->   758/1424  53.2%

  OutputPublisher       0.0% -> 97.5%      MainActivityKt     6.8% -> 86.4%
  ConversionViewModel   0.0% -> 85.4%      ConverterScreenKt  6.6% -> 62.8%

The discriminator, so this is not cargo cult: inside ConverterScreenKt, `describe` is the
one non-Composable and is exercised by a plain JVM test. It reported 8/8 covered while
every @Composable in the same class reported 0 -- including ones whose mutations
demonstrably failed the build when reverted. Across files the split is exactly
Robolectric-vs-not: StagingSweep, tested purely, 100%; OutputPublisher, ConversionViewModel
and FailureOutcome, tested under Robolectric, 0%.

`excludes = listOf("jdk.internal.*")` is not decoration. Without it JaCoCo walks
JDK-internal classes Robolectric has no location for either and the test JVM dies rather
than reporting a number.

CLAUDE.md's coverage bullet is rewritten, because it was wrong twice over. The figure was
an artifact, and the explanation attached to it -- that coverage fell as the suite grew
from 11 test files to 43 because the denominator outran the numerator on framework-edge
code "the JVM cannot reach" -- described a cause that does not exist. The JVM reaches that
code fine. The new tests were disproportionately Robolectric, so each one added denominator
and no numerator: the measurement was punishing precisely the tests that were hardest to
write, and the conclusion drawn from it was that writing them had not helped.

Mutation, run both ways on this branch: remove the block and jacocoTestReport collapses
back to 29.7% / 29.8%; restore it and it returns to 69.2% / 53.2%.

Two things that were true stay true. There is still no coverage gate, and a floor still
needs a settled baseline -- this one just moved 39 points in one build change. And
"re-measure before quoting it" was already written down; following it is the only reason
this was found.
2026-08-24 18:45:30 -05:00
Jason Ross e06b0826a0 Merge pull request #73 from JMR-dev/docs/instrumented-tests-correction
Say where instrumented tests run, instead of where they used to not run
2026-08-24 17:17:18 -05:00
JMR-dev 7b578c1ccf Merge branch 'main' into docs/instrumented-tests-correction 2026-08-24 17:10:27 -05:00
Jason Ross 9e7f80feaa Merge pull request #72 from JMR-dev/test/r38-2-filecard
Say in tests what the file card says when it does not know
2026-08-24 17:10:06 -05:00
JMR-dev 3d51fefeff Say where instrumented tests run, instead of where they used to not run
CLAUDE.md carried three claims about instrumented tests. All three were false, one of
them contradicted a paragraph forty lines below it in the same file, and a subagent
working on #58 hit the contradiction and had to stop and flag it rather than trust the
project's own instructions. That is the cost being paid here: this file is what every
contributor and every agent reads first.

  "Instrumented tests do not run locally"  -- they do, API 33-36, since 22c7914.
  "Emulators segfault on this host"        -- solved 2026-08-22; it was SwiftShader's
                                              Reactor JIT meeting SELinux execheap, not a
                                              broken machine, and another renderer avoids
                                              it. docs/local-emulator.md is titled
                                              "Emulators do run on this host".
  "CI's matrix therefore stops at API 36"  -- the matrix has been 33/34/35/36/37 since
                                              #56 merged, with a gating API 37 leg.

The contradiction was the worst of it. The testing-norm section added in #51 says "E2E is
runnable locally now", so the file simultaneously told you the emulator works and that it
segfaults on every AVD. A reader has no way to tell which half is current, and the wrong
half is the one that stops work: an agent that believes emulators are impossible here does
not try, and the local e2e half of the definition-of-done in #51 quietly stops being
enforceable.

The replacement says what is true now and names what is still true and why -- API 37 still
needs the manual Pixel check before a release, because the two advisory tests are the one
thing CI cannot answer for. It also states plainly that the advisory job is red on every
PR by design, which is the other thing agents keep rediscovering the hard way: three
separate subagents have now flagged that failure as possibly theirs.

The norm bullet now points at the section rather than re-arguing it, so there is one place
to correct next time rather than two that can drift apart again.

Same defect class as R14, R15, R20 and R25, all of which were documentation claims this
repo's own review falsified. The pattern is not that the docs were careless; it is that
they were written at a moment and the moment moved.
2026-08-24 16:26:47 -05:00
2 changed files with 67 additions and 17 deletions
+43 -17
View File
@@ -64,16 +64,28 @@ of the first one that fails.
on this machine (below), so without it an androidTest compile error is not discovered until CI.
ktlint and detekt also cover the `test`/`androidTest` source sets that `lintDebug` skips.
## Instrumented tests do not run locally
## Instrumented tests: where they actually run
Two independent reasons, so do not spend time on either:
This section said the opposite until 2026-08-24, and both of its claims had been false for two
days. Read it as the current answer, and see the git history if you need the old one.
- **Emulators segfault on this host.** qemu dies on every AVD. Instrumented tests run on CI or on
the physical Pixel, never in a local emulator.
- **The API 37 image is broken.** `android-37.0` crash-loops surfaceflinger inside its own gralloc
mapper, so every test fails there regardless of this app. `docs/api-37-emulator-crash.md` records
the evidence and the ruled-out fixes; CI's matrix therefore stops at API 36 even though targetSdk
is 37. **API 37 needs a manual check on the Pixel 10 Pro XL before each release.**
- **Local emulators work, for API 33-36.** `tools/local-emulator/run-e2e.sh` runs them on this
host. The segfault that made this look impossible was not a broken machine: SwiftShader's Reactor
JIT writes generated shader code onto the heap and executes it, Fedora's SELinux policy denies
`execheap`, and qemu dies. Choosing a different renderer avoids it entirely — `-gpu host`,
`angle_indirect` and `swangle_indirect` all boot, while `auto`, `off`, `guest` and
`swiftshader_indirect` do not. `docs/local-emulator.md` has the evidence and the per-API renderer
table.
- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. Two Media3 hardware-transcode
tests fail inside the emulator's own `c2.goldfish.h264.decoder` rather than on anything this app
does; they carry `@FailsOnEmulatorApi37` and run in a separate `continue-on-error` job,
`E2E API 37 Media3 hardware transcode (advisory)`. The gating leg runs the other 55.
**That advisory job is red on every PR, by design** — do not read it as your change breaking
something, and do not read a green run as evidence those two tests pass.
`docs/api-37-emulator-crash.md` has the measurements.
Still true, and the reason the advisory job is not simply deleted: **API 37 needs a manual check on
the Pixel 10 Pro XL before each release.** The advisory pair is the one thing CI cannot answer for.
On a device or emulator, build only the ABI it can execute:
@@ -96,12 +108,26 @@ install for code that can never run — and on API 37 the full APK does not fit
- The `model` package is excluded from `ReturnCount` and `CyclomaticComplexMethod` only. It is the
decision layer, where one branch is one documented user-visible outcome and the metric counts
answers rather than complexity. Every other rule still applies there.
- **Coverage is reported, not gated** — **29.8% of lines (629/2113), 28.7% of branches**, measured
on `main` 2026-08-23 with `./gradlew :app:jacocoTestReport`. A floor needs a baseline that has
settled first, and this one has not: the figure **fell** from the ~31% recorded earlier even
though the JVM suite went from 11 test files to 43. Main source grew 4,114 -> 5,715 lines over
the same period, so the denominator outran the numerator. Re-measure before quoting it; do not
assume more tests means a higher percentage here.
- **Coverage is reported, not gated** — **69.2% of lines (1519/2194), 53.2% of branches**,
measured 2026-08-24 with `./gradlew :app:jacocoTestReport`.
**Every figure this file carried before that date was an artifact, roughly half the real one.**
Robolectric loads classes through its own sandbox classloader with no source location, JaCoCo
skips no-location classes by default, and nothing told it otherwise — so **not one Robolectric
test counted**, and Robolectric is what exercises the framework edge here. The
`isIncludeNoLocationClasses` block in `app/build.gradle.kts` is what fixes it; **do not delete
it as stray config**, and re-run the numbers if you ever touch it. Same commit, same 335 tests:
29.7% -> 69.2% with that block alone.
The old entry also explained the wrong thing. It said coverage **fell** as the suite grew from 11
test files to 43 because "the denominator outran the numerator" on framework-edge code "the JVM
cannot reach". The JVM reaches that code fine. What actually happened is that the new tests were
disproportionately Robolectric, so each one added denominator and no numerator — the measurement
was punishing exactly the tests that were hardest to write.
Two things still hold. A floor needs a baseline that has settled, and this one has now moved by
39 points in a single build change, so it has not. And **re-measure before quoting** — that
instruction is the only reason this was caught.
- **Testable code is not done until it is tested.** If a piece is unit testable, it gets unit
tests before it counts as done. If it is e2e testable, it gets e2e tests. Both clauses apply —
a change that is both needs both.
@@ -112,9 +138,9 @@ install for code that can never run — and on API 37 the full APK does not fit
documents the reasoning — turns "needs a device" into "a pure function plus a thin edge".
Robolectric is in the JVM source set, `compose-ui-test-junit4` with it, so Compose screens are
unit testable too. Reach for the seam before concluding something cannot be unit tested.
- **E2E is runnable locally now.** `tools/local-emulator/run-e2e.sh` runs API 33-36 on this
machine; see `docs/local-emulator.md`. That was believed impossible until the SELinux/renderer
cause was found, and it is what makes the e2e half of this norm enforceable.
- **E2E is runnable locally**, API 33-36, via `tools/local-emulator/run-e2e.sh` — see
"Instrumented tests: where they actually run" above. That was believed impossible until the
SELinux/renderer cause was found, and it is what makes the e2e half of this norm enforceable.
- **A test has to bite.** Revert the line it covers, confirm it goes red, restore. A review of
this codebase ran 46 mutations against a 257-test suite and **9 were vacuous** — five of them
passing the whole suite over a completely unguarded code path. Green is not evidence.
+24
View File
@@ -188,6 +188,30 @@ detekt {
}
// Pin the coverage agent rather than inheriting whatever Gradle bundles.
// Robolectric loads every class it touches through its own sandbox classloader, and those
// classes arrive with no source location. JaCoCo skips no-location classes by default, so
// without this block **not one Robolectric test counts** -- and Robolectric is what exercises
// the framework edge here: the workers, the publisher, both ViewModels, every Compose screen.
//
// Measured on e06b082, same 335 tests, same 0 failures, only this block added:
//
// LINE 29.7% -> 69.2% OutputPublisher 0.0% -> 97.5%
// BRANCH 29.8% -> 53.2% ConversionViewModel 0.0% -> 85.4%
//
// The discriminator, if this ever looks like superstition: inside ConverterScreenKt, `describe`
// is the one non-Composable and is exercised by a plain JVM test -- it reported 8/8 covered while
// every @Composable in the same class reported 0, including ones whose mutations demonstrably
// failed the build when reverted.
//
// `excludes` is not optional. Without it JaCoCo walks JDK-internal classes that Robolectric has
// no location for either, and the test JVM dies rather than reporting a number.
tasks.withType<Test>().configureEach {
extensions.configure<JacocoTaskExtension> {
isIncludeNoLocationClasses = true
excludes = listOf("jdk.internal.*")
}
}
jacoco {
toolVersion = libs.versions.jacoco.get()
}