Skip to content

Commit

Permalink
test_runner: support combining coverage reports
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
cjihrig authored and danielleadams committed Jul 6, 2023
1 parent 817bd0e commit 6ebef57
Show file tree
Hide file tree
Showing 9 changed files with 293 additions and 55 deletions.
4 changes: 4 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,10 @@ See [customizing ESM specifier resolution][] for example usage.

<!-- YAML
added: v18.15.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/47686
description: This option can be used with `--test`.
-->

When used in conjunction with the `node:test` module, a code coverage report is
Expand Down
4 changes: 0 additions & 4 deletions doc/api/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -420,10 +420,6 @@ if (anAlwaysFalseCondition) {
The test runner's code coverage functionality has the following limitations,
which will be addressed in a future Node.js release:

* Although coverage data is collected for child processes, this information is
not included in the coverage report. Because the command line test runner uses
child processes to execute test files, it cannot be used with
`--experimental-test-coverage`.
* Source maps are not supported.
* Excluding specific files or directories from the coverage report is not
supported.
Expand Down
156 changes: 142 additions & 14 deletions lib/internal/test_runner/coverage.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
'use strict';
const {
ArrayFrom,
ArrayPrototypeMap,
ArrayPrototypePush,
JSONParse,
MathFloor,
NumberParseInt,
RegExp,
RegExpPrototypeExec,
RegExpPrototypeSymbolSplit,
SafeMap,
SafeSet,
StringPrototypeIncludes,
StringPrototypeLocaleCompare,
StringPrototypeStartsWith,
Expand All @@ -23,9 +25,7 @@ const { setupCoverageHooks } = require('internal/util');
const { tmpdir } = require('os');
const { join, resolve } = require('path');
const { fileURLToPath } = require('url');
const kCoveragePattern =
`^coverage\\-${process.pid}\\-(\\d{13})\\-(\\d+)\\.json$`;
const kCoverageFileRegex = new RegExp(kCoveragePattern);
const kCoverageFileRegex = /^coverage-(\d+)-(\d{13})-(\d+)\.json$/;
const kIgnoreRegex = /\/\* node:coverage ignore next (?<count>\d+ )?\*\//;
const kLineEndingRegex = /\r?\n$/u;
const kLineSplitRegex = /(?<=\r?\n)/u;
Expand Down Expand Up @@ -95,13 +95,6 @@ class TestCoverage {
for (let i = 0; i < coverage.length; ++i) {
const { functions, url } = coverage[i];

if (StringPrototypeStartsWith(url, 'node:') ||
StringPrototypeIncludes(url, '/node_modules/') ||
// On Windows some generated coverages are invalid.
!StringPrototypeStartsWith(url, 'file:')) {
continue;
}

// Split the file source into lines. Make sure the lines maintain their
// original line endings because those characters are necessary for
// determining offsets in the file.
Expand Down Expand Up @@ -345,8 +338,7 @@ function mapRangeToLines(range, lines) {
}

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

try {
Expand All @@ -359,13 +351,149 @@ function getCoverageFromDirectory(coverageDirectory) {

const coverageFile = join(coverageDirectory, entry.name);
const coverage = JSONParse(readFileSync(coverageFile, 'utf8'));
return coverage.result;

mergeCoverage(result, coverage.result);
}

return ArrayFrom(result.values());
} finally {
if (dir) {
dir.closeSync();
}
}
}

function mergeCoverage(merged, coverage) {
for (let i = 0; i < coverage.length; ++i) {
const newScript = coverage[i];
const { url } = newScript;

// Filter out core modules and the node_modules/ directory from results.
if (StringPrototypeStartsWith(url, 'node:') ||
StringPrototypeIncludes(url, '/node_modules/') ||
// On Windows some generated coverages are invalid.
!StringPrototypeStartsWith(url, 'file:')) {
continue;
}

const oldScript = merged.get(url);

if (oldScript === undefined) {
merged.set(url, newScript);
} else {
mergeCoverageScripts(oldScript, newScript);
}
}
}

function mergeCoverageScripts(oldScript, newScript) {
// Merge the functions from the new coverage into the functions from the
// existing (merged) coverage.
for (let i = 0; i < newScript.functions.length; ++i) {
const newFn = newScript.functions[i];
let found = false;

for (let j = 0; j < oldScript.functions.length; ++j) {
const oldFn = oldScript.functions[j];

if (newFn.functionName === oldFn.functionName &&
newFn.ranges?.[0].startOffset === oldFn.ranges?.[0].startOffset &&
newFn.ranges?.[0].endOffset === oldFn.ranges?.[0].endOffset) {
// These are the same functions.
found = true;

// If newFn is block level coverage, then it will:
// - Replace oldFn if oldFn is not block level coverage.
// - Merge with oldFn if it is also block level coverage.
// If newFn is not block level coverage, then it has no new data.
if (newFn.isBlockCoverage) {
if (oldFn.isBlockCoverage) {
// Merge the oldFn ranges with the newFn ranges.
mergeCoverageRanges(oldFn, newFn);
} else {
// Replace oldFn with newFn.
oldFn.isBlockCoverage = true;
oldFn.ranges = newFn.ranges;
}
}

break;
}
}

if (!found) {
// This is a new function to track. This is possible because V8 can
// generate a different list of functions depending on which code paths
// are executed. For example, if a code path dynamically creates a
// function, but that code path is not executed then the function does
// not show up in the coverage report. Unfortunately, this also means
// that the function counts in the coverage summary can never be
// guaranteed to be 100% accurate.
ArrayPrototypePush(oldScript.functions, newFn);
}
}
}

function mergeCoverageRanges(oldFn, newFn) {
const mergedRanges = new SafeSet();

// Keep all of the existing covered ranges.
for (let i = 0; i < oldFn.ranges.length; ++i) {
const oldRange = oldFn.ranges[i];

if (oldRange.count > 0) {
mergedRanges.add(oldRange);
}
}

// Merge in the new ranges where appropriate.
for (let i = 0; i < newFn.ranges.length; ++i) {
const newRange = newFn.ranges[i];
let exactMatch = false;

for (let j = 0; j < oldFn.ranges.length; ++j) {
const oldRange = oldFn.ranges[j];

if (doesRangeEqualOtherRange(newRange, oldRange)) {
// These are the same ranges, so keep the existing one.
oldRange.count += newRange.count;
mergedRanges.add(oldRange);
exactMatch = true;
break;
}

// Look at ranges representing missing coverage and add ranges that
// represent the intersection.
if (oldRange.count === 0 && newRange.count === 0) {
if (doesRangeContainOtherRange(oldRange, newRange)) {
// The new range is completely within the old range. Discard the
// larger (old) range, and keep the smaller (new) range.
mergedRanges.add(newRange);
} else if (doesRangeContainOtherRange(newRange, oldRange)) {
// The old range is completely within the new range. Discard the
// larger (new) range, and keep the smaller (old) range.
mergedRanges.add(oldRange);
}
}
}

// Add new ranges that do not represent missing coverage.
if (newRange.count > 0 && !exactMatch) {
mergedRanges.add(newRange);
}
}

oldFn.ranges = ArrayFrom(mergedRanges);
}

function doesRangeEqualOtherRange(range, otherRange) {
return range.startOffset === otherRange.startOffset &&
range.endOffset === otherRange.endOffset;
}

function doesRangeContainOtherRange(range, otherRange) {
return range.startOffset <= otherRange.startOffset &&
range.endOffset >= otherRange.endOffset;
}

module.exports = { setupCoverage };
7 changes: 0 additions & 7 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,6 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
}

if (test_runner) {
if (test_runner_coverage) {
// TODO(cjihrig): This restriction can be removed once multi-process
// code coverage is supported.
errors->push_back(
"--experimental-test-coverage cannot be used with --test");
}

if (syntax_check_only) {
errors->push_back("either --test or --check can be used, not both");
}
Expand Down
69 changes: 69 additions & 0 deletions test/fixtures/v8-coverage/combined_coverage/common.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
'use strict';
function fnA() {
let cnt = 0;

try {
cnt++;
throw new Error('boom');
cnt++;
} catch (err) {
cnt++;
} finally {
if (false) {

}

return cnt;
}
cnt++;
}

function fnB(arr) {
for (let i = 0; i < arr.length; ++i) {
if (i === 2) {
continue;
} else {
fnE(1);
}
}
}

function fnC(arg1, arg2) {
if (arg1 === 1) {
if (arg2 === 3) {
return -1;
}

if (arg2 === 4) {
return 3;
}

if (arg2 === 5) {
return 9;
}
}
}

function fnD(arg) {
let cnt = 0;

if (arg % 2 === 0) {
cnt++;
} else if (arg === 1) {
cnt++;
} else if (arg === 3) {
cnt++;
} else {
fnC(1, 5);
}

return cnt;
}

function fnE(arg) {
const a = arg ?? 5;

return a;
}

module.exports = { fnA, fnB, fnC, fnD, fnE };
12 changes: 12 additions & 0 deletions test/fixtures/v8-coverage/combined_coverage/first.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';
const { test } = require('node:test');
const common = require('./common');

function notCalled() {
}

test('first 1', () => {
common.fnA();
common.fnD(100);
common.fnB([1, 2, 3]);
});
8 changes: 8 additions & 0 deletions test/fixtures/v8-coverage/combined_coverage/second.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';
const { test } = require('node:test');
const common = require('./common');

test('second 1', () => {
common.fnB([1, 2, 3]);
common.fnD(3);
});
8 changes: 8 additions & 0 deletions test/fixtures/v8-coverage/combined_coverage/third.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';
const { test } = require('node:test');
const common = require('./common');

test('third 1', () => {
common.fnC(1, 4);
common.fnD(99);
});

0 comments on commit 6ebef57

Please sign in to comment.