Skip to content

Commit 0242b3c

Browse files
committed
fix(coverage-guard): scope the ratchet to the files a change touches
Adopts the canonical script from ConductionNL/.github (quality-config/coverage-guard.php). The whole-project comparison fires on measurement noise. doriath#240 was a PR whose entire diff was `webpack.config.js` — no PHP at all — and the guard failed it: identical denominator (13723), both runs reporting exactly `Tests: 948, Assertions: 3051, Skipped: 1`, and six covered statements of run-to-run xdebug variance between them. The measured `--against` floor cancels driver variance (xdebug vs pcov), as its header says. It does not cancel run-to-run variance within one driver, and the ratchet has no tolerance. Scoping the comparison to the PHP a change actually touches keeps full strength where a regression matters and makes the noise unreachable by construction — a diff with no PHP cannot fail. New `changed-files` capability; the shared workflow PROBES for it rather than assuming, so an un-updated copy keeps the previous behaviour instead of silently accepting and ignoring the flag. Script only — no behaviour change until the workflow passes `--changed-files`. Byte-identical to the canonical copy (md5 5be122aad209da030c79b22a133232fb).
1 parent b8dd0a7 commit 0242b3c

1 file changed

Lines changed: 170 additions & 1 deletion

File tree

scripts/coverage-guard.php

Lines changed: 170 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,121 @@
5353
* check that did not run looking exactly like one that passed. The probe turns
5454
* that into a loud failure.
5555
*/
56-
const CG_CAPABILITIES = ['against', 'update-baseline', 'capabilities'];
56+
const CG_CAPABILITIES = ['against', 'update-baseline', 'capabilities', 'changed-files'];
57+
58+
/**
59+
* Sum clover metrics for a named subset of files.
60+
*
61+
* WHY THIS EXISTS. Comparing the project-wide aggregate made the ratchet fire on
62+
* measurement noise. Measured on doriath#240 — a pull request whose entire diff
63+
* was `webpack.config.js`, containing no PHP at all:
64+
*
65+
* Coverage current: 60.50% (8303/13723 statements)
66+
* Coverage merge base: 60.55% (8309/13723 statements)
67+
* FAIL: coverage dropped by 0.05% against the merge base.
68+
*
69+
* The denominator is identical, so both runs compiled the same source, and both
70+
* reported exactly `Tests: 948, Assertions: 3051, Skipped: 1`. Only which six
71+
* statements xdebug recorded as covered moved. The header above argues that a
72+
* measured `--against` floor cancels driver variance, and it does — but it does
73+
* not cancel RUN-TO-RUN variance, and the ratchet has no tolerance.
74+
*
75+
* A tolerance band was the obvious alternative and is the wrong one: it blinds
76+
* the gate to small real regressions permanently, at a threshold nobody can
77+
* justify. Scoping to the files the change actually touched keeps full strength
78+
* where it matters and makes the noise unreachable — a diff with no PHP cannot
79+
* fail, by construction, rather than by being forgiven.
80+
*
81+
* Matching is by path SUFFIX. Clover records absolute paths from the machine
82+
* that produced it, and the two reports here are produced from two different
83+
* checkouts, so absolute equality would match nothing and silently measure zero.
84+
*
85+
* @param string $file Clover report to read.
86+
* @param string $label Human label for error messages.
87+
* @param array<int,string> $only Repo-relative paths to include.
88+
*
89+
* @return array{0:int,1:int,2:float} statements, covered, percentage
90+
*/
91+
function cgMeasureFiles(string $file, string $label, array $only): array
92+
{
93+
if (file_exists($file) === false) {
94+
fwrite(STDERR, "Error: {$label} clover file not found: {$file}\n");
95+
exit(CG_INPUT);
96+
}
97+
98+
$xml = @simplexml_load_file($file);
99+
if ($xml === false) {
100+
fwrite(STDERR, "Error: could not parse {$label} report {$file}\n");
101+
exit(CG_INPUT);
102+
}
103+
104+
$statements = 0;
105+
$covered = 0;
106+
$matched = 0;
107+
108+
foreach ($xml->xpath('//file') as $entry) {
109+
$name = (string) $entry['name'];
110+
if ($name === '') {
111+
continue;
112+
}
113+
114+
foreach ($only as $wanted) {
115+
if (str_ends_with($name, $wanted) === false) {
116+
continue;
117+
}
118+
119+
$matched++;
120+
$statements += (int) $entry->metrics['statements'];
121+
$covered += (int) $entry->metrics['coveredstatements'];
122+
break;
123+
}
124+
}
125+
126+
// Zero matches is NOT zero coverage. It means the changed files are absent
127+
// from this report — typically because they are new on the head side and do
128+
// not exist at the merge base, which is normal and must not read as a drop.
129+
if ($matched === 0) {
130+
return [0, 0, 0.0];
131+
}
132+
133+
$percentage = 0.0;
134+
if ($statements > 0) {
135+
$percentage = round((($covered / $statements) * 100), 2);
136+
}
137+
138+
return [$statements, $covered, $percentage];
139+
}
140+
141+
/**
142+
* Read the changed-file list, keeping only PHP files the guard can measure.
143+
*
144+
* @param string $path File containing one repo-relative path per line.
145+
*
146+
* @return array<int,string>
147+
*/
148+
function cgReadChangedFiles(string $path): array
149+
{
150+
if (file_exists($path) === false) {
151+
fwrite(STDERR, "Error: changed-files list not found: {$path}\n");
152+
exit(CG_INPUT);
153+
}
154+
155+
$lines = file($path, (FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES));
156+
if ($lines === false) {
157+
fwrite(STDERR, "Error: could not read changed-files list: {$path}\n");
158+
exit(CG_INPUT);
159+
}
160+
161+
$php = [];
162+
foreach ($lines as $line) {
163+
$line = trim($line);
164+
if ($line !== '' && str_ends_with($line, '.php') === true) {
165+
$php[] = $line;
166+
}
167+
}
168+
169+
return array_values(array_unique($php));
170+
}
57171

