From cc09908ca5343ef7af1a8e51d9d8fb74fefb647c Mon Sep 17 00:00:00 2001
From: Llorx <dallorx@gmail.com>
Date: Sat, 2 Nov 2024 09:39:48 +0100
Subject: [PATCH] test_runner: exclude test files from code coverage by default

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

Fixes: https://github.com/nodejs/node/issues/53508
---
 doc/api/cli.md                                |  3 ++
 doc/api/test.md                               |  2 +
 lib/internal/test_runner/coverage.js          |  6 ++-
 lib/internal/test_runner/runner.js            |  3 +-
 .../test-runner-coverage-source-map.js        |  1 +
 .../test-runner-coverage-thresholds.js        | 12 +++++
 test/parallel/test-runner-coverage.js         | 53 +++++++++++++++----
 test/parallel/test-runner-run-coverage.mjs    | 45 ++++++++++++++--
 8 files changed, 106 insertions(+), 19 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index eeb7f25f4f82d9..a3c62d072ac0d0 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -2352,6 +2352,9 @@ This option may be specified multiple times to include 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.
 
+By default, the files being tested are excluded from code coverage. They can be explicitly
+included via this flag.
+
 ### `--test-coverage-lines=threshold`
 
 <!-- YAML
diff --git a/doc/api/test.md b/doc/api/test.md
index a6ca7531d503a2..cff48757f9590b 100644
--- a/doc/api/test.md
+++ b/doc/api/test.md
@@ -1359,6 +1359,8 @@ 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.
+    By default, the files being tested are excluded from code coverage. They can be explicitly
+	included via this parameter.
     **Default:** `undefined`.
   * `lineCoverage` {number} Require a minimum percent of covered lines. If code
     coverage does not reach the threshold specified, the process will exit with code `1`.
