coverage-guard: the merge-base leg measures 54 fewer tests than the same commit on development, so PRs fail for statements they did not add

[rubenvdlinde]

What

The coverage ratchet's merge-base measurement runs a different test set than the
same commit does on development, so it under-reports the baseline and fails
PRs for statements they did not add.

Measured on openregister#2498, all three numbers for the same merge-base commit
(6ca5ee04c):

measurement	tests	statements
development's own CI run	16,449	85622 / 145144
merge base, measured inside the PR job	16,395	85460 / 144872
the PR head	16,452	85618 / 145148

The in-job base measurement is 54 tests and 272 statements short of what the
identical commit produces on development.

Why that matters

The guard then reports:

FAIL: coverage dropped against the merge base by less than 0.01%
      This change adds 276 statements. Adding code without tests drops coverage.

276 = 145148 − 144872. 272 of those are the base side under-measuring, not
code the PR added. Verified independently from the run's own clover.xml
artifact: the PR's new lib/Contract/ directory does not appear in the report
at all (an interface has no executable body, so it contributes zero
statements), and the three lib/ files the PR touches account for ~4 statements
between them — which matches 145148 − 145144 = 4 exactly.

So the sentence "This change adds 276 statements" is a conclusion the guard has
not verified, and it is wrong by two orders of magnitude here.

Where it comes from

The base leg is:

git checkout --force --detach "$MERGE_BASE"
./vendor/bin/phpunit -c phpunit.xml --coverage-clover="$RUNNER_TEMP/base-clover.xml" || true
test -s "$RUNNER_TEMP/base-clover.xml" || { echo "::error::Could not measure coverage at merge base…"; exit 1; }

The || true means a base run that partially fails still produces a clover file,
and test -s only checks that the file is non-empty. A base run that loaded
54 fewer test files passes that check and becomes the floor. Fewer tests loaded →
fewer files in the report → a lower statement total → every PR looks like it
"adds" the difference.

Suggested fix

Make the base leg refuse to be trusted when it did not run properly:

• capture the base run's exit code instead of || true, and fail loudly if it is
  non-zero — a base that errored is not a baseline;
• assert the base clover's file count and test count are within a small delta
  of the head run's, and fail with both numbers when they are not. Two runs of
  nearly the same tree should analyse nearly the same files; a 54-test gap is the
  tell.

The existing "refusing to report a ratchet that did not run" guard has the right
instinct — it just checks for an empty file rather than for a complete run.

Impact

Any PR whose base leg loses test files fails the ratchet for statements it did
not add. The failure is <0.01%, which makes it look like a rounding-level
regression rather than a broken measurement, so the natural response is to add
coverage that was never missing.

Found while landing ADR-084 (openregister#2498, #463).

🤖 Generated with Claude Code