Skip to content

Commit 2f47fb2

Browse files
trivikraduh95
trivikr
authored and
aduh95
committed
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 ee66a38 commit 2f47fb2

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
@@ -32,6 +32,7 @@ const { fileURLToPath, URL } = require('internal/url');
3232
const{ kMappings, SourceMap }=require('internal/source_map/source_map');
3333
const{
3434
codes: {
35+
ERR_OPERATION_FAILED,
3536
ERR_SOURCE_MAP_CORRUPT,
3637
ERR_SOURCE_MAP_MISSING_SOURCE,
3738
},
@@ -332,7 +333,7 @@ class TestCoverage {
332333
}
333334

334335
constcoverageFile=join(this.coverageDirectory,entry.name);
335-
constcoverage=JSONParse(readFileSync(coverageFile,'utf8'));
336+
constcoverage=readCoverageFile(coverageFile);
336337
this.mergeCoverage(result,this.mapCoverageWithSourceMap(coverage));
337338
}
338339

@@ -508,6 +509,24 @@ class TestCoverage {
508509
}
509510
}
510511

512+
functionreadCoverageFile(coverageFile){
513+
constrawCoverage=readFileSync(coverageFile,'utf8');
514+
515+
if(rawCoverage.length===0){
516+
thrownewERR_OPERATION_FAILED(`coverage file is empty: ${coverageFile}`);
517+
}
518+
519+
try{
520+
returnJSONParse(rawCoverage);
521+
}catch(err){
522+
consterror=newERR_OPERATION_FAILED(
523+
`failed to parse coverage file ${coverageFile}: ${err.message}`,
524+
);
525+
error.cause=err;
526+
throwerror;
527+
}
528+
}
529+
511530
functiontoPercentage(covered,total){
512531
returntotal===0 ? 100 : (covered/total)*100;
513532
}

‎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
returnreport;
7878
}
7979

80+
functionformatSpawnSyncResult(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+
functionassertIncludesReport(result,report){
90+
assert(
91+
result.stdout.toString().includes(report),
92+
formatSpawnSyncResult(result),
93+
);
94+
}
95+
8096
test('test coverage report',async(t)=>{
8197
awaitt.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
constoptions={env: { ...process.env,NODE_V8_COVERAGE: tmpdir.path}};
112128
constresult=spawnSync(process.execPath,args,options);
113129
constreport=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
constresult=spawnSync(process.execPath,args);
130146
constreport=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
constresult=spawnSync(process.execPath,args,options);
150166
constreport=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
constresult=spawnSync(process.execPath,args);
167183
constreport=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
constreport=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
conststdout=child.stdout.toString();
304+
assert.notStrictEqual(stdout,'',formatSpawnSyncResult(child));
288305
constcoverage=JSON.parse(stdout);
289306

290307
awaitt.test('does not include node_modules',()=>{
@@ -366,7 +383,7 @@ test('coverage with ESM hook - source irrelevant', skipIfNoInspector, () => {
366383
constresult=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
constresult=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
returnreport.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
returnreport.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
returnreport.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)