Skip to content

Commit 2ed6545

Browse files
committed
fix(ci): compare the coverage ratchet against the measured merge base
.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.
1 parent 6f4462c commit 2ed6545

1 file changed

Lines changed: 241 additions & 35 deletions

File tree

scripts/coverage-guard.php

Lines changed: 241 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,57 +3,263 @@
33
/**
44
* Coverage guard — prevents test coverage from dropping.
55
*
6-
* Usage: php scripts/coverage-guard.php <clover.xml> [--update-baseline]
6+
* Usage:
7+
* php scripts/coverage-guard.php <clover.xml> [--against=<clover.xml>]
8+
* php scripts/coverage-guard.php <clover.xml> [--update-baseline]
9+
* php scripts/coverage-guard.php --capabilities
10+
*
11+
* TWO FLOORS, AND ONLY ONE OF THEM IS AUTHORITATIVE
12+
*
13+
* `--against` names a clover report MEASURED at the merge base. When it is
14+
* given it is the ONLY floor: the committed `.coverage-baseline` is reported
15+
* for information and deliberately not enforced.
16+
*
17+
* That precedence is the whole fix. `.coverage-baseline` is a number a human
18+
* types, and against a base branch that moves there is no value a pull-request
19+
* author can commit that is guaranteed correct when it lands. Measured on
20+
* openregister: an author committed 58.93, `development` advanced from 16030
21+
* to 16038 tests while the PR sat, the merge result measured 58.88, and the
22+
* guard reported "dropped by 0.05%". Taking max(committed, measured) would
23+
* preserve exactly that failure, so the measured value does not merely win
24+
* ties — it replaces the committed one outright.
25+
*
26+
* A measured floor also disposes of a second trap: CI measures with xdebug and
27+
* local runs typically use pcov, and the two do not count statements
28+
* identically. With `--against`, both numbers come from the same driver in the
29+
* same job, so the difference cancels instead of being baked into a constant.
30+
*
31+
* Without `--against` the committed `.coverage-baseline` is used as a
32+
* conservative fail-safe floor. It is a floor and never an exact target: a
33+
* measurement ABOVE it is good news and exits 0. Demanding equality is what
34+
* made this gate unsatisfiable, because closing "the file is stale" required
35+
* committing the value the tree would measure after landing.
736
*
837
* Exit codes:
9-
* 0 — coverage is equal to or higher than baseline
10-
* 1 — coverage dropped (PR should be blocked)
11-
* 2 — missing files or invalid input
38+
* 0 — coverage is equal to or higher than the floor
39+
* 1 — coverage dropped (the change should be blocked)
40+
* 2 — missing, unparseable or empty input
41+
*/
42+
43+
const CG_OK = 0;
44+
const CG_DROPPED = 1;
45+
const CG_INPUT = 2;
46+
47+
/**
48+
* Capabilities this script understands.
49+
*
50+
* The workflow probes this before relying on `--against`. An older copy of this
51+
* script accepts the flag silently and ignores it, which would downgrade the
52+
* ratchet to the committed-constant check while still reporting success — a
53+
* check that did not run looking exactly like one that passed. The probe turns
54+
* that into a loud failure.
55+
*/
56+
const CG_CAPABILITIES = ['against', 'update-baseline', 'capabilities'];
57+
58+
/**
59+
* Parse `--key=value` / `--flag` into a map, and everything else in order.
60+
*
61+
* @param array<int,string> $argv
62+
*
63+
* @return array{0: array<string,string|bool>, 1: array<int,string>}
64+
*/
65+
function cgParseArgs(array $argv): array
66+
{
67+
$options = [];
68+
$positional = [];
69+
70+
foreach (array_slice($argv, 1) as $arg) {
71+
if (strncmp($arg, '--', 2) !== 0) {
72+
$positional[] = $arg;
73+
continue;
74+
}
75+
76+
$body = substr($arg, 2);
77+
$eq = strpos($body, '=');
78+
79+
if ($eq === false) {
80+
$options[$body] = true;
81+
continue;
82+
}
83+
84+
$options[substr($body, 0, $eq)] = substr($body, ($eq + 1));
85+
}
86+
87+
return [$options, $positional];
88+
}
89+
90+
/**
91+
* Read a clover report and return its statement counts and percentage.
92+
*
93+
* An empty or zero-statement report is a HARD ERROR, not 0% and not 100%.
94+
* Reading "no statements" as a number would let a run that produced nothing
95+
* satisfy the guard — and when the empty report is the merge-base side, it
96+
* would set the floor to 0 and pass every possible drop.
97+
*
98+
* @return array{0: int, 1: int, 2: float}
99+
*/
100+
function cgMeasure(string $file, string $label): array
101+
{
102+
if (file_exists($file) === false) {
103+
fwrite(STDERR, "Error: {$label} clover file not found: {$file}\n");
104+
exit(CG_INPUT);
105+
}
106+
107+
$xml = @simplexml_load_file($file);
108+
if ($xml === false || isset($xml->project->metrics) === false) {
109+
fwrite(STDERR, "Error: could not parse coverage metrics from {$label} report {$file}\n");
110+
exit(CG_INPUT);
111+
}
112+
113+
$metrics = $xml->project->metrics;
114+
$statements = (int) $metrics['statements'];
115+
$covered = (int) $metrics['coveredstatements'];
116+
117+
if ($statements <= 0) {
118+
fwrite(
119+
STDERR,
120+
"Error: {$label} report {$file} counts zero statements. Refusing to turn an empty "
121+
. "report into a percentage — this is a measurement failure, not a coverage result.\n"
122+
);
123+
exit(CG_INPUT);
124+
}
125+
126+
return [$statements, $covered, round((($covered / $statements) * 100), 2)];
127+
}
128+
129+
/**
130+
* Compare percentages as integer hundredths.
131+
*
132+
* Used only against the committed floor, which IS a two-decimal constant, so
133+
* two decimals is all the precision that comparison can carry.
12134
*/
135+
function cgHundredths(float $percentage): int
136+
{
137+
return (int) round(($percentage * 100));
138+
}
13139

