Skip to content

Commit 266e165

Browse files
trivikrtrivikr
authored
test_runner: improve coverage failure diagnostics
Include the coverage filename when parsing V8 coverage data fails, and make test-runner coverage assertions report spawned child process output when the expected coverage report is missing. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #64050 Refs: https://github.com/nodejs/reliability/issues?q=%22parallel%2Ftest-runner-coverage%22 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent ff58f43 commit 266e165

2 files changed

Lines changed: 51 additions & 15 deletions

File tree

‎lib/internal/test_runner/coverage.js‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ const { fileURLToPath, URL } = require('internal/url');
3333
const { kMappings, SourceMap } = require('internal/source_map/source_map');
3434
const {
3535
codes: {
36+
ERR_OPERATION_FAILED,
3637
ERR_SOURCE_MAP_CORRUPT,
3738
ERR_SOURCE_MAP_MISSING_SOURCE,
3839
},
@@ -402,7 +403,7 @@ class TestCoverage {
402403
}
403404

404405
const coverageFile = join(this.coverageDirectory, entry.name);
405-
const coverage = JSONParse(readFileSync(coverageFile, 'utf8'));
406+
const coverage = readCoverageFile(coverageFile);
406407
this.mergeCoverage(result, this.mapCoverageWithSourceMap(coverage));
407408
}
408409

@@ -587,6 +588,24 @@ class TestCoverage {
587588
}
588589
}
589590

591+
function readCoverageFile(coverageFile) {
592+
const rawCoverage = readFileSync(coverageFile, 'utf8');
593+
594+
if (rawCoverage.length === 0) {
595+
throw new ERR_OPERATION_FAILED(`coverage file is empty: ${coverageFile}`);
596+
}
597+
598+
try {
599+
return JSONParse(rawCoverage);
600+
} catch (err) {
601+
const error = new ERR_OPERATION_FAILED(
602+
`failed to parse coverage file ${coverageFile}: ${err.message}`,
603+
);
604+
error.cause = err;
605+
throw error;
606+
}
607+
}
608+
590609
function toPercentage(covered, total) {
591610
return total === 0 ? 100 : (covered / total) * 100;
592611
}

‎test/parallel/test-runner-coverage.js‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,22 @@ function getSpecCoverageFixtureReport() {
7777
return report;
7878
}
7979

