Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_runner: support combining coverage reports #47686

Merged
merged 1 commit into from
Apr 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,10 @@ Use this flag to enable [ShadowRealm][] support.
added:
- v19.7.0
- 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 @@ -428,10 +428,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/') ||
MoLow marked this conversation as resolved.
Show resolved Hide resolved
// On Windows some generated coverages are invalid.
!StringPrototypeStartsWith(url, 'file:')) {
continue;
}

const oldScript = merged.get(url);
cjihrig marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -141,13 +141,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);
});
Loading