golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#4355): add --merge-async to test:coverage:unit:raw (#4356)

Browse Source 
* test(#4355): assert test:coverage:unit:raw carries --merge-async

tests/c8-merge-async-flag.test.cjs asserted the opposite based on a
disproven assumption that `--reporter none` skips c8's merge phase --
it does not (Report.run() computes the merge unconditionally before
consulting the reporter list). This is the failing-first assertion for
the fix in the next commit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(#4355): add --merge-async to test:coverage:unit:raw

c8's Report.run() computes the full coverage merge unconditionally,
even with --reporter none -- it only skips the final text/json output,
not the merge dispatch (verified against node_modules/c8/lib/report.js).
Without --merge-async this used the synchronous _getMergedProcessCov(),
loading every raw per-process V8 coverage dump into memory at once and
OOM-crashing release.yml's finalize-test job (run 33997057100) with a
silent exit 1 and no diagnostic ~21s after the test suite itself
finished cleanly ("# fail 0"). Same class already fixed on the other
three coverage scripts via #4068/#4172.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(changeset): add changeset for #4355 coverage:unit:raw fix

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(changeset): backfill PR number for #4355 fix

pr:0 -> pr:4356

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-09-05 20:28:51 -04:00
committed by GitHub
parent a65cb291e8
commit 48789fe9a9
3 changed files with 36 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/zesty-ravens-snooze.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 4356
---
**Fixed a silent CI failure in the raw-coverage test shards.** `test:coverage:unit:raw` (used by test.yml's sharded lane and release.yml's rc/finalize jobs) could OOM-crash after the test suite itself passed cleanly, showing no error beyond a bare non-zero exit code.

2
package.json
View File

@@ -151,7 +151,7 @@
"test:coverage": "c8 --check-coverage --lines 70 --branches 60 --reporter text --include 'gsd-core/bin/lib/**/*.cjs' --exclude 'tests/**' --all node scripts/run-tests.cjs",
"test:coverage:scripts-floor": "c8 report --check-coverage --lines 55 --merge-async --include 'scripts/**/*.cjs' --exclude 'tests/**' --all",
"test:coverage:unit": "c8 --reporter text --reporter json-summary --merge-async --include 'gsd-core/bin/lib/**/*.cjs' --exclude 'tests/**' --all node scripts/run-tests.cjs --suite unit && node scripts/check-coverage-gate.cjs",
"test:coverage:unit:raw": "c8 --reporter none node scripts/run-tests.cjs --suite unit",
"test:coverage:unit:raw": "c8 --reporter none --merge-async node scripts/run-tests.cjs --suite unit",
"test:coverage:report": "c8 report --reporter text --reporter json-summary --merge-async --include 'gsd-core/bin/lib/**/*.cjs' --exclude 'tests/**' --all && node scripts/check-coverage-gate.cjs",
"test:coverage:all": "npm run test:coverage",
"test:mutation": "stryker run",

41
tests/c8-merge-async-flag.test.cjs
View File

@@ -19,6 +19,22 @@
// #4068's own new test file plus other suite growth pushed the un-flagged sync merge
// over the 8192 MB heap ceiling (#4172).
//
// #4355: #4068's fix ALSO missed test:coverage:unit:raw, based on an incorrect
// assumption (see the test this replaces, below) that `--reporter none` makes c8
// skip the merge/report dispatch entirely. Verified false against
// node_modules/c8/lib/report.js: Report.prototype.run() unconditionally awaits
// `this.getCoverageMapFromAllCoverageFiles()` to build `context.coverageMap` BEFORE
// it ever looks at `this.reporter` -- the reporter list only controls what happens
// to that already-merged map in the loop AFTER, so `--reporter none` skips nothing
// but the final text/json output. Without --merge-async this still runs the
// synchronous `_getMergedProcessCov()`, which loads every raw per-process V8
// coverage dump into memory at once -- the same #4068/#4172 OOM shape, confirmed
// live on release.yml's finalize-test job (run 33997057100, shard 2/3: "# fail 0"
// then a silent exit 1 ~21s later with no stack trace -- an external OOM-kill of
// the wrapping c8 process). test:coverage:unit:raw is used by both test.yml's
// sharded full-test lane and (since #4335) release.yml's rc-test/finalize-test
// matrix jobs, so every shard of every CI run and release was exposed.
//
// IMPORTANT c8@11.0.0 gotcha (verified against node_modules/c8/lib/commands/
// check-coverage.js and report.js directly): the `check-coverage` CLI subcommand's
// handler does NOT forward `argv.mergeAsync` into the `Report(...)` constructor --
@@ -102,7 +118,7 @@ describe('coverage-merge-async-flag (#4068, #4172)', () => {
});
test('--merge-async lands in the c8 invocation, not after the node runner', () => {
for (const key of ['test:coverage:unit', 'test:coverage:report', 'test:coverage:scripts-floor']) {
for (const key of ['test:coverage:unit', 'test:coverage:report', 'test:coverage:scripts-floor', 'test:coverage:unit:raw']) {
const script = scripts[key];
const flagIndex = script.indexOf('--merge-async');
const nodeIndex = script.indexOf(' node ');
@@ -131,19 +147,22 @@ describe('coverage-merge-async-flag (#4068, #4172)', () => {
);
});
test('test:coverage:unit:raw is intentionally unchanged (--reporter none skips the merge/report phase entirely)', () => {
test('test:coverage:unit:raw carries --merge-async (#4355)', () => {
assert.match(
scripts['test:coverage:unit:raw'],
/(?:^|\s)c8\s.*--merge-async/,
'test:coverage:unit:raw must pass --merge-async to c8 -- `--reporter none` does ' +
'NOT skip the merge phase (Report.run() computes it unconditionally before ' +
'ever consulting the reporter list, verified against ' +
'node_modules/c8/lib/report.js), so without this flag it still OOMs the same ' +
'way #4068/#4172 already fixed on the other three coverage scripts (#4355)'
);
assert.match(
scripts['test:coverage:unit:raw'],
/--reporter none/,
'test:coverage:unit:raw must keep --reporter none -- this is what defers all ' +
'merging to the separate coverage-gate job (test:coverage:report)'
);
assert.doesNotMatch(
scripts['test:coverage:unit:raw'],
/--merge-async/,
'--merge-async on a --reporter none run is a no-op (Report.run() never ' +
'reaches the merge dispatch) and would misleadingly imply this script does ' +
'its own merging'
'test:coverage:unit:raw must keep --reporter none -- the merge still needs to ' +
'happen for c8 to write nothing misleading, but the actual report output is ' +
'deferred to the separate coverage-gate job (test:coverage:report)'
);
});
});