diff options
| -rw-r--r-- | doc/api/cli.md | 4 | ||||
| -rw-r--r-- | doc/api/test.md | 4 | ||||
| -rw-r--r-- | lib/internal/test_runner/coverage.js | 156 | ||||
| -rw-r--r-- | src/node_options.cc | 7 | ||||
| -rw-r--r-- | test/fixtures/v8-coverage/combined_coverage/common.js | 69 | ||||
| -rw-r--r-- | test/fixtures/v8-coverage/combined_coverage/first.test.js | 12 | ||||
| -rw-r--r-- | test/fixtures/v8-coverage/combined_coverage/second.test.js | 8 | ||||
| -rw-r--r-- | test/fixtures/v8-coverage/combined_coverage/third.test.js | 8 | ||||
| -rw-r--r-- | test/parallel/test-runner-coverage.js | 80 |
9 files changed, 293 insertions, 55 deletions
diff --git a/doc/api/cli.md b/doc/api/cli.md index b4406bcec5..8daba8b46e 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -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 diff --git a/doc/api/test.md b/doc/api/test.md index 47cf679a10..61658b886c 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -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. diff --git a/lib/internal/test_runner/coverage.js b/lib/internal/test_runner/coverage.js index 12dcb1da98..1f5aa919a4 100644 --- a/lib/internal/test_runner/coverage.js +++ b/lib/internal/test_runner/coverage.js @@ -1,13 +1,15 @@ 'use strict'; const { + ArrayFrom, ArrayPrototypeMap, ArrayPrototypePush, JSONParse, MathFloor, NumberParseInt, - RegExp, RegExpPrototypeExec, RegExpPrototypeSymbolSplit, + SafeMap, + SafeSet, StringPrototypeIncludes, StringPrototypeLocaleCompare, StringPrototypeStartsWith, @@ -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; @@ -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. @@ -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 { @@ -359,8 +351,11 @@ 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(); @@ -368,4 +363,137 @@ function getCoverageFromDirectory(coverageDirectory) { } } +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 }; diff --git a/src/node_options.cc b/src/node_options.cc index bbe74d3f83..4e3633e5b5 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -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"); } diff --git a/test/fixtures/v8-coverage/combined_coverage/common.js b/test/fixtures/v8-coverage/combined_coverage/common.js new file mode 100644 index 0000000000..4d8bda4305 --- /dev/null +++ b/test/fixtures/v8-coverage/combined_coverage/common.js @@ -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 }; diff --git a/test/fixtures/v8-coverage/combined_coverage/first.test.js b/test/fixtures/v8-coverage/combined_coverage/first.test.js new file mode 100644 index 0000000000..29a98e349b --- /dev/null +++ b/test/fixtures/v8-coverage/combined_coverage/first.test.js @@ -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]); +}); diff --git a/test/fixtures/v8-coverage/combined_coverage/second.test.js b/test/fixtures/v8-coverage/combined_coverage/second.test.js new file mode 100644 index 0000000000..0b7e292ddb --- /dev/null +++ b/test/fixtures/v8-coverage/combined_coverage/second.test.js @@ -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); +}); diff --git a/test/fixtures/v8-coverage/combined_coverage/third.test.js b/test/fixtures/v8-coverage/combined_coverage/third.test.js new file mode 100644 index 0000000000..5416f44022 --- /dev/null +++ b/test/fixtures/v8-coverage/combined_coverage/third.test.js @@ -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); +}); diff --git a/test/parallel/test-runner-coverage.js b/test/parallel/test-runner-coverage.js index 01fc866719..125fb3228d 100644 --- a/test/parallel/test-runner-coverage.js +++ b/test/parallel/test-runner-coverage.js @@ -6,6 +6,9 @@ const { readdirSync } = require('node:fs'); const { test } = require('node:test'); const fixtures = require('../common/fixtures'); const tmpdir = require('../common/tmpdir'); +const skipIfNoInspector = { + skip: !process.features.inspector ? 'inspector disabled' : false +}; tmpdir.refresh(); @@ -57,20 +60,6 @@ function getSpecCoverageFixtureReport() { } test('test coverage report', async (t) => { - await t.test('--experimental-test-coverage and --test cannot be combined', () => { - // TODO(cjihrig): This test can be removed once multi-process code coverage - // is supported. - const args = ['--test', '--experimental-test-coverage']; - const result = spawnSync(process.execPath, args); - - // 9 is the documented exit code for an invalid CLI argument. - assert.strictEqual(result.status, 9); - assert.match( - result.stderr.toString(), - /--experimental-test-coverage cannot be used with --test/ - ); - }); - await t.test('handles the inspector not being available', (t) => { if (process.features.inspector) { return; @@ -87,12 +76,8 @@ test('test coverage report', async (t) => { }); }); -test('test tap coverage reporter', async (t) => { +test('test tap coverage reporter', skipIfNoInspector, async (t) => { await t.test('coverage is reported and dumped to NODE_V8_COVERAGE if present', (t) => { - if (!process.features.inspector) { - return; - } - const fixture = fixtures.path('test-runner', 'coverage.js'); const args = ['--experimental-test-coverage', '--test-reporter', 'tap', fixture]; const options = { env: { ...process.env, NODE_V8_COVERAGE: tmpdir.path } }; @@ -106,10 +91,6 @@ test('test tap coverage reporter', async (t) => { }); await t.test('coverage is reported without NODE_V8_COVERAGE present', (t) => { - if (!process.features.inspector) { - return; - } - const fixture = fixtures.path('test-runner', 'coverage.js'); const args = ['--experimental-test-coverage', '--test-reporter', 'tap', fixture]; const result = spawnSync(process.execPath, args); @@ -122,11 +103,8 @@ test('test tap coverage reporter', async (t) => { }); }); -test('test spec coverage reporter', async (t) => { +test('test spec coverage reporter', skipIfNoInspector, async (t) => { await t.test('coverage is reported and dumped to NODE_V8_COVERAGE if present', (t) => { - if (!process.features.inspector) { - return; - } const fixture = fixtures.path('test-runner', 'coverage.js'); const args = ['--experimental-test-coverage', '--test-reporter', 'spec', fixture]; const options = { env: { ...process.env, NODE_V8_COVERAGE: tmpdir.path } }; @@ -140,9 +118,6 @@ test('test spec coverage reporter', async (t) => { }); await t.test('coverage is reported without NODE_V8_COVERAGE present', (t) => { - if (!process.features.inspector) { - return; - } const fixture = fixtures.path('test-runner', 'coverage.js'); const args = ['--experimental-test-coverage', '--test-reporter', 'spec', fixture]; const result = spawnSync(process.execPath, args); @@ -154,3 +129,48 @@ test('test spec coverage reporter', async (t) => { assert(!findCoverageFileForPid(result.pid)); }); }); + +test('single process coverage is the same with --test', skipIfNoInspector, () => { + const fixture = fixtures.path('test-runner', 'coverage.js'); + const args = [ + '--test', '--experimental-test-coverage', '--test-reporter', 'tap', fixture, + ]; + const result = spawnSync(process.execPath, args); + const report = getTapCoverageFixtureReport(); + + assert.strictEqual(result.stderr.toString(), ''); + assert(result.stdout.toString().includes(report)); + assert.strictEqual(result.status, 0); + assert(!findCoverageFileForPid(result.pid)); +}); + +test('coverage is combined for multiple processes', skipIfNoInspector, () => { + let report = [ + '# start of coverage report', + '# file | line % | branch % | funcs % | uncovered lines', + '# test/fixtures/v8-coverage/combined_coverage/common.js | 89.86 | ' + + '62.50 | 100.00 | 8, 13, 14, 18, 34, 35, 53', + '# test/fixtures/v8-coverage/combined_coverage/first.test.js | 83.33 | ' + + '100.00 | 50.00 | 5, 6', + '# test/fixtures/v8-coverage/combined_coverage/second.test.js | 100.00 ' + + '| 100.00 | 100.00 | ', + '# test/fixtures/v8-coverage/combined_coverage/third.test.js | 100.00 | ' + + '100.00 | 100.00 | ', + '# all files | 90.72 | 72.73 | 88.89 |', + '# end of coverage report', + ].join('\n'); + + if (common.isWindows) { + report = report.replaceAll('/', '\\'); + } + + const fixture = fixtures.path('v8-coverage', 'combined_coverage'); + const args = [ + '--test', '--experimental-test-coverage', '--test-reporter', 'tap', fixture, + ]; + const result = spawnSync(process.execPath, args); + + assert.strictEqual(result.stderr.toString(), ''); + assert(result.stdout.toString().includes(report)); + assert.strictEqual(result.status, 0); +}); |