Count the annotation, not the comment saying a test does not carry it

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>
This commit is contained in:
2026-08-25 23:30:55 -05:00
co-authored by Claude Opus 5
parent 959bd8be13
commit 0f41bc3f6b
3 changed files with 274 additions and 4 deletions
+27 -4
View File
@@ -208,11 +208,34 @@ if [ -n "$BASELINE_FILE" ]; then
advisory="yes"
[ -f "$BASELINE_FILE" ] \
&& baseline="$(sed -nE 's/^const val FAILS_ON_EMULATOR_API37_BASELINE = ([0-9]+).*/\1/p' "$BASELINE_FILE" | head -1)"
# The #81 check, verbatim: what the tree actually carries. Reported next to the baseline so a
# stale baseline shows up here rather than only once the emulator disagrees with it.
# What the tree actually carries. Reported next to the baseline so a stale baseline shows up
# here rather than only once the emulator disagrees with it.
#
# ANCHORED AT LINE START, AND WHITESPACE-OR-END-OF-LINE AFTER THE NAME (#120). The #81 check
# this replaces matched the string anywhere on any line, so #113's KDoc reading `Deliberately
# not @FailsOnEmulatorApi37` counted as a fourth marked test and the report announced a
# deviation on every PR. That is worse than a wrong number: #83 built this so a new failure
# could not be invisible, and a notice that is wrong every time teaches everyone to skim past
# deviation notices.
#
# THE OBVIOUS REPAIR IS A TRAP, and the reason for the second half of the pattern.
# `^[[:space:]]*@NAME[[:space:]]*$` -- "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, which is the one thing this exists to
# catch. Measured against `testdata/marker-shapes`, a fixture carrying every shape at once:
# the old matcher says 5, own-line-only says 2, this one says 3. On the real tree, 4 / 3 / 3.
# e2e-report-shape-test.sh runs that fixture through this whole script.
#
# `^[[:space:]]*@` cannot match an `import` line, so the old `grep -v import` goes with it
# rather than staying to imply a filter is still doing work.
#
# This is a regex over source text and not a parser. An annotation inside a multi-line string,
# or inside a `/* */` block that opened mid-line, would still be counted. Neither exists here;
# if one ever does, this check wants a different tool rather than a longer regex.
if [ -d "$REPO_ROOT/app/src/androidTest" ]; then
marked="$(grep -rn "@FailsOnEmulatorApi37" "$REPO_ROOT/app/src/androidTest" --include='*.kt' \
| grep -v import | grep -c FailsOn || true)"
marked="$(grep -rhcE '^[[:space:]]*@FailsOnEmulatorApi37([[:space:]]|$)' \
"$REPO_ROOT/app/src/androidTest" --include='*.kt' \
| awk '{ total += $1 } END { print total + 0 }' || true)"
fi
fi