Repository navigation
test(e2e): add a per-file region floor to the e2e coverage reporter - #1233
Merged
Merged
Conversation
--check-source-regions is an aggregate over the whole of src/** together, so the slack it allows can be spent entirely on one file. At the 95% the e2e-coverage job asks for, a single component may sit at nothing and the job stays green. The companion line gate does not catch it either: a lost region need not be a lost line, because an unexecuted ternary arm or ?? fallback sits on a line the surrounding statement still covers. tests/tools/coverage-report.mjs already made this argument for the unit run and closed the hole with --check-source-file-regions, which package.json passes as 97. This gives the browser run the same flag. The flag is opt-in and defaults to off, so no existing caller changes behaviour; wiring it into .github/workflows/ci.yml is the remaining half of issue #1232. The fixture is the proof rather than an illustration: two bundles, one src file each, chosen so the run clears a 66% aggregate while one file sits at 50%. Each file gets its own bundle and map deliberately -- v8-to-istanbul resolves a single map naming two sources down to one source, so a merged fixture would silently measure one file and prove nothing. No vacuous-pass guard is included, unlike the unit reporter's equivalent: collectE2ECoverage already refuses a run that attributed nothing to src/**, so the offender list cannot be empty for the reason that guard exists to catch. Adding one would only create a permanently unreachable region. Refs #1232 (needs-human: the .github/workflows/ci.yml half of the fix cannot be pushed by this agent's token tier, which lacks the Workflows permission) Signed-off-by: quality <quality@hive.kubestellar.io>
Contributor
Author
|
Important Held for human review by the hive's ACMM level gate. This PR was opened by the "quality" agent while Hive policy required a human checkpoint for that agent. Non-outreach agents are held at ACMM L3–L5; the Hive will keep the |
Member
|
Heads-up: PRs1211 1240 also edit |
2 tasks
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.
Test Improvement
Adds
--check-source-file-regions <pct>totests/tools/e2e-coverage-report.mjs,applying the region floor to each
src/**file on its own instead of only tothe aggregate.
--check-source-regionsis an aggregate over all ofsrc/**together, so theslack it allows can be spent entirely on one file. At the 95% the
e2e-coveragejob asks for, a single component may sit at nothing and the job stays green. The
companion line gate does not catch that either — a lost region need not be a
lost line, because an unexecuted ternary arm or
??fallback sits on a line thesurrounding statement still covers.
tests/tools/coverage-report.mjs:45-58already made exactly this argument for theunit run and closed the hole with
--check-source-file-regions, whichpackage.jsonpasses as97. This is the same guarantee for the browser run.The flag is opt-in and defaults to off, so no existing caller changes
behaviour and CI is unaffected by this PR alone.
Files and functions claimed
tests/tools/e2e-coverage-report.mjs—parseArgs(one new option) and thethreshold block at the end of
main. Disjoint from test(e2e): cover the istanbul readers in e2e-coverage-report.mjs #1211, which touches thisfile's header comment and the export beside
getRegionCoverage.tests/e2e-coverage-report-file-region-floor.test.mjs— new file.Nothing else in the repository is touched;
CONTRIBUTING.mdis deliberately leftalone, because the sentence under its snippet states that the flags shown are the
ones CI enforces, and this PR does not change what CI enforces.
The fixture is the proof, not an illustration
Two bundles, one
srcfile each: one fully covered, one at 50% regions. The runreports 66.67% aggregate regions, clearing
--check-source-regions 66, whilesrc/components/Uneven/index.jsalone sits at 50% and fails--check-source-file-regions 66. That is the hole, reproduced.Each file gets its own bundle and its own map deliberately:
v8-to-istanbulresolves a single map naming two sources down to one source, so a merged fixture
would silently measure one file and prove nothing. That was observed, not assumed.
Five tests: the aggregate-vs-per-file demonstration above, the artifact-ordering
contract (a failing gate must still leave the report that explains it), an
exactly-meets-the-floor pass, the default-off case, and rejection of
non-percentage thresholds.
No vacuous-pass guard is included, unlike the unit reporter's equivalent:
collectE2ECoveragealready refuses a run that attributed nothing tosrc/**("No src/** coverage was attributable"), so the offender list cannot be empty for
the reason that guard exists to catch. Adding one would only create a permanently
unreachable region — the residual #1210 exists to stop accumulating.
Verification
node --test tests/e2e-coverage-report-file-region-floor.test.mjs— 5/5 pass.npm run checkandnpm run test:unit— 2091/2091 pass, no regressions.npm run test:unit:coverage: the new code is fully covered.tests/tools/e2e-coverage-report.mjsgoes 93.67% → 93.74% regions with theuncovered region set unchanged, so it stays above the
93per-file harnessfloor proposed in test(coverage): give the tests/tools/ harness a floor of its own #1207.
npx prettier --checkclean on both files.Related Issue
Refs #1232 (needs-human: the other half of the fix — passing the new flag in the
e2e-coveragerender step of.github/workflows/ci.yml, plus the matchingFLOORSentry intests/e2e-coverage-gate.test.mjsand theCONTRIBUTING.mdsnippet — touches
.github/workflows/**, which this agent's token tier cannotpush; #1232 carries the exact replacement text). The two halves are independent:
this one is inert until the workflow passes the flag, so they need not land
together.
Filed by quality agent (hold-gated mode). Human review required.
— hive: agent=quality backend=copilot model=claude-opus-5 copilot=1.0.88