14-
$baselineFile = __DIR__ . '/../.coverage-baseline';
15-
$cloverFile = $argv[1] ?? 'coverage/clover.xml';
16-
$updateBaseline = in_array('--update-baseline', $argv, true);
140+
/**
141+
* Is the head ratio strictly below the merge-base ratio?
142+
*
143+
* Compared as exact integer cross-products, NOT as rounded percentages, and
144+
* that distinction is load-bearing. Rounding to two decimals hides roughly one
145+
* statement: measured on openbuild, dropping 8229/13987 to 8228/13987 leaves
146+
* both sides reading 58.83%, so the guard printed two different statement
147+
* counts next to the word "unchanged" and exited 0. One statement is a small
148+
* hole, but it is a hole that does not close by itself — a regression that
149+
* gives back a statement at a time is invisible for as many pull requests as it
150+
* cares to take.
151+
*
152+
* The cost is that genuine measurement jitter now reds a build instead of being
153+
* swallowed. That is the intended direction: the counts are printed on failure,
154+
* so a flake is immediately legible as a flake, whereas a swallowed drop is
155+
* legible as nothing at all.
156+
*/
157+
function cgRatioDropped(int $covered, int $statements, int $baseCovered, int $baseStatements): bool
158+
{
159+
return (($covered * $baseStatements) < ($baseCovered * $statements));
160+
}
17161