58172
/**
59173
* Parse `--key=value` / `--flag` into a map, and everything else in order.
@@ -184,6 +298,61 @@ function cgReport(string $label, int $statements, int $covered, float $percentag
184298
$cloverFile = ($positional[0] ?? 'coverage/clover.xml');
185299
$baselineFile = (__DIR__ . '/../.coverage-baseline');
186300
$against = ($options['against'] ?? null);
301+
$changedList = ($options['changed-files'] ?? null);
302+
303+
// ── scoped mode: compare ONLY the PHP the change touched ────────────────────
304+
//
305+
// Requires --against. Without a merge-base report there is nothing to compare a
306+
// file subset to, and falling back to the committed whole-project constant here
307+
// would compare a subset against a whole and fail everything.
308+
if (is_string($changedList) === true && $changedList !== '') {
309+
if (is_string($against) === false || $against === '') {
310+
fwrite(STDERR, "Error: --changed-files requires --against; a file subset has no meaning against the committed whole-project baseline.\n");
311+
exit(CG_INPUT);
312+
}
313+
314+
$changed = cgReadChangedFiles($changedList);
315+
316+
if (empty($changed) === true) {
317+
echo "OK: this change touches no PHP files, so there is no coverage to compare.\n";
318+
echo " (Whole-project drift between two runs of identical code is measurement noise, not a regression.)\n";
319+
exit(CG_OK);
320+
}
321+
322+
echo 'Scoped to ' . count($changed) . " changed PHP file(s).\n";
323+
324+
[$statements, $covered, $current] = cgMeasureFiles($cloverFile, 'current', $changed);
325+
[$baseStatements, $baseCovered, $base] = cgMeasureFiles($against, 'merge-base', $changed);
326+
327+
if ($statements === 0 && $baseStatements === 0) {
328+
echo "OK: none of the changed PHP files appear in either coverage report.\n";
329+
echo " Nothing was measured, so nothing is claimed about them.\n";
330+
exit(CG_OK);
331+
}
332+
333+
cgReport('Changed files, head:', $statements, $covered, $current);
334+
cgReport('Changed files, base:', $baseStatements, $baseCovered, $base);
335+
336+
if ($baseStatements === 0) {
337+
echo "OK: the changed PHP is new at the merge base, so there is no prior figure to drop below.\n";
338+
exit(CG_OK);
339+
}
340+
341+
if (cgRatioDropped($covered, $statements, $baseCovered, $baseStatements) === true) {
342+
$delta = round(($base - $current), 2);
343+
echo "FAIL: coverage of the files this change touches dropped by {$delta}%.\n";
344+
echo " base {$baseCovered}/{$baseStatements} -> head {$covered}/{$statements} statements.\n";
345+
if ($statements > $baseStatements) {
346+
$added = ($statements - $baseStatements);
347+
echo " This change adds {$added} statements to those files. Adding code without tests drops coverage.\n";
348+
}
349+
350+
exit(CG_DROPPED);
351+
}
352+
353+
echo "OK: coverage of the changed files did not drop.\n";
354+
exit(CG_OK);
355+
}//end if
187356

188357
[$statements, $covered, $current] = cgMeasure($cloverFile, 'current');
189358
cgReport('Coverage current:', $statements, $covered, $current);

0 commit comments

Comments
 (0)