Skip to content

Commit 4c0036b

Browse files
cjihrigMoLow
authored andcommitted
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: nodejs#47686 Fixes: nodejs#47669 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent 64d19f4 commit 4c0036b

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
@@ -444,6 +444,10 @@ See [customizing ESM specifier resolution][] for example usage.
444444

445445
<!-- YAML
446446
added: v18.15.0
447+
changes:
448+
- version: REPLACEME
449+
pr-url: https://github.com/nodejs/node/pull/47686
450+
description: This option can be used with `--test`.
447451
-->
448452

449453
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
@@ -420,10 +420,6 @@ if (anAlwaysFalseCondition) {
420420
The test runner's code coverage functionality has the following limitations,
421421
which will be addressed in a future Node.js release:
422422

423-
* Although coverage data is collected for child processes, this information is
424-
not included in the coverage report. Because the command line test runner uses
425-
child processes to execute test files, it cannot be used with
426-
`--experimental-test-coverage`.
427423
* Source maps are not supported.
428424
* Excluding specific files or directories from the coverage report is not
429425
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
@@ -149,13 +149,6 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
149149
}
150150

151151
if (test_runner) {
152-
if (test_runner_coverage) {
153-
// TODO(cjihrig): This restriction can be removed once multi-process
154-
// code coverage is supported.
155-
errors->push_back(
156-
"--experimental-test-coverage cannot be used with --test");
157-
}
158-
159152
if (syntax_check_only) {
160153
errors->push_back("either --test or --check can be used, not both");
161154
}
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+
});

0 commit comments

Comments
 (0)