test_runner: exclude test files from code coverage by default

Is not usual to test the code coverage of test files.

Fixes: nodejs#53508

doc/api/cli.md

@@ -2324,6 +2324,8 @@ This option may be specified multiple times to exclude multiple glob patterns.
 If both `--test-coverage-exclude` and `--test-coverage-include` are provided,
 files must meet **both** criteria to be included in the coverage report.
 
+If no `--test-coverage-exclude` is provided, all test files will be excluded.
+
 ### `--test-coverage-functions=threshold`
 
 <!-- YAML

doc/api/test.md

@@ -1353,6 +1353,7 @@ changes:
     This property is only applicable when `coverage` was set to `true`.
     If both `coverageExcludeGlobs` and `coverageIncludeGlobs` are provided,
     files must meet **both** criteria to be included in the coverage report.
+    If `undefined`, all test files will be excluded.
     **Default:** `undefined`.
   * `coverageIncludeGlobs` {string|Array} Includes specific files in code coverage
     using a glob pattern, which can match both absolute and relative file paths.

lib/internal/test_runner/coverage.js

@@ -138,9 +138,9 @@ class TestCoverage {
     return lines;
   }
 
-  summary() {
+  summary(excludeFiles) {
     internalBinding('profiler').takeCoverage();
-    const coverage = this.getCoverageFromDirectory();
+    const coverage = this.getCoverageFromDirectory(excludeFiles);
     const coverageSummary = {
       __proto__: null,
       workingDirectory: this.workingDirectory,
@@ -319,7 +319,7 @@ class TestCoverage {
     }
   }
 
-  getCoverageFromDirectory() {
+  getCoverageFromDirectory(excludeFiles) {
     const result = new SafeMap();
     let dir;
 
@@ -333,7 +333,7 @@ class TestCoverage {
 
         const coverageFile = join(this.coverageDirectory, entry.name);
         const coverage = JSONParse(readFileSync(coverageFile, 'utf8'));
-        this.mergeCoverage(result, this.mapCoverageWithSourceMap(coverage));
+        this.mergeCoverage(result, this.mapCoverageWithSourceMap(coverage, excludeFiles), excludeFiles);
       }
 
       return ArrayFrom(result.values());
@@ -345,7 +345,7 @@ class TestCoverage {
   }
 
 
-  mapCoverageWithSourceMap(coverage) {
+  mapCoverageWithSourceMap(coverage, excludeFiles) {
     const { result } = coverage;
     const sourceMapCache = coverage['source-map-cache'];
     if (!this.sourceMaps || !sourceMapCache) {
@@ -356,7 +356,7 @@ class TestCoverage {
       const script = result[i];
       const { url, functions } = script;
 
-      if (this.shouldSkipFileCoverage(url) || sourceMapCache[url] == null) {
+      if (this.shouldSkipFileCoverage(url, excludeFiles) || sourceMapCache[url] == null) {
         newResult.set(url, script);
         continue;
       }
@@ -436,12 +436,12 @@ class TestCoverage {
     return MathMin(lines[line].startOffset + entry.originalColumn, lines[line].endOffset);
   }
 
-  mergeCoverage(merged, coverage) {
+  mergeCoverage(merged, coverage, excludeFiles) {
     for (let i = 0; i < coverage.length; ++i) {
       const newScript = coverage[i];
       const { url } = newScript;
 
-      if (this.shouldSkipFileCoverage(url)) {
+      if (this.shouldSkipFileCoverage(url, excludeFiles)) {
         continue;
       }
 
@@ -455,7 +455,7 @@ class TestCoverage {
     }
   }
 
-  shouldSkipFileCoverage(url) {
+  shouldSkipFileCoverage(url, excludeFiles) {
     // This check filters out core modules, which start with 'node:' in
     // coverage reports, as well as any invalid coverages which have been
     // observed on Windows.
@@ -465,11 +465,13 @@ class TestCoverage {
     const relativePath = relative(this.workingDirectory, absolutePath);
 
     // This check filters out files that match the exclude globs.
-    if (this.excludeGlobs?.length > 0) {
+    if (this.excludeGlobs) {
       for (let i = 0; i < this.excludeGlobs.length; ++i) {
         if (matchesGlob(relativePath, this.excludeGlobs[i]) ||
             matchesGlob(absolutePath, this.excludeGlobs[i])) return true;
       }
+    } else if (excludeFiles?.includes(absolutePath)) {
+      return true;
     }
 
     // This check filters out files that do not match the include globs.

lib/internal/test_runner/harness.js

@@ -167,7 +167,7 @@ function collectCoverage(rootTest, coverage) {
   let summary = null;
 
   try {
-    summary = coverage.summary();
+    summary = coverage.summary(rootTest.getFiles());
   } catch (err) {
     rootTest.diagnostic(`Warning: Could not report code coverage. ${err}`);
     rootTest.harness.success = false;

lib/internal/test_runner/runner.js

@@ -186,6 +186,7 @@ class FileTest extends Test {
 
   constructor(options) {
     super(options);
+    this.filePath = resolve(this.config.cwd, this.name);
     this.loc ??= {
       __proto__: null,
       line: 1,

lib/internal/test_runner/test.js

@@ -998,7 +998,19 @@ class Test extends AsyncResource {
       process.exit();
     }
   }
-
+  getFiles() {
+    const files = [];
+    if (this.filePath) {
+      files.push(this.filePath);
+    }
+    if (this.subtests) {
+      for (const test of this.subtests) {
+        const subFiles = test.getFiles();
+        files.push(...subFiles);
+      }
+    }
+    return files;
+  }
   postRun(pendingSubtestsError) {
     // If the test was cancelled before it started, then the start and end
     // times need to be corrected.

test/parallel/test-runner-coverage-source-map.js

@@ -20,7 +20,7 @@ function generateReport(report) {
 
 const flags = [
   '--enable-source-maps',
-  '--test', '--experimental-test-coverage', '--test-reporter', 'tap',
+  '--test', '--experimental-test-coverage', '--no-warnings', '--test-coverage-include=**', '--test-reporter', 'tap',
 ];
 
 describe('Coverage with source maps', async () => {

test/parallel/test-runner-coverage-thresholds.js

@@ -61,6 +61,8 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--no-warnings',
+      '--test-coverage-include=**',
       `${coverage.flag}=25`,
       '--test-reporter', 'tap',
       fixture,
@@ -77,6 +79,8 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--no-warnings',
+      '--test-coverage-include=**',
       `${coverage.flag}=25`,
       '--test-reporter', reporter,
       fixture,
@@ -92,6 +96,8 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--no-warnings',
+      '--test-coverage-include=**',
       `${coverage.flag}=99`,
       '--test-reporter', 'tap',
       fixture,
@@ -108,6 +114,8 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--no-warnings',
+      '--test-coverage-include=**',
       `${coverage.flag}=99`,
       '--test-reporter', reporter,
       fixture,
@@ -123,6 +131,8 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--no-warnings',
+      '--test-coverage-include=**',
       `${coverage.flag}=101`,
       fixture,
     ]);
@@ -136,6 +146,8 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--no-warnings',
+      '--test-coverage-include=**',
       `${coverage.flag}=-1`,
       fixture,
     ]);

test/parallel/test-runner-coverage.js

@@ -150,7 +150,9 @@ test('test spec coverage reporter', skipIfNoInspector, async (t) => {
 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,
+    '--test', '--experimental-test-coverage',
+    '--no-warnings', '--test-coverage-include=**',
+    '--test-reporter', 'tap', fixture,
   ];
   const result = spawnSync(process.execPath, args);
   const report = getTapCoverageFixtureReport();
@@ -183,7 +185,7 @@ test('coverage is combined for multiple processes', skipIfNoInspector, () => {
 
   const fixture = fixtures.path('v8-coverage', 'combined_coverage');
   const args = [
-    '--test', '--experimental-test-coverage', '--test-reporter', 'tap',
+    '--test', '--experimental-test-coverage', '--no-warnings', '--test-coverage-include=**', '--test-reporter', 'tap',
   ];
   const result = spawnSync(process.execPath, args, {
     env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
@@ -236,7 +238,8 @@ test.skip('coverage works with isolation=none', skipIfNoInspector, () => {
 test('coverage reports on lines, functions, and branches', skipIfNoInspector, async (t) => {
   const fixture = fixtures.path('test-runner', 'coverage.js');
   const child = spawnSync(process.execPath,
-                          ['--test', '--experimental-test-coverage', '--test-reporter',
+                          ['--test', '--experimental-test-coverage',
+                           '--no-warnings', '--test-coverage-include=**', '--test-reporter',
                            fixtures.fileURL('test-runner/custom_reporters/coverage.mjs'),
                            fixture]);
   assert.strictEqual(child.stderr.toString(), '');
@@ -297,7 +300,6 @@ test('coverage with ESM hook - source irrelevant', skipIfNoInspector, () => {
     '# ------------------------------------------------------------------',
     '# hooks.mjs         | 100.00 |   100.00 |  100.00 | ',
     '# register-hooks.js | 100.00 |   100.00 |  100.00 | ',
-    '# virtual.js        | 100.00 |   100.00 |  100.00 | ',
     '# ------------------------------------------------------------------',
     '# all files         | 100.00 |   100.00 |  100.00 | ',
     '# ------------------------------------------------------------------',
@@ -327,7 +329,6 @@ test('coverage with ESM hook - source transpiled', skipIfNoInspector, () => {
     '# ------------------------------------------------------------------',
     '# hooks.mjs         | 100.00 |   100.00 |  100.00 | ',
     '# register-hooks.js | 100.00 |   100.00 |  100.00 | ',
-    '# sum.test.ts       | 100.00 |   100.00 |  100.00 | ',
     '# sum.ts            | 100.00 |   100.00 |  100.00 | ',
     '# ------------------------------------------------------------------',
     '# all files         | 100.00 |   100.00 |  100.00 | ',
@@ -384,7 +385,38 @@ test('coverage with excluded files', skipIfNoInspector, () => {
   assert.strictEqual(result.status, 0);
   assert(!findCoverageFileForPid(result.pid));
 });
+test('coverage should not include test files by default', skipIfNoInspector, () => {
+  const fixture = fixtures.path('test-runner', 'coverage.js');
+  const args = [
+    '--experimental-test-coverage', '--test-reporter', 'tap',
+    fixture,
+  ];
+  const result = spawnSync(process.execPath, args);
+  const report = [
+    '# start of coverage report',
+    '# ---------------------------------------------------------------',
+    '# file           | line % | branch % | funcs % | uncovered lines',
+    '# ---------------------------------------------------------------',
+    '# test           |        |          |         | ',
+    '#  fixtures      |        |          |         | ',
+    '#   test-runner  |        |          |         | ',
+    '#   v8-coverage  |        |          |         | ',
+    '#    throw.js    |  71.43 |    50.00 |  100.00 | 5-6',
+    '# ---------------------------------------------------------------',
+    '# all files      |  78.13 |    40.00 |   60.00 | ',
+    '# ---------------------------------------------------------------',
+    '# end of coverage report',
+  ].join('\n');
 
+
+  if (common.isWindows) {
+    return report.replaceAll('/', '\\');
+  }
+
+  assert(result.stdout.toString().includes(report));
+  assert.strictEqual(result.status, 0);
+  assert(!findCoverageFileForPid(result.pid));
+});
 test('coverage with included files', skipIfNoInspector, () => {
   const fixture = fixtures.path('test-runner', 'coverage.js');
   const args = [
@@ -458,18 +490,17 @@ test('coverage with included and excluded files', skipIfNoInspector, () => {
 test('correctly prints the coverage report of files contained in parent directories', skipIfNoInspector, () => {
   let report = [
     '# start of coverage report',
-    '# --------------------------------------------------------------------------------------------',
+    '# ------------------------------------------------------------------',
     '# file              | line % | branch % | funcs % | uncovered lines',
-    '# --------------------------------------------------------------------------------------------',
+    '# ------------------------------------------------------------------',
     '# ..                |        |          |         | ',
-    '#  coverage.js      |  78.65 |    38.46 |   60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72',
     '#  invalid-tap.js   | 100.00 |   100.00 |  100.00 | ',
     '#  ..               |        |          |         | ',
     '#   v8-coverage     |        |          |         | ',
     '#    throw.js       |  71.43 |    50.00 |  100.00 | 5-6',
-    '# --------------------------------------------------------------------------------------------',
-    '# all files         |  78.35 |    43.75 |   60.00 | ',
-    '# --------------------------------------------------------------------------------------------',
+    '# ------------------------------------------------------------------',
+    '# all files         |  75.00 |    66.67 |  100.00 | ',
+    '# ------------------------------------------------------------------',
     '# end of coverage report',
   ].join('\n');
 

test/parallel/test-runner-run-coverage.mjs

@@ -137,13 +137,13 @@ describe('require(\'node:test\').run coverage settings', { concurrency: true },
 
     await it('should run while including and excluding globs', async () => {
       const stream = run({
-        files: [...files, fixtures.path('test-runner/invalid-tap.js')],
+        files: files,
         coverage: true,
         coverageIncludeGlobs: ['test/fixtures/test-runner/*.js'],
         coverageExcludeGlobs: ['test/fixtures/test-runner/*-tap.js']
       });
       stream.on('test:fail', common.mustNotCall());
-      stream.on('test:pass', common.mustCall(2));
+      stream.on('test:pass', common.mustCall(1));
       stream.on('test:coverage', common.mustCall(({ summary: { files } }) => {
         const filesPaths = files.map(({ path }) => path);
         assert.strictEqual(filesPaths.every((path) => !path.includes(`test-runner${sep}invalid-tap.js`)), true);
@@ -157,7 +157,8 @@ describe('require(\'node:test\').run coverage settings', { concurrency: true },
       const thresholdErrors = [];
       const originalExitCode = process.exitCode;
       assert.notStrictEqual(originalExitCode, 1);
-      const stream = run({ files, coverage: true, lineCoverage: 99, branchCoverage: 99, functionCoverage: 99 });
+      const stream = run({ files, coverageIncludeGlobs: '**', coverage: true,
+                           lineCoverage: 99, branchCoverage: 99, functionCoverage: 99 });
       stream.on('test:fail', common.mustNotCall());
       stream.on('test:pass', common.mustCall(1));
       stream.on('test:diagnostic', ({ message }) => {