diff --git a/lib/internal/test_runner/coverage.js b/lib/internal/test_runner/coverage.js
index c27b34b9c762c4..e42e779479ef94 100644
--- a/lib/internal/test_runner/coverage.js
+++ b/lib/internal/test_runner/coverage.js
@@ -1,6 +1,7 @@
 'use strict';
 const {
   ArrayFrom,
+  ArrayPrototypeIncludes,
   ArrayPrototypeMap,
   ArrayPrototypePush,
   JSONParse,
@@ -463,6 +464,7 @@ class TestCoverage {
     const {
       coverageExcludeGlobs: excludeGlobs,
       coverageIncludeGlobs: includeGlobs,
+      coverageExcludeFiles: excludeFiles,
     } = this.options;
     // This check filters out files that match the exclude globs.
     if (excludeGlobs?.length > 0) {
@@ -481,8 +483,8 @@ class TestCoverage {
       return true;
     }
 
-    // This check filters out the node_modules/ directory, unless it is explicitly included.
-    return StringPrototypeIncludes(url, '/node_modules/');
+    // This check filters out the node_modules/ directory and the test files, unless they are explicitly included.
+    return StringPrototypeIncludes(url, '/node_modules/') || !excludeFiles || ArrayPrototypeIncludes(excludeFiles, absolutePath);
   }
 }
 
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 0ba40d7ba7f71f..7082b17779dc12 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -666,6 +666,7 @@ function run(options = kEmptyObject) {
   validateStringArray(execArgv, 'options.execArgv');
 
   const rootTestOptions = { __proto__: null, concurrency, timeout, signal };
+  let testFiles = files ?? createTestFileList(globPatterns, cwd);
   const globalOptions = {
     __proto__: null,
     // parseCommandLine() should not be used here. However, The existing run()
@@ -675,13 +676,13 @@ function run(options = kEmptyObject) {
     coverage,
     coverageExcludeGlobs,
     coverageIncludeGlobs,
+    coverageExcludeFiles: ArrayPrototypeMap(testFiles, (file) => resolve(cwd, file)),
     lineCoverage: lineCoverage,
     branchCoverage: branchCoverage,
     functionCoverage: functionCoverage,
     cwd,
   };
   const root = createTestTree(rootTestOptions, globalOptions);
-  let testFiles = files ?? createTestFileList(globPatterns, cwd);
 
   if (shard) {
     testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
diff --git a/test/parallel/test-runner-coverage-source-map.js b/test/parallel/test-runner-coverage-source-map.js
index 48807fb2d193b4..06c2b522571167 100644
--- a/test/parallel/test-runner-coverage-source-map.js
+++ b/test/parallel/test-runner-coverage-source-map.js
@@ -20,6 +20,7 @@ function generateReport(report) {
 
 const flags = [
   '--enable-source-maps',
+  '--no-warnings', '--test-coverage-include=**',
   '--test', '--experimental-test-coverage', '--test-reporter', 'tap',
 ];
 
diff --git a/test/parallel/test-runner-coverage-thresholds.js b/test/parallel/test-runner-coverage-thresholds.js
index c1b64cb06a83ff..ae8aa69bed454c 100644
--- a/test/parallel/test-runner-coverage-thresholds.js
+++ b/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,
     ]);
diff --git a/test/parallel/test-runner-coverage.js b/test/parallel/test-runner-coverage.js
index 5756f1d237605c..fd0893ea0cc780 100644
--- a/test/parallel/test-runner-coverage.js
+++ b/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');
 
diff --git a/test/parallel/test-runner-run-coverage.mjs b/test/parallel/test-runner-run-coverage.mjs
index 15fcfef5238843..3215d0ef79ba8f 100644
--- a/test/parallel/test-runner-run-coverage.mjs
+++ b/test/parallel/test-runner-run-coverage.mjs
@@ -106,7 +106,21 @@ describe('require(\'node:test\').run coverage settings', { concurrency: true },
       // eslint-disable-next-line no-unused-vars
       for await (const _ of stream);
     });
-
+    await it('should not include test files with complex paths', async () => {
+      const coverageFiles = [];
+      const stream = run({ files: [fixtures.path('no-folder', '..', 'test-runner', 'coverage.js')], coverage: true });
+      stream.on('test:fail', common.mustNotCall());
+      stream.on('test:pass', common.mustCall());
+      stream.on('test:coverage', msg => {
+        coverageFiles.push(...msg.summary.files.map(file => file.path));
+      });
+      // eslint-disable-next-line no-unused-vars
+      for await (const _ of stream);
+      assert.deepStrictEqual(coverageFiles.sort(), [
+        fixtures.path('test-runner', 'invalid-tap.js'),
+        fixtures.path('v8-coverage', 'throw.js')
+      ]);
+    });
     await it('should run with coverage and exclude by glob', async () => {
       const stream = run({ files, coverage: true, coverageExcludeGlobs: ['test/*/test-runner/invalid-tap.js'] });
       stream.on('test:fail', common.mustNotCall());
@@ -137,13 +151,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);
@@ -153,11 +167,12 @@ describe('require(\'node:test\').run coverage settings', { concurrency: true },
       for await (const _ of stream);
     });
 
-    await it('should run with coverage and fail when below line threshold', async () => {
+    await it('should run with coverage and coverageIncludeGlobs and fail when below thresholds', async () => {
       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 }) => {
@@ -172,6 +187,26 @@ describe('require(\'node:test\').run coverage settings', { concurrency: true },
       assert.strictEqual(process.exitCode, 1);
       process.exitCode = originalExitCode;
     });
+    await it('should run with coverage and fail when below thresholds', async () => {
+      const thresholdErrors = [];
+      const originalExitCode = process.exitCode;
+      assert.notStrictEqual(originalExitCode, 1);
+      const stream = run({ files, 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 }) => {
+        const match = message.match(/Error: \d{2}\.\d{2}% (line|branch|function) coverage does not meet threshold of 99%/);
+        if (match) {
+          thresholdErrors.push(match[1]);
+        }
+      });
+      // eslint-disable-next-line no-unused-vars
+      for await (const _ of stream);
+      assert.deepStrictEqual(thresholdErrors.sort(), ['branch', 'line']);
+      assert.strictEqual(process.exitCode, 1);
+      process.exitCode = originalExitCode;
+    });
   });
 });