fix(ci): compare the coverage ratchet against the measured merge base - #231
Merged
Merged
Conversation
.coverage-baseline was read as a floor by the phpunit guard and as an exact target by the push-side staleness check. Together they demand equality with a checked-in constant, which against a moving base branch is not satisfiable: closing "stale" means committing the value the tree will measure after the PR lands. Measured on openregister — committed 58.93, development advanced 16030->16038 tests, merge result measured 58.88, guard reported a 0.05% drop. coverage-guard.php gains --against=<clover.xml>, naming a report measured at the merge base. When present it is the only floor; the committed constant is reported but not enforced. Both numbers then come from one driver in one job, so the xdebug/pcov statement-counting difference cancels rather than being baked in, and the merge base cannot go stale. Ratios are compared as exact integer cross-products, not rounded percentages: at two decimals a one-statement regression read as "unchanged" and exited 0. An empty or zero-statement report is now a hard error rather than 0%, which as the merge-base side would set the floor to zero and pass every drop. Verified on real CI clover artifacts: a genuine 1.44% drop fails, an unchanged tree passes, and adding untested code fails while adding tested code passes.
Contributor
Quality Report — ConductionNL/nldesign @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ✅ | ✅ 2/2 | |||
| PHPUnit | ✅ | ||||
| Newman | ❌ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-06 06:32 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The coverage ratchet asks for a number nobody can compute
.coverage-baselineis read two ways at once:Together those demand exact equality with a checked-in constant. The only way to restore that equality is to commit, in a pull request, the number the tree will measure after that pull request lands — while the base branch keeps moving and your own diff changes the answer.
Measured on openregister: an author committed
58.93;developmentadvanced 16030 → 16038 tests while the PR sat; the merge result measured58.88; the guard reported "dropped by 0.05%". While the follow-up pinned58.88, development moved two further commits. There is no value a PR author can commit that is guaranteed correct when it lands.It is not one repo. On 2026-08-06, six of the sixteen apps were red on this job, every one of them for coverage being higher than recorded:
Three of those reds are one or two hundredths of a percent — a single extra covered statement.
A third trap sits underneath: CI measures with xdebug, local runs typically use pcov, and the two do not count statements identically. A baseline committed from a local measurement can be wrong on arrival.
The fix: measure the floor, don't type it
scripts/coverage-guard.phpgains--against=<clover.xml>, naming a coverage report measured at the merge base. When it is given it is the only floor — the committed.coverage-baselineis printed for information and deliberately not enforced.That precedence is the point. Taking
max(committed, measured)would preserve the openregister failure exactly, so the measured value does not merely win ties, it replaces the constant outright. Because both numbers then come from the same driver in the same job, the xdebug/pcov difference cancels instead of being baked into a constant. And the merge base is immutable for a given head, so the comparison cannot go stale: a rebase re-measures both sides together.The companion change in
ConductionNL/.githubmeasures the merge base in the phpunit job and retires the push-side staleness job. That change lands after this one, and it probes--capabilitiesand fails loudly rather than silently falling back — an older copy of this script accepts--againstand ignores it, which would demote the ratchet to the constant check while still reporting success.Without
--against(pushes tomain/development) the committed.coverage-baselineis still enforced, but strictly as a floor: below it fails, above it is fine. Demanding equality is what made the gate unsatisfiable.It still catches a real drop — proof
Verified against two real CI clover artifacts from this fleet (openbuild runs
31016006546→ 8018/13971 = 57.39% and31050116167→ 8229/13987 = 58.83%), not synthetic input:Two of those cases are holes found in the first draft of this change and closed before it shipped:
And the load-bearing comparison — the old guard passes that same real 1.44% drop:
openbuild's floor had decayed so far below actual coverage that a regression all the way back to 57.39% was invisible to it. The new comparison fails it. This change makes the ratchet stricter, not laxer.
What is deliberately not changed
.coverage-baselinevalues are left alone. Every repo's committed floor is currently at or below its measured coverage, so none is wrong in the dangerous direction, and pinning each to today's exact measurement would put the push-side check back on a knife edge — the brittleness this change exists to remove. A floor below actual coverage is conservative, never wrong, and is never the binding constraint on a PR. It may be raised at any time; the Coverage Baseline Protection job still refuses to let it be lowered.No waiver, no
continue-on-error, no widened threshold.