80+
function formatSpawnSyncResult(result) {
81+
return [
82+
`status: ${result.status}`,
83+
`signal: ${result.signal}`,
84+
`stdout:\n${result.stdout}`,
85+
`stderr:\n${result.stderr}`,
86+
].join('\n');
87+
}
88+
89+
function assertIncludesReport(result, report) {
90+
assert(
91+
result.stdout.toString().includes(report),
92+
formatSpawnSyncResult(result),
93+
);
94+
}
95+
8096
test('test coverage report', async (t) => {
8197
await t.test('handles the inspector not being available', (t) => {
8298
if (process.features.inspector) {
@@ -111,7 +127,7 @@ test('test tap coverage reporter', skipIfNoInspector, async (t) => {
111127
const options = { env: { ...process.env, NODE_V8_COVERAGE: tmpdir.path } };
112128
const result = spawnSync(process.execPath, args, options);
113129
const report = getTapCoverageFixtureReport();
114-
assert(result.stdout.toString().includes(report));
130+
assertIncludesReport(result, report);
115131
assert.strictEqual(result.stderr.toString(), '');
116132
assert.strictEqual(result.status, 0);
117133
assert(findCoverageFileForPid(result.pid));
@@ -129,7 +145,7 @@ test('test tap coverage reporter', skipIfNoInspector, async (t) => {
129145
const result = spawnSync(process.execPath, args);
130146
const report = getTapCoverageFixtureReport();
131147

132-
assert(result.stdout.toString().includes(report));
148+
assertIncludesReport(result, report);
133149
assert.strictEqual(result.stderr.toString(), '');
134150
assert.strictEqual(result.status, 0);
135151
assert(!findCoverageFileForPid(result.pid));
@@ -149,7 +165,7 @@ test('test spec coverage reporter', skipIfNoInspector, async (t) => {
149165
const result = spawnSync(process.execPath, args, options);
150166
const report = getSpecCoverageFixtureReport();
151167

152-
assert(result.stdout.toString().includes(report));
168+
assertIncludesReport(result, report);
153169
assert.strictEqual(result.stderr.toString(), '');
154170
assert.strictEqual(result.status, 0);
155171
assert(findCoverageFileForPid(result.pid));
@@ -166,7 +182,7 @@ test('test spec coverage reporter', skipIfNoInspector, async (t) => {
166182
const result = spawnSync(process.execPath, args);
167183
const report = getSpecCoverageFixtureReport();
168184

169-
assert(result.stdout.toString().includes(report));
185+
assertIncludesReport(result, report);
170186
assert.strictEqual(result.stderr.toString(), '');
171187
assert.strictEqual(result.status, 0);
172188
assert(!findCoverageFileForPid(result.pid));
@@ -187,7 +203,7 @@ test('single process coverage is the same with --test', skipIfNoInspector, () =>
187203
const report = getTapCoverageFixtureReport();
188204

189205
assert.strictEqual(result.stderr.toString(), '');
190-
assert(result.stdout.toString().includes(report));
206+
assertIncludesReport(result, report);
191207
assert.strictEqual(result.status, 0);
192208
assert(!findCoverageFileForPid(result.pid));
193209
});
@@ -226,7 +242,7 @@ test('coverage is combined for multiple processes', skipIfNoInspector, () => {
226242
});
227243

228244
assert.strictEqual(result.stderr.toString(), '');
229-
assert(result.stdout.toString().includes(report));
245+
assertIncludesReport(result, report);
230246
assert.strictEqual(result.status, 0);
231247
});
232248

@@ -268,7 +284,7 @@ test.skip('coverage works with isolation=none', skipIfNoInspector, common.mustCa
268284
});
269285

270286
assert.strictEqual(result.stderr.toString(), '');
271-
assert(result.stdout.toString().includes(report));
287+
assertIncludesReport(result, report);
272288
assert.strictEqual(result.status, 0);
273289
}, 0));
274290

@@ -285,6 +301,7 @@ test('coverage reports on lines, functions, and branches', skipIfNoInspector, as
285301
]);
286302
assert.strictEqual(child.stderr.toString(), '');
287303
const stdout = child.stdout.toString();
304+
assert.notStrictEqual(stdout, '', formatSpawnSyncResult(child));
288305
const coverage = JSON.parse(stdout);
289306

290307
await t.test('does not include node_modules', () => {
@@ -366,7 +383,7 @@ test('coverage with ESM hook - source irrelevant', skipIfNoInspector, () => {
366383
const result = spawnSync(process.execPath, args, { cwd: fixture });
367384

368385
assert.strictEqual(result.stderr.toString(), '');
369-
assert(result.stdout.toString().includes(report));
386+
assertIncludesReport(result, report);
370387
assert.strictEqual(result.status, 0);
371388
});
372389

@@ -401,7 +418,7 @@ test('coverage with ESM hook - source transpiled', skipIfNoInspector, () => {
401418
const result = spawnSync(process.execPath, args, { cwd: fixture });
402419

403420
assert.strictEqual(result.stderr.toString(), '');
404-
assert(result.stdout.toString().includes(report));
421+
assertIncludesReport(result, report);
405422
assert.strictEqual(result.status, 0);
406423
});
407424

@@ -435,7 +452,7 @@ test('coverage with excluded files', skipIfNoInspector, () => {
435452
return report.replaceAll('/', '\\');
436453
}
437454

438-
assert(result.stdout.toString().includes(report));
455+
assertIncludesReport(result, report);
439456
assert.strictEqual(result.status, 0);
440457
assert(!findCoverageFileForPid(result.pid));
441458
});
@@ -472,7 +489,7 @@ test('coverage with included files', skipIfNoInspector, () => {
472489
return report.replaceAll('/', '\\');
473490
}
474491

475-
assert(result.stdout.toString().includes(report));
492+
assertIncludesReport(result, report);
476493
assert.strictEqual(result.status, 0);
477494
assert(!findCoverageFileForPid(result.pid));
478495
});
@@ -506,7 +523,7 @@ test('coverage with included and excluded files', skipIfNoInspector, () => {
506523
return report.replaceAll('/', '\\');
507524
}
508525

509-
assert(result.stdout.toString().includes(report));
526+
assertIncludesReport(result, report);
510527
assert.strictEqual(result.status, 0);
511528
assert(!findCoverageFileForPid(result.pid));
512529
});
@@ -547,7 +564,7 @@ test('correctly prints the coverage report of files contained in parent director
547564
});
548565

549566
assert.strictEqual(result.stderr.toString(), '');
550-
assert(result.stdout.toString().includes(report));
567+
assertIncludesReport(result, report);
551568
assert.strictEqual(result.status, 0);
552569
});
553570

@@ -564,5 +581,5 @@ test('coverage with directory and file named "file"', skipIfNoInspector, () => {
564581

565582
assert.strictEqual(result.stderr.toString(), '');
566583
assert.strictEqual(result.status, 0);
567-
assert(result.stdout.toString().includes('start of coverage report'));
584+
assertIncludesReport(result, 'start of coverage report');
568585
});

0 commit comments

Comments
 (0)