From 748e97df55c62aca4e1b2ab1fb4173c477d3dac2 Mon Sep 17 00:00:00 2001 From: Bob Killen Date: Sun, 11 Oct 2026 09:54:48 -0500 Subject: [PATCH] ci(e2e-coverage): gate region coverage per source file and ratchet the aggregate to 96 The e2e-coverage job gated browser coverage on two aggregates over all of src/**, so one region in twenty could be uncovered anywhere and nothing required that slack to be spread out: a single component could fall to nothing while the job stayed green. The unit lane already closed the same hole with --check-source-file-regions, and #1233 taught the e2e reporter the flag, but the workflow never passed it. Pass --check-source-file-regions 66 in the render step, and ratchet --check-source-regions from 95 to 96. Both numbers come from a full local coverage run at main (344 passed, 202 scripts captured, none uncaptured): 97.26% aggregate (639/657) and PeopleFreshness at 66.67% (2/3) as the lowest file, whose one zero is in the documented union residual (#1256), so 66 is bound by the data rather than chosen as a target. The aggregate stops at 96 because 97 would leave one region between floor and ceiling. Coupled edits: tests/e2e-coverage-gate.test.mjs records the new floors and measurement and gains lowered/deleted fixtures for the per-file flag; tests/workflow-scripts.test.mjs matches both literals; the CONTRIBUTING.md snippet carries the same flags CI enforces, as its prose promises. Closes #1232 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Bob Killen --- .github/workflows/ci.yml | 3 +- CONTRIBUTING.md | 3 +- tests/e2e-coverage-gate.test.mjs | 88 ++++++++++++++++++++++++-------- tests/workflow-scripts.test.mjs | 19 +++++-- 4 files changed, 84 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b5251a3..f0c161a3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -261,7 +261,8 @@ jobs: --input "$RUN_DIR" \ --build build \ --check-source 100 \ - --check-source-regions 95 \ + --check-source-regions 96 \ + --check-source-file-regions 66 \ --require-source-files \ --json coverage/e2e/report.json \ --text coverage/e2e/report.txt diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 809e32d2..f14cb57c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -260,7 +260,8 @@ node tests/tools/e2e-coverage-run.mjs seal \ --dir "$RUN_DIR" --status passed npm run report:e2e:coverage -- \ --input "$RUN_DIR" --build build \ - --check-source 100 --check-source-regions 95 \ + --check-source 100 --check-source-regions 96 \ + --check-source-file-regions 66 \ --require-source-files \ --json "coverage/e2e/$RUN_ID-report.json" \ --text "coverage/e2e/$RUN_ID-report.txt" diff --git a/tests/e2e-coverage-gate.test.mjs b/tests/e2e-coverage-gate.test.mjs index 8fc4a3bb..882c463a 100644 --- a/tests/e2e-coverage-gate.test.mjs +++ b/tests/e2e-coverage-gate.test.mjs @@ -32,28 +32,39 @@ const REPORT_SCRIPT = 'report:e2e:coverage'; const COVERAGE_JOB = 'e2e-coverage'; // Measured on the suite this commit ships: the e2e-coverage job renders with -// `--check-source 100 --check-source-regions 95` and the run clears both; the -// full suite measured 100.00% source lines and 81.43% source regions when the -// region gate landed (#1026, #1031), 91.63% source regions when the floor was -// ratcheted to 91 (#1105), and 95.22% (438/460) when it was ratcheted to 95 -// after #1163 folded the crossing-region source-map drift and #1060 covered -// the member-directory toolbar arms (#1079). +// `--check-source 100 --check-source-regions 96 --check-source-file-regions 66` +// and the run clears all three; the full suite measured 100.00% source lines +// and 81.43% source regions when the region gate landed (#1026, #1031), +// 91.63% source regions when the floor was ratcheted to 91 (#1105), 95.22% +// (438/460) when it was ratcheted to 95 after #1163 folded the +// crossing-region source-map drift and #1060 covered the member-directory +// toolbar arms (#1079), and 97.26% (639/657) when it was ratcheted to 96 and +// the per-file floor was added (#1232). // -// Ceiling, as of main run 38082557106: 97.29% (646/664), 18 regions at zero. -// Thirteen are defensive fallbacks of the #1253 class. The other five -- -// ReferenceArchitectures:13 and :34, PeopleFreshness:22, GroupLinkStatus:33 -// (x2) -- are guard arms that a sibling fixture build (data-no-revision, -// data-variant) demonstrably executes, yet the union never credits (#1256, -// #1236): the build that takes the arm records no deviation at the arm's own -// span, and isContradictedRegion only admits witnesses from the script that -// produced the zero. That restriction is deliberate (#1066: a different -// bundle maps the same skipped arm to a different span), so the five are a -// documented reporting residual, not a coverage gap. Do not set -// --check-source-regions above 97 or a per-file floor above 66 -// (PeopleFreshness, 2/3) without first changing the union. +// Ceiling, as of that 97.26% run: 18 regions at zero. Thirteen are defensive +// fallbacks of the #1253 class. The other five -- ReferenceArchitectures:13 +// and :34, PeopleFreshness:22, GroupLinkStatus:33 (x2) -- are guard arms that +// a sibling fixture build (data-no-revision, data-variant) demonstrably +// executes, yet the union never credits (#1256, #1236): the build that takes +// the arm records no deviation at the arm's own span, and +// isContradictedRegion only admits witnesses from the script that produced +// the zero. That restriction is deliberate (#1066: a different bundle maps +// the same skipped arm to a different span), so the five are a documented +// reporting residual, not a coverage gap. Do not set --check-source-regions +// above 97 or --check-source-file-regions above 66 (PeopleFreshness, 2/3, +// whose one zero is in that residual) without first changing the union. The +// aggregate stops at 96 rather than 97 because 97 would leave one region +// between the floor and the ceiling. +// +// The per-file floor is the unit lane's --check-source-file-regions argument +// one level down: the aggregate lets roughly one region in twenty-five go +// uncovered anywhere in src/**, and nothing else stops a single file from +// holding all of that slack. At 66 it is bound by the lowest file, so it is +// a floor against a file falling, not a target. const FLOORS = { '--check-source': 100, - '--check-source-regions': 95, + '--check-source-regions': 96, + '--check-source-file-regions': 66, }; // Present-or-absent guarantees, which carry no percentage. --require-source-files @@ -203,10 +214,10 @@ test('a job that never invokes the reporter fails the guard', () => { // fixture text it edits is its own line, so it gets its own staleness guard // and its own proof the floor can go red. function weakenedRegions(replacement) { - const original = ' --check-source-regions 95 \\\n'; + const original = ' --check-source-regions 96 \\\n'; assert.ok( workflowText.includes(original), - 'fixture is stale: ci.yml no longer renders with --check-source-regions 95', + 'fixture is stale: ci.yml no longer renders with --check-source-regions 96', ); return workflowText.replace(original, replacement); } @@ -219,7 +230,7 @@ test('a lowered --check-source-regions fails the guard', () => { weakenedRegions(' --check-source-regions 50 \\\n'), ), ), - /--check-source-regions is 50, below the 95/, + /--check-source-regions is 50, below the 96/, ); }); @@ -229,3 +240,36 @@ test('a deleted --check-source-regions fails the guard', () => { /must pass --check-source-regions/, ); }); + +// The per-file floor is the one a reviewer is most likely to wave through a +// drop in, because it reads as low already; the fixture proves 66 is a floor +// the guard holds, not a default it tolerates moving. +function weakenedFileRegions(replacement) { + const original = ' --check-source-file-regions 66 \\\n'; + assert.ok( + workflowText.includes(original), + 'fixture is stale: ci.yml no longer renders with --check-source-file-regions 66', + ); + return workflowText.replace(original, replacement); +} + +test('a lowered --check-source-file-regions fails the guard', () => { + assert.throws( + () => + check( + reporterFlags( + weakenedFileRegions( + ' --check-source-file-regions 50 \\\n', + ), + ), + ), + /--check-source-file-regions is 50, below the 66/, + ); +}); + +test('a deleted --check-source-file-regions fails the guard', () => { + assert.throws( + () => check(reporterFlags(weakenedFileRegions(''))), + /must pass --check-source-file-regions/, + ); +}); diff --git a/tests/workflow-scripts.test.mjs b/tests/workflow-scripts.test.mjs index 25acdbe6..a4690b7d 100644 --- a/tests/workflow-scripts.test.mjs +++ b/tests/workflow-scripts.test.mjs @@ -349,13 +349,22 @@ test('browser coverage is isolated in a visible gating job with a source floor', // the line gate cannot give. The gate landed at 80, just under the 81.43% // a full run of the suite measured at the time, was ratcheted to 91 // under the 91.63% measured after the 2026-10-07 coverage merges (#1105), - // and to 95 under the 95.22% measured after #1163 folded the - // crossing-region source-map drift (#1079) -- the same ratchet pattern as - // the unit lane's region floors. + // to 95 under the 95.22% measured after #1163 folded the crossing-region + // source-map drift (#1079), and to 96 under the 97.26% measured when the + // per-file floor landed (#1232) -- the same ratchet pattern as the unit + // lane's region floors. assert.match( coverageCommands, - /--check-source-regions\s+95\b/, - 'the e2e coverage report must gate on --check-source-regions 95', + /--check-source-regions\s+96\b/, + 'the e2e coverage report must gate on --check-source-regions 96', + ); + // #1232: the aggregate lets its slack concentrate in one file. The per-file + // floor fails on that file's name instead of averaging it away; 66 is bound + // by the lowest file (PeopleFreshness, 2/3), not chosen as a target. + assert.match( + coverageCommands, + /--check-source-file-regions\s+66\b/, + 'the e2e coverage report must gate on --check-source-file-regions 66', ); // #992: the ratio is computed over observed files only, so a src module the // bundle drops vanishes from numerator and denominator alike. The file-set