Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 0e70c18

Browse filesBrowse files
cjihrigtargos
cjihrig
authored and
targos
committed
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 <moshe@atlow.co.il> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 464ddf9 commit 0e70c18
Copy full SHA for 0e70c18

File tree

Expand file treeCollapse file tree

9 files changed

+293
-55
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

9 files changed

+293
-55
lines changed
Open diff view settings
Collapse file

‎doc/api/cli.md‎

Copy file name to clipboardExpand all lines: doc/api/cli.md
+4Lines changed: 4 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
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
Collapse file

‎doc/api/test.md‎

Copy file name to clipboardExpand all lines: doc/api/test.md
-4Lines changed: 0 additions & 4 deletions
  • Display the source diff
  • Display the rich diff
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.
Collapse file

‎lib/internal/test_runner/coverage.js‎

Copy file name to clipboardExpand all lines: lib/internal/test_runner/coverage.js
+142-14Lines changed: 142 additions & 14 deletions
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 };
Collapse file

‎src/node_options.cc‎

Copy file name to clipboardExpand all lines: src/node_options.cc
-7Lines changed: 0 additions & 7 deletions
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
}
Collapse file

‎test/fixtures/v8-coverage/combined_coverage/common.js‎

Copy file name to clipboard
+69Lines changed: 69 additions & 0 deletions
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 };
Collapse file

‎test/fixtures/v8-coverage/combined_coverage/first.test.js‎

Copy file name to clipboard
+12Lines changed: 12 additions & 0 deletions
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+
});
Collapse file

‎test/fixtures/v8-coverage/combined_coverage/second.test.js‎

Copy file name to clipboard
+8Lines changed: 8 additions & 0 deletions
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+
});
Collapse file

‎test/fixtures/v8-coverage/combined_coverage/third.test.js‎

Copy file name to clipboard
+8Lines changed: 8 additions & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.