Repository navigation
test: add branch-coverage floor over scripts/*.mjs helpers - #254
Conversation
Every quality fix in this repository since early October has been the same change: a test asserting a helper branch that already ran under the suite but was never checked (#232, #241, #247). Nothing failed when a new branch was added to a helper without an assertion, so the class recurred. scripts/coverage-floor.test.mjs runs the rest of the suite as a child with NODE_V8_COVERAGE, merges the raw V8 block-coverage ranges for scripts/*.mjs (not *.test.mjs) across processes, and fails naming each range no process executed: file, line and the quoted source slice. A tool's main-module guard is excluded because its self-test runs it via spawnSync against a copy. Fixture self-tests cover the cross-process merge, the exclusions, the message shape and a real child run over a tmp helper with one dead branch. The ranges the gate found at HEAD are asserted in the same change: page-csp.mjs parseCsp without <head>, a contentless CSP meta (null content offset/quote), empty directive segments, script-src with no default-src, neither img-src nor default-src, <img> without src; and update-csp-hashes.mjs refusing a contentless policy meta. Closes #251 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Checked out 2085dba, ran the gate, and mutation-tested it. Looks correct to me.
node --test scripts/coverage-floor.test.mjspasses in ~5s locally (the live gate re-runs the 40-file suite as a child in 4.4s), so the cost added to the CItestjob (.github/workflows/links.yml:75,node --test scripts/*.test.mjs) is small.- Appending
export function zz(x) { return x ? 1 : 2; }toscripts/html-pages.mjsmakes the live gate fail with exactlyscripts/html-pages.mjs:32 \function zz(x) …` never executed by the suite`, so the detection path works against a real helper, not only the tmp fixture. cliEntrySpans(scripts/coverage-floor.test.mjs:34) matches the one real main-module guard in the tree (scripts/update-csp-hashes.mjs:64) exactly;html-pages.mjs,page-csp.mjsandactions-if.mjshave none, so nothing is over-excluded.collect()(:131-135) stripsNODE_TEST_CONTEXTbefore spawning — without that, the childnode --testwould try to report over IPC to the parent instead of running the files. SetsCOVERAGE_FLOOR_CHILDso the live test skips itself inside the child;suiteFiles()already excludesSELF, so there is no recursion either way.- The six new
page-csp.test.mjsfixtures and the contentless-meta case inupdate-csp-hashes.test.mjs:85-87cover the branches the gate would otherwise have flagged (parseCspnull-content path atupdate-csp-hashes.mjs:22-24), which is why the live gate is green on this head.
One note for the maintainer, not a defect: this is a zero-tolerance floor (any unexecuted V8 block range fails), so future helper edits must come with a test per branch. The README entry says this plainly.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read scripts/coverage-floor.test.mjs end to end plus the two test additions and the CI test job (.github/workflows/links.yml:75, node --test scripts/*.test.mjs). This looks correct to me; the live gate is already green on this head, so the nine newly asserted ranges do close the floor.
What I verified:
deadRanges/countOf(coverage-floor.test.mjs:52-82): a block with no range of its own inherits the innermost enclosing range's count, which is the right reading of V8 block coverage (ranges are only emitted when the count differs from the parent). The fixture at:160-168pins both the inherited-executed case and the all-runs-dead case. A nested zero range that is narrower in one run than another is still reported through its own key, so cross-run dedup can't hide it.collect(:122-137) dropsNODE_TEST_CONTEXTbefore spawning the childnode --test; without that the child would report over IPC to the parent runner instead of running the files. Good catch — that is the non-obvious part of this design.suiteFilesexcludesSELFand the live test skips underCOVERAGE_FLOOR_CHILD, so there is no recursion path even if the file list ever included it.page-csp.test.mjsandupdate-csp-hashes.test.mjsadditions assert behaviour (parseCspnull-content,;;segments, missingdefault-src,externalImageOriginscasing) rather than just executing branches.
One low note, not a blocker: cliEntrySpans (:35-45) matches braces by counting {/} characters without lexing, so a } inside a string or template literal within a tool's main-module guard would end the span early and the remainder of the guard would surface as a dead range. It fails closed and names the line, so it would be obvious when it happens; just worth knowing when editing update-csp-hashes.mjs's guard.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
The floor from #254 reads V8 coverage for scripts/*.mjs only; the hero, ACMM, projects and story-dialog scripts the pages ship run under their harnesses via new Function, which V8 reports with an empty URL, so a branch added to a page script without an assertion never failed it (#274 added three such branches in keyGoTo). scripts/inline-scripts.mjs now owns HARNESSED_SCRIPTS and an inlineScript() extractor that tags each block with a //# sourceURL=inline:<page>#<hook> trailer; the three harnesses extract through it, and coverage-floor.test.mjs reads those URLs, shifts the offsets past the new Function wrapper and reports page#hook at the page line, with fixture self-tests including a real child run over a tmp page. The 21 ranges the extended floor found are asserted: hero no-root, no-stage, no-controls-bar and unstyled measurement arms, positional 'Slide N' fallbacks, link-less slide refocus and the focus() options retry; projects no-root, empty padding-left inset and focusin outside a card; ACMM no-root, panel/level count skew and a tab-less level; story dialog missing dialog/form and a field without an error slot. Closes #275 Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Test Improvement
Adds
scripts/coverage-floor.test.mjs, the branch-coverage floor over thescripts/*.mjshelpers accepted in #251 (maintainer decision: in-tree test, no workflow edit, listed in README## Checks, nine zero-count ranges asserted in the same PR).The gate. Runs the rest of the suite once more as a child with
NODE_V8_COVERAGE, merges the raw V8 block-coverage ranges for everyscripts/*.mjshelper (*.test.mjsexcluded) across child processes, and fails naming each range no process executed — file, line, quoted source slice — so the fix is a test, not arithmetic. A range counts as executed if any process ran it (V8 emits a range only where a block's count differs from its parent, so the merge uses the innermost range enclosing the whole span per process). A tool's main-module guard (if (process.argv[1] … import.meta.url) {) is excluded:update-csp-hashes.test.mjsruns it viaspawnSyncagainst a tmp copy, which V8 attributes to the copy. Picked up by the existingnode --test scripts/*.test.mjsglob inlinks.yml;ci-wiring.test.mjsalready required the README entry.Fixture self-tests: cross-process merge (zero in one run, inherited as executed in another → not dead), whitespace-only and guard-block exclusions, message shape/ordering/truncation, and a real child run over a tmp
scripts/with one dead helper and one fully asserted helper → exactly one problem.The assertions (
page-csp.test.mjs,update-csp-hashes.test.mjs): page without<head>; CSP meta with nocontentattribute (empty directives,content: null,contentAt: -1,contentQuote: null, andupdateCspHashesthrows "fix the policy manually");;;/trailing;segments ignored;script-srcpresent butdefault-srcmissing → "default-src must be 'none'"; neitherimg-srcnordefault-srcwith an external<img>;<img>withoutsrc.Verified: with the new assertions stashed, the gate reports exactly the remaining zero-count ranges from #251 (line 45 turned out to already be executed by
update-csp-hashes.test.mjs:30's src-less<img onerror>, which the issue's per-range sum missed; it is now asserted directly anyway). Fullnode --test scripts/*.test.mjs: 738 pass; suite time 3 s → 7 s because the gate re-runs the suite once.Closes #251
Filed by quality agent (ACMM L4/L6 — full mode)
— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88