18-
if (!file_exists($cloverFile)) {
19-
fwrite(STDERR, "Error: Clover file not found: $cloverFile\n");
20-
exit(2);
162+
/**
163+
* Print a percentage together with the counts it came from.
164+
*
165+
* The raw statement counts are printed on purpose. A bare percentage cannot
166+
* distinguish "tests were deleted" from "untested code was added", and the
167+
* second is the common way a ratchet is tripped by a change that added no
168+
* tests at all.
169+
*/
170+
function cgReport(string $label, int $statements, int $covered, float $percentage): void
171+
{
172+
printf("%-22s %6.2f%% (%d/%d statements)\n", $label, $percentage, $covered, $statements);
21173
}
22174

23-
if (!file_exists($baselineFile)) {
24-
fwrite(STDERR, "Error: Baseline file not found: $baselineFile\n");
25-
exit(2);
175+
// ── main ────────────────────────────────────────────────────────────────────
176+
177+
[$options, $positional] = cgParseArgs($argv);
178+
179+
if (isset($options['capabilities']) === true) {
180+
echo implode("\n", CG_CAPABILITIES), "\n";
181+
exit(CG_OK);
26182
}
27183

28-
$xml = simplexml_load_file($cloverFile);
29-
if ($xml === false) {
30-
fwrite(STDERR, "Error: Could not parse $cloverFile\n");
31-
exit(2);
184+
$cloverFile = ($positional[0] ?? 'coverage/clover.xml');
185+
$baselineFile = (__DIR__ . '/../.coverage-baseline');
186+
$against = ($options['against'] ?? null);
187+
188+
[$statements, $covered, $current] = cgMeasure($cloverFile, 'current');
189+
cgReport('Coverage current:', $statements, $covered, $current);
190+
191+
// ── the ratchet: a MEASURED floor ───────────────────────────────────────────
192+
if (is_string($against) === true && $against !== '') {
193+
[$baseStatements, $baseCovered, $base] = cgMeasure($against, 'merge-base');
194+
cgReport('Coverage merge base:', $baseStatements, $baseCovered, $base);
195+
196+
if (file_exists($baselineFile) === true) {
197+
$committed = trim(file_get_contents($baselineFile));
198+
echo "Committed .coverage-baseline: {$committed}% — recorded only, NOT enforced on this run.\n";
199+
echo "The merge base is measured, so it is the floor; see the header of this script.\n";
200+
}
201+
202+
if (cgRatioDropped($covered, $statements, $baseCovered, $baseStatements) === true) {
203+
$delta = round(($base - $current), 2);
204+
echo($delta > 0 ? "FAIL: coverage dropped by {$delta}% against the merge base.\n"
205+
: "FAIL: coverage dropped against the merge base by less than 0.01% — too little to show in "
206+
. "the percentage, but a real loss in the counts below.\n");
207+
echo " merge base {$baseCovered}/{$baseStatements} -> head {$covered}/{$statements} statements.\n";
208+
if ($statements > $baseStatements) {
209+
$added = ($statements - $baseStatements);
210+
echo " This change adds {$added} statements. Adding code without tests drops coverage.\n";
211+
}
212+
213+
exit(CG_DROPPED);
214+
}
215+
216+
if (cgRatioDropped($baseCovered, $baseStatements, $covered, $statements) === true) {
217+
$gain = round(($current - $base), 2);
218+
echo "OK: coverage improved by {$gain}% against the merge base "
219+
. "({$baseCovered}/{$baseStatements} -> {$covered}/{$statements}).\n";
220+
exit(CG_OK);
221+
}
222+
223+
echo "OK: coverage unchanged against the merge base.\n";
224+
exit(CG_OK);
32225
}
33226

34-
$metrics = $xml->project->metrics;
35-
$statements = (int)$metrics['statements'];
36-
$covered = (int)$metrics['coveredstatements'];
37-
$current = $statements > 0 ? round(($covered / $statements) * 100, 2) : 0.0;
227+
// ── fail-safe: the committed floor ──────────────────────────────────────────
228+
if (file_exists($baselineFile) === false) {
229+
fwrite(STDERR, "Error: baseline file not found: {$baselineFile}\n");
230+
exit(CG_INPUT);
231+
}
38232

39-
$baseline = (float)trim(file_get_contents($baselineFile));
233+
$raw = trim(file_get_contents($baselineFile));
234+
if (preg_match('/^[0-9]+(\.[0-9]+)?$/', $raw) !== 1) {
235+
fwrite(STDERR, "Error: unparseable coverage baseline value '{$raw}' — refusing to guess.\n");
236+
exit(CG_INPUT);
237+
}
40238

41-
echo "Coverage baseline: {$baseline}%\n";
42-
echo "Coverage current: {$current}%\n";
239+
$baseline = (float) $raw;
240+
printf("%-22s %6.2f%% (committed fail-safe floor)\n", 'Coverage baseline:', $baseline);
43241

44-
if ($current < $baseline) {
45-
echo "FAIL: Coverage dropped by " . round($baseline - $current, 2) . "%\n";
46-
exit(1);
242+
if (cgHundredths($current) < cgHundredths($baseline)) {
243+
$delta = round(($baseline - $current), 2);
244+
echo "FAIL: coverage dropped by {$delta}% below the committed floor.\n";
245+
exit(CG_DROPPED);
47246
}
48247

49-
if ($current > $baseline) {
50-
echo "Coverage improved by " . round($current - $baseline, 2) . "%\n";
51-
if ($updateBaseline) {
52-
file_put_contents($baselineFile, number_format($current, 2) . "\n");
53-
echo "Baseline updated to {$current}%\n";
248+
if (cgHundredths($current) > cgHundredths($baseline)) {
249+
$gain = round(($current - $baseline), 2);
250+
echo "OK: coverage is {$gain}% above the committed floor.\n";
251+
252+
// Kept for callers that want the recomputed value written out. Being ABOVE
253+
// the committed floor is explicitly not a failure: the floor is conservative
254+
// by design and drifts low as coverage rises. The live ratchet is the
255+
// merge-base comparison above, which cannot go stale at all.
256+
if (isset($options['update-baseline']) === true) {
257+
file_put_contents($baselineFile, (number_format($current, 2) . "\n"));
258+
echo "Recomputed floor written: {$current}%\n";
54259
}
55-
} else {
56-
echo "Coverage unchanged.\n";
260+
261+
exit(CG_OK);
57262
}
58263

59-
exit(0);
264+
echo "OK: coverage unchanged.\n";
265+
exit(CG_OK);

0 commit comments

Comments
 (0)