FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: improve coverage failure diagnostics · nodejs/node@2f47fb2 · GitHub

/ node Public

Commit 2f47fb2

Browse files
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
const coverageFile = join(this.coverageDirectory, entry.name);
335-
const coverage = JSONParse(readFileSync(coverageFile, 'utf8'));
336+
const coverage = readCoverageFile(coverageFile);
336337
this.mergeCoverage(result, this.mapCoverageWithSourceMap(coverage));
337338
}
338339

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

512+
function readCoverageFile(coverageFile) {
513+
const rawCoverage = readFileSync(coverageFile, 'utf8');
514+
515+
if (rawCoverage.length === 0) {
516+
throw new ERR_OPERATION_FAILED(`coverage file is empty: ${coverageFile}`);
517+
}
518+
519+
try {
520+
return JSONParse(rawCoverage);
521+
} catch (err) {
522+
const error = new ERR_OPERATION_FAILED(
523+
`failed to parse coverage file ${coverageFile}: ${err.message}`,
524+
);
525+
error.cause = err;
526+
throw error;
527+
}
528+
}
529+
511530
function toPercentage(covered, total) {
512531
return total === 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
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)
Back | FazBrowse Home | New Git URL