Skip to content

Commit 6ee5e42

Browse files
cjihrigtargos
authored andcommittedMay 3, 2023
test_runner: support combining coverage reports
This commit adds support for combining code coverage reports in the test runner. This allows coverage to be collected for child processes, and by extension, the test runner CLI. PR-URL: #47686 Fixes: #47669 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent b954ea9 commit 6ee5e42

File tree

9 files changed

+293
-55
lines changed

9 files changed

+293
-55
lines changed
 

‎doc/api/cli.md

+4
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,10 @@ Use this flag to enable [ShadowRealm][] support.
624624
added:
625625
- v19.7.0
626626
- v18.15.0
627+
changes:
628+
- version: REPLACEME
629+
pr-url: https://github.com/nodejs/node/pull/47686
630+
description: This option can be used with `--test`.
627631
-->
628632

629633
When used in conjunction with the `node:test` module, a code coverage report is

‎doc/api/test.md

-4
Original file line numberDiff line numberDiff line change
@@ -428,10 +428,6 @@ if (anAlwaysFalseCondition) {
428428
The test runner's code coverage functionality has the following limitations,
429429
which will be addressed in a future Node.js release:
430430

431-
* Although coverage data is collected for child processes, this information is
432-
not included in the coverage report. Because the command line test runner uses
433-
child processes to execute test files, it cannot be used with
434-
`--experimental-test-coverage`.
435431
* Source maps are not supported.
436432
* Excluding specific files or directories from the coverage report is not
437433
supported.

‎lib/internal/test_runner/coverage.js

+142-14
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
'use strict';
22
const {
3+
ArrayFrom,
34
ArrayPrototypeMap,
45
ArrayPrototypePush,
56
JSONParse,
67
MathFloor,
78
NumberParseInt,
8-
RegExp,
99
RegExpPrototypeExec,
1010
RegExpPrototypeSymbolSplit,
11+
SafeMap,
12+
SafeSet,
1113
StringPrototypeIncludes,
1214
StringPrototypeLocaleCompare,
1315
StringPrototypeStartsWith,
@@ -23,9 +25,7 @@ const { setupCoverageHooks } = require('internal/util');
2325
const { tmpdir } = require('os');
2426
const { join, resolve } = require('path');
2527
const { fileURLToPath } = require('url');
26-
const kCoveragePattern =
27-
`^coverage\\-${process.pid}\\-(\\d{13})\\-(\\d+)\\.json$`;
28-
const kCoverageFileRegex = new RegExp(kCoveragePattern);
28+
const kCoverageFileRegex = /^coverage-(\d+)-(\d{13})-(\d+)\.json$/;
2929
const kIgnoreRegex = /\/\* node:coverage ignore next (?<count>\d+ )?\*\//;
3030
const kLineEndingRegex = /\r?\n$/u;
3131
const kLineSplitRegex = /(?<=\r?\n)/u;
@@ -95,13 +95,6 @@ class TestCoverage {
9595
for (let i = 0; i < coverage.length; ++i) {
9696
const { functions, url } = coverage[i];
9797

98-
if (StringPrototypeStartsWith(url, 'node:') ||
99-
StringPrototypeIncludes(url, '/node_modules/') ||
100-
// On Windows some generated coverages are invalid.
101-
!StringPrototypeStartsWith(url, 'file:')) {
102-
continue;
103-
}
104-
10598
// Split the file source into lines. Make sure the lines maintain their
10699
// original line endings because those characters are necessary for
107100
// determining offsets in the file.
@@ -345,8 +338,7 @@ function mapRangeToLines(range, lines) {
345338
}
346339

347340
function getCoverageFromDirectory(coverageDirectory) {
348-
// TODO(cjihrig): Instead of only reading the coverage file for this process,
349-
// combine all coverage files in the directory into a single data structure.
341+
const result = new SafeMap();
350342
let dir;
351343

352344
try {
@@ -359,13 +351,149 @@ function getCoverageFromDirectory(coverageDirectory) {
359351

360352
const coverageFile = join(coverageDirectory, entry.name);
361353
const coverage = JSONParse(readFileSync(coverageFile, 'utf8'));
362-
return coverage.result;
354+
355+
mergeCoverage(result, coverage.result);
363356
}
357+
358+
return ArrayFrom(result.values());
364359
} finally {
365360
if (dir) {
366361
dir.closeSync();
367362
}
368363
}
369364
}
370365

366+
function mergeCoverage(merged, coverage) {
367+
for (let i = 0; i < coverage.length; ++i) {
368+
const newScript = coverage[i];
369+
const { url } = newScript;
370+
371+
// Filter out core modules and the node_modules/ directory from results.
372+
if (StringPrototypeStartsWith(url, 'node:') ||
373+
StringPrototypeIncludes(url, '/node_modules/') ||
374+
// On Windows some generated coverages are invalid.
375+
!StringPrototypeStartsWith(url, 'file:')) {
376+
continue;
377+
}
378+
379+
const oldScript = merged.get(url);
380+
381+
if (oldScript === undefined) {
382+
merged.set(url, newScript);
383+
} else {
384+
mergeCoverageScripts(oldScript, newScript);
385+
}
386+
}
387+
}
388+
389+
function mergeCoverageScripts(oldScript, newScript) {
390+
// Merge the functions from the new coverage into the functions from the
391+
// existing (merged) coverage.
392+
for (let i = 0; i < newScript.functions.length; ++i) {
393+
const newFn = newScript.functions[i];
394+
let found = false;
395+
396+
for (let j = 0; j < oldScript.functions.length; ++j) {
397+
const oldFn = oldScript.functions[j];
398+
399+
if (newFn.functionName === oldFn.functionName &&
400+
newFn.ranges?.[0].startOffset === oldFn.ranges?.[0].startOffset &&
401+
newFn.ranges?.[0].endOffset === oldFn.ranges?.[0].endOffset) {
402+
// These are the same functions.
403+
found = true;
404+
405+
// If newFn is block level coverage, then it will:
406+
// - Replace oldFn if oldFn is not block level coverage.
407+
// - Merge with oldFn if it is also block level coverage.
408+
// If newFn is not block level coverage, then it has no new data.
409+
if (newFn.isBlockCoverage) {
410+
if (oldFn.isBlockCoverage) {
411+
// Merge the oldFn ranges with the newFn ranges.
412+
mergeCoverageRanges(oldFn, newFn);
413+
} else {
414+
// Replace oldFn with newFn.
415+
oldFn.isBlockCoverage = true;
416+
oldFn.ranges = newFn.ranges;
417+
}
418+
}
419+
420+
break;
421+
}
422+
}
423+
424+
if (!found) {
425+
// This is a new function to track. This is possible because V8 can
426+
// generate a different list of functions depending on which code paths
427+
// are executed. For example, if a code path dynamically creates a
428+
// function, but that code path is not executed then the function does
429+
// not show up in the coverage report. Unfortunately, this also means
430+
// that the function counts in the coverage summary can never be
431+
// guaranteed to be 100% accurate.
432+
ArrayPrototypePush(oldScript.functions, newFn);
433+
}
434+
}
435+
}
436+
437+
function mergeCoverageRanges(oldFn, newFn) {
438+
const mergedRanges = new SafeSet();
439+
440+
// Keep all of the existing covered ranges.
441+
for (let i = 0; i < oldFn.ranges.length; ++i) {
442+
const oldRange = oldFn.ranges[i];
443+
444+
if (oldRange.count > 0) {
445+
mergedRanges.add(oldRange);
446+
}
447+
}
448+
449+
// Merge in the new ranges where appropriate.
450+
for (let i = 0; i < newFn.ranges.length; ++i) {
451+
const newRange = newFn.ranges[i];
452+
let exactMatch = false;
453+
454+
for (let j = 0; j < oldFn.ranges.length; ++j) {
455+
const oldRange = oldFn.ranges[j];
456+
457+
if (doesRangeEqualOtherRange(newRange, oldRange)) {
458+
// These are the same ranges, so keep the existing one.
459+
oldRange.count += newRange.count;
460+
mergedRanges.add(oldRange);
461+
exactMatch = true;
462+
break;
463+
}
464+
465+
// Look at ranges representing missing coverage and add ranges that
466+
// represent the intersection.
467+
if (oldRange.count === 0 && newRange.count === 0) {
468+
if (doesRangeContainOtherRange(oldRange, newRange)) {
469+
// The new range is completely within the old range. Discard the
470+
// larger (old) range, and keep the smaller (new) range.
471+
mergedRanges.add(newRange);
472+
} else if (doesRangeContainOtherRange(newRange, oldRange)) {
473+
// The old range is completely within the new range. Discard the
474+
// larger (new) range, and keep the smaller (old) range.
475+
mergedRanges.add(oldRange);
476+
}
477+
}
478+
}
479+
480+
// Add new ranges that do not represent missing coverage.
481+
if (newRange.count > 0 && !exactMatch) {
482+
mergedRanges.add(newRange);
483+
}
484+
}
485+
486+
oldFn.ranges = ArrayFrom(mergedRanges);
487+
}
488+
489+
function doesRangeEqualOtherRange(range, otherRange) {
490+
return range.startOffset === otherRange.startOffset &&
491+
range.endOffset === otherRange.endOffset;
492+
}
493+
494+
function doesRangeContainOtherRange(range, otherRange) {
495+
return range.startOffset <= otherRange.startOffset &&
496+
range.endOffset >= otherRange.endOffset;
497+
}
498+
371499
module.exports = { setupCoverage };

‎src/node_options.cc

-7
Original file line numberDiff line numberDiff line change
@@ -141,13 +141,6 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
141141
}
142142

143143
if (test_runner) {
144-
if (test_runner_coverage) {
145-
// TODO(cjihrig): This restriction can be removed once multi-process
146-
// code coverage is supported.
147-
errors->push_back(
148-
"--experimental-test-coverage cannot be used with --test");
149-
}
150-
151144
if (syntax_check_only) {
152145
errors->push_back("either --test or --check can be used, not both");
153146
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
'use strict';
2+
function fnA() {
3+
let cnt = 0;
4+
5+
try {
6+
cnt++;
7+
throw new Error('boom');
8+
cnt++;
9+
} catch (err) {
10+
cnt++;
11+
} finally {
12+
if (false) {
13+
14+
}
15+
16+
return cnt;
17+
}
18+
cnt++;
19+
}
20+
21+
function fnB(arr) {
22+
for (let i = 0; i < arr.length; ++i) {
23+
if (i === 2) {
24+
continue;
25+
} else {
26+
fnE(1);
27+
}
28+
}
29+
}
30+
31+
function fnC(arg1, arg2) {
32+
if (arg1 === 1) {
33+
if (arg2 === 3) {
34+
return -1;
35+
}
36+
37+
if (arg2 === 4) {
38+
return 3;
39+
}
40+
41+
if (arg2 === 5) {
42+
return 9;
43+
}
44+
}
45+
}
46+
47+
function fnD(arg) {
48+
let cnt = 0;
49+
50+
if (arg % 2 === 0) {
51+
cnt++;
52+
} else if (arg === 1) {
53+
cnt++;
54+
} else if (arg === 3) {
55+
cnt++;
56+
} else {
57+
fnC(1, 5);
58+
}
59+
60+
return cnt;
61+
}
62+
63+
function fnE(arg) {
64+
const a = arg ?? 5;
65+
66+
return a;
67+
}
68+
69+
module.exports = { fnA, fnB, fnC, fnD, fnE };
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict';
2+
const { test } = require('node:test');
3+
const common = require('./common');
4+
5+
function notCalled() {
6+
}
7+
8+
test('first 1', () => {
9+
common.fnA();
10+
common.fnD(100);
11+
common.fnB([1, 2, 3]);
12+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
'use strict';
2+
const { test } = require('node:test');
3+
const common = require('./common');
4+
5+
test('second 1', () => {
6+
common.fnB([1, 2, 3]);
7+
common.fnD(3);
8+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
'use strict';
2+
const { test } = require('node:test');
3+
const common = require('./common');
4+
5+
test('third 1', () => {
6+
common.fnC(1, 4);
7+
common.fnD(99);
8+
});

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

+50-30
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ const { readdirSync } = require('node:fs');
66
const { test } = require('node:test');
77
const fixtures = require('../common/fixtures');
88
const tmpdir = require('../common/tmpdir');
9+
const skipIfNoInspector = {
10+
skip: !process.features.inspector ? 'inspector disabled' : false
11+
};
912

1013
tmpdir.refresh();
1114

@@ -57,20 +60,6 @@ function getSpecCoverageFixtureReport() {
5760
}
5861

5962
test('test coverage report', async (t) => {
60-
await t.test('--experimental-test-coverage and --test cannot be combined', () => {
61-
// TODO(cjihrig): This test can be removed once multi-process code coverage
62-
// is supported.
63-
const args = ['--test', '--experimental-test-coverage'];
64-
const result = spawnSync(process.execPath, args);
65-
66-
// 9 is the documented exit code for an invalid CLI argument.
67-
assert.strictEqual(result.status, 9);
68-
assert.match(
69-
result.stderr.toString(),
70-
/--experimental-test-coverage cannot be used with --test/
71-
);
72-
});
73-
7463
await t.test('handles the inspector not being available', (t) => {
7564
if (process.features.inspector) {
7665
return;
@@ -87,12 +76,8 @@ test('test coverage report', async (t) => {
8776
});
8877
});
8978

90-
test('test tap coverage reporter', async (t) => {
79+
test('test tap coverage reporter', skipIfNoInspector, async (t) => {
9180
await t.test('coverage is reported and dumped to NODE_V8_COVERAGE if present', (t) => {
92-
if (!process.features.inspector) {
93-
return;
94-
}
95-
9681
const fixture = fixtures.path('test-runner', 'coverage.js');
9782
const args = ['--experimental-test-coverage', '--test-reporter', 'tap', fixture];
9883
const options = { env: { ...process.env, NODE_V8_COVERAGE: tmpdir.path } };
@@ -106,10 +91,6 @@ test('test tap coverage reporter', async (t) => {
10691
});
10792

10893
await t.test('coverage is reported without NODE_V8_COVERAGE present', (t) => {
109-
if (!process.features.inspector) {
110-
return;
111-
}
112-
11394
const fixture = fixtures.path('test-runner', 'coverage.js');
11495
const args = ['--experimental-test-coverage', '--test-reporter', 'tap', fixture];
11596
const result = spawnSync(process.execPath, args);
@@ -122,11 +103,8 @@ test('test tap coverage reporter', async (t) => {
122103
});
123104
});
124105

125-
test('test spec coverage reporter', async (t) => {
106+
test('test spec coverage reporter', skipIfNoInspector, async (t) => {
126107
await t.test('coverage is reported and dumped to NODE_V8_COVERAGE if present', (t) => {
127-
if (!process.features.inspector) {
128-
return;
129-
}
130108
const fixture = fixtures.path('test-runner', 'coverage.js');
131109
const args = ['--experimental-test-coverage', '--test-reporter', 'spec', fixture];
132110
const options = { env: { ...process.env, NODE_V8_COVERAGE: tmpdir.path } };
@@ -140,9 +118,6 @@ test('test spec coverage reporter', async (t) => {
140118
});
141119

142120
await t.test('coverage is reported without NODE_V8_COVERAGE present', (t) => {
143-
if (!process.features.inspector) {
144-
return;
145-
}
146121
const fixture = fixtures.path('test-runner', 'coverage.js');
147122
const args = ['--experimental-test-coverage', '--test-reporter', 'spec', fixture];
148123
const result = spawnSync(process.execPath, args);
@@ -154,3 +129,48 @@ test('test spec coverage reporter', async (t) => {
154129
assert(!findCoverageFileForPid(result.pid));
155130
});
156131
});
132+
133+
test('single process coverage is the same with --test', skipIfNoInspector, () => {
134+
const fixture = fixtures.path('test-runner', 'coverage.js');
135+
const args = [
136+
'--test', '--experimental-test-coverage', '--test-reporter', 'tap', fixture,
137+
];
138+
const result = spawnSync(process.execPath, args);
139+
const report = getTapCoverageFixtureReport();
140+
141+
assert.strictEqual(result.stderr.toString(), '');
142+
assert(result.stdout.toString().includes(report));
143+
assert.strictEqual(result.status, 0);
144+
assert(!findCoverageFileForPid(result.pid));
145+
});
146+
147+
test('coverage is combined for multiple processes', skipIfNoInspector, () => {
148+
let report = [
149+
'# start of coverage report',
150+
'# file | line % | branch % | funcs % | uncovered lines',
151+
'# test/fixtures/v8-coverage/combined_coverage/common.js | 89.86 | ' +
152+
'62.50 | 100.00 | 8, 13, 14, 18, 34, 35, 53',
153+
'# test/fixtures/v8-coverage/combined_coverage/first.test.js | 83.33 | ' +
154+
'100.00 | 50.00 | 5, 6',
155+
'# test/fixtures/v8-coverage/combined_coverage/second.test.js | 100.00 ' +
156+
'| 100.00 | 100.00 | ',
157+
'# test/fixtures/v8-coverage/combined_coverage/third.test.js | 100.00 | ' +
158+
'100.00 | 100.00 | ',
159+
'# all files | 90.72 | 72.73 | 88.89 |',
160+
'# end of coverage report',
161+
].join('\n');
162+
163+
if (common.isWindows) {
164+
report = report.replaceAll('/', '\\');
165+
}
166+
167+
const fixture = fixtures.path('v8-coverage', 'combined_coverage');
168+
const args = [
169+
'--test', '--experimental-test-coverage', '--test-reporter', 'tap', fixture,
170+
];
171+
const result = spawnSync(process.execPath, args);
172+
173+
assert.strictEqual(result.stderr.toString(), '');
174+
assert(result.stdout.toString().includes(report));
175+
assert.strictEqual(result.status, 0);
176+
});

0 commit comments

Comments
 (0)
Please sign in to comment.