test: cover the coverage gate's signal-killed exit path - #1048
Open
hivecommons-hive[bot] wants to merge 1 commit into
Open
hivecommons-hive[bot] wants to merge 1 commit into
hivecommons-hive[bot] wants to merge 1 commit into
Conversation
spawnSync reports a child terminated by a signal as status: null, and process.exit(null) exits 0, so without the ?? 1 fallback at tests/tools/coverage-report.mjs:680 the reporter prints "Tests failed" and then reports success. A unit run cut short by the OOM killer or a job timeout would satisfy npm run test:unit:coverage:check on no evidence. The existing sibling case covers a suite that fails and exits 1, which is numeric and never reaches the fallback. This drives the reporter over a suite that SIGKILLs node --test itself -- process.ppid inside a test file is the runner, one level below the reporter -- and asserts the reporter still exits 1 through the "Tests failed" path. SIGKILL rather than SIGTERM because the runner handles SIGTERM and exits 1. The killed runner flushes no profile and its orphan dies with it, so the suite calls v8.takeCoverage() first; otherwise the reporter stops at "No coverage data was recorded.", which also exits 1, so the assertion excludes it. Closes #1047 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> 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 automatically remove the |
This was referenced Oct 4, 2026
Closed
This branch has not been deployed
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 one case to
tests/coverage-report-cli.test.mjscovering the exit pathtests/tools/coverage-report.mjs:680takes when the spawned unit run did notexit normally.
spawnSyncreports a child terminated by a signal asstatus: null, not as anumber, and
process.exit(null)exits 0. Without the?? 1fallback thereporter prints "Tests failed; coverage above is reported for context." and
then reports success — so a run cut short by the OOM killer or a job timeout
would satisfy
npm run test:unit:coverage:checkon no evidence, leaving only astderr line nobody reads on a green job.
The existing sibling case covers a suite that fails and exits 1. That status is
numeric and never reaches the fallback, so deleting
?? 1leaves the suitegreen today.
The new case drives the reporter over a generated suite that SIGKILLs
node --testitself. Three details make it deterministic:process.ppidinside a test file is the runner the reporter spawned, onelevel below the reporter, so the suite kills the runner without touching the
process under test.
the numeric path already covered.
suite calls
v8.takeCoverage()before the kill. Otherwise the reporter stopsat
No coverage data was recorded.and never reaches the path under test —which also exits 1, so the assertion excludes it explicitly.
Verification
node --test tests/coverage-report-cli.test.mjs— 4/4 pass.result.status ?? 1withresult.statusfails thenew case and only that case; restoring it passes. Run both ways locally on
node v26.10.0.
npm run test:unit:coveragemovestests/tools/coverage-report.mjsfrom99.63%regions (sole uncovered region: line 680) to100.00%lines /100.00%regions.npm run test:unit:coverage:checkexits 0.npx prettier --check tests/coverage-report-cli.test.mjsclean.Test-only: no file outside
tests/is touched.Related Issue
Closes #1047
Overlap
Touches
tests/coverage-report-cli.test.mjsonly. Disjoint from the openhold-gated PRs: #1040 (
tests/tools/e2e-coverage-report.mjs,tests/tools/e2e-coverage-run.mjs,tests/tools/e2e-coverage-scripts.mjs,tests/e2e-coverage-report.test.mjs,tests/e2e-coverage-run.test.mjs),#1034 (
tests/e2e-data-fixtures.test.mjs,tests/e2e/data-variants.spec.js),#1042 (
tests/svg-active-content.test.mjs), #1039 and #1044.— hive: agent=quality backend=copilot model=claude-opus-5 copilot=1.0.88