diff --git a/.changeset/zesty-ravens-snooze.md b/.changeset/zesty-ravens-snooze.md new file mode 100644 index 000000000..3062c4341 --- /dev/null +++ b/.changeset/zesty-ravens-snooze.md @@ -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. diff --git a/package.json b/package.json index 6d0f5db0c..f999f7c20 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/tests/c8-merge-async-flag.test.cjs b/tests/c8-merge-async-flag.test.cjs index a950664ca..8a5e46eb9 100644 --- a/tests/c8-merge-async-flag.test.cjs +++ b/tests/c8-merge-async-flag.test.cjs @@ -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)' ); }); });