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: https://github.com/nodejs/node/pull/47686
Fixes: https://github.com/nodejs/node/issues/47669
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

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);
+});