This PR's own advisory leg caught it. With five markers and a truncated run it
printed
failed: 4
...
baseline: matches (5 expected, 5 failed)
three lines apart. The match line has always printed the baseline twice, which
was true while `failed` had to equal it to get there -- and the previous commit
removed that requirement for truncated runs without noticing this line depended
on it.
So the truncated spelling says what happened: `matches (5 expected; 4 of 5
failed, on a run the abort truncated -- not compared)`. Pinned by a third case
beside the two from that commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The verification dispatch of the reworked harness (34011072884) came back
4 expected / 3 received / 3 failed, where the one before it (34008889182) had
been 4/4/4 on the identical configuration. Nothing about the test list changed
between them: the abort landed one test earlier and the picker test never
started.
The baseline check would have called that "one now passes", which is the wrong
reading and the kind of notice #120 is about -- a deviation that is wrong often
enough to teach everyone to skim past deviation notices. So `failed` is compared
only when `completed cleanly` is yes, and `expected` is compared always, because
`Starting N tests` is printed before anything can abort and is what actually
answers "is the marked set the size the baseline says".
Two cases in e2e-report-shape-test.sh, as a pair: a truncated run short by one is
not a deviation, and a CLEAN run short by one still is -- so the first cannot have
bought its quiet by disabling the check.
This is a consequence of adding the fourth marker rather than a pre-existing bug
worth its own ticket: with three, the advisory leg had been completing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The advisory baseline check has announced a deviation on every PR since #113:
the tree "carries 4 tests marked @FailsOnEmulatorApi37" where it carries three
and FAILS_ON_EMULATOR_API37_BASELINE says three. The fourth is a KDoc in
Media3EngineTest saying the opposite -- "Deliberately not
`@FailsOnEmulatorApi37`: nothing here decodes or encodes" -- which the old
matcher counted because it looked for the string anywhere on any line.
Neither ingredient was wrong on its own, and the number is not the real damage.
#83 added this check so that a new failure joining the known ones could not be
invisible; a notice that is wrong every single time teaches everyone to skim
past deviation notices, which is precisely the signal it was built to create.
Editing the baseline to 4 would have silenced it by breaking it -- the check
would then have been wrong the moment someone added or removed a real marker.
Anchor the pattern at line start and require whitespace or end-of-line after the
name. The second half is the part that is easy to get wrong: "only the
annotation on a line of its own" also stops counting `@FailsOnEmulatorApi37
@Test`, which is legal Kotlin, and undercounting is the dangerous direction --
it hides a genuine new marker, the one thing this exists to catch. Measured
against a fixture carrying every shape at once: the old matcher 5, own-line-only
2, this one 3; on the real tree 4 / 3 / 3, so the baseline is untouched.
`grep -v import` goes too, since `^[[:space:]]*@` cannot match an import.
The check is a pure function of the working tree, so the fixture is committed
and e2e-report-shape-test.sh runs the real report against it -- inside a
throwaway repo root, which the script finds from BASH_SOURCE, so no knob had to
be added that could point the live count somewhere else. The fixture sits under
.github/, where Gradle does not compile it and :app's ktlint and detekt do not
see it; running the report against the real root with it committed still
reports 3.
Every other path through the report is byte-identical to the previous version on
both stdout and the job summary -- passing, failing, wedged, no-run, and
advisory-with-an-unreadable-baseline all diff empty -- and the two advisory legs
differ only by the false line disappearing. No job's status or pass/fail rules
change; the advisory leg stays continue-on-error and stays red by design.
The test is deliberately not wired into CI: adding a step to Static analysis
would add a new way for a gating job to go red, which #120 ruled out. shellcheck
still covers the file, since that step reads `git ls-files '*.sh'`.
Closes#120
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JMR-devandClaude Opus 5