test_runner: exclude test files from coverage by default

nodejs · Dec 8, 2024 · 4f5885e · 4f5885e
1 parent b17a1fb
commit 4f5885e
Show file tree

Hide file tree

Showing 9 changed files with 157 additions and 35 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -2291,6 +2291,9 @@ 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.
 
+By default all the matching test files are excluded from the coverage report.
+Specifying this option will override the default behavior.
+
 ### `--test-coverage-functions=threshold`
 
 <!-- YAML

diff --git a/doc/api/test.md b/doc/api/test.md
@@ -476,8 +476,10 @@ all tests have completed. If the [`NODE_V8_COVERAGE`][] environment variable is
 used to specify a code coverage directory, the generated V8 coverage files are
 written to that directory. Node.js core modules and files within
 `node_modules/` directories are, by default, not included in the coverage report.
-However, they can be explicitly included via the [`--test-coverage-include`][] flag. If
-coverage is enabled, the coverage report is sent to any [test reporters][] via
+However, they can be explicitly included via the [`--test-coverage-include`][] flag.
+By default all the matching test files are excluded from the coverage report.
+Exclusions can be overridden by using the [`--test-coverage-exclude`][] flag.
+If coverage is enabled, the coverage report is sent to any [test reporters][] via
 the `'test:coverage'` event.
 
 Coverage can be disabled on a series of lines using the following
@@ -3594,6 +3596,7 @@ Can be used to abort test subtasks when the test has been aborted.
 [`--experimental-test-module-mocks`]: cli.md#--experimental-test-module-mocks
 [`--import`]: cli.md#--importmodule
 [`--test-concurrency`]: cli.md#--test-concurrency
+[`--test-coverage-exclude`]: cli.md#--test-coverage-exclude
 [`--test-coverage-include`]: cli.md#--test-coverage-include
 [`--test-name-pattern`]: cli.md#--test-name-pattern
 [`--test-only`]: cli.md#--test-only

diff --git a/lib/internal/test_runner/coverage.js b/lib/internal/test_runner/coverage.js
@@ -25,9 +25,9 @@ const {
   readFileSync,
   rmSync,
 } = require('fs');
-const { setupCoverageHooks } = require('internal/util');
+const { setupCoverageHooks, isWindows, isMacOS } = require('internal/util');
 const { tmpdir } = require('os');
-const { join, resolve, relative, matchesGlob } = require('path');
+const { join, resolve, relative } = require('path');
 const { fileURLToPath } = require('internal/url');
 const { kMappings, SourceMap } = require('internal/source_map/source_map');
 const {
@@ -42,6 +42,25 @@ const kLineEndingRegex = /\r?\n$/u;
 const kLineSplitRegex = /(?<=\r?\n)/u;
 const kStatusRegex = /\/\* node:coverage (?<status>enable|disable) \*\//;
 
+let minimatch;
+function lazyMinimatch() {
+  minimatch ??= require('internal/deps/minimatch/index');
+  return minimatch;
+}
+
+function glob(path, pattern, windows) {
+  return lazyMinimatch().minimatch(path, pattern, {
+    __proto__: null,
+    nocase: isMacOS || isWindows,
+    windowsPathsNoEscape: true,
+    nonegate: true,
+    nocomment: true,
+    optimizationLevel: 2,
+    platform: windows ? 'win32' : 'posix',
+    nocaseMagicOnly: true,
+  });
+}
+
 class CoverageLine {
   constructor(line, startOffset, src, length = src?.length) {
     const newlineLength = src == null ? 0 :
@@ -464,19 +483,24 @@ class TestCoverage {
       coverageExcludeGlobs: excludeGlobs,
       coverageIncludeGlobs: includeGlobs,
     } = this.options;
+
     // This check filters out files that match the exclude globs.
     if (excludeGlobs?.length > 0) {
       for (let i = 0; i < excludeGlobs.length; ++i) {
-        if (matchesGlob(relativePath, excludeGlobs[i]) ||
-            matchesGlob(absolutePath, excludeGlobs[i])) return true;
+        if (
+          glob(relativePath, excludeGlobs[i]) ||
+          glob(absolutePath, excludeGlobs[i])
+        ) return true;
       }
     }
 
     // This check filters out files that do not match the include globs.
     if (includeGlobs?.length > 0) {
       for (let i = 0; i < includeGlobs.length; ++i) {
-        if (matchesGlob(relativePath, includeGlobs[i]) ||
-            matchesGlob(absolutePath, includeGlobs[i])) return false;
+        if (
+          glob(relativePath, includeGlobs[i]) ||
+          glob(absolutePath, includeGlobs[i])
+        ) return false;
       }
       return true;
     }

diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
@@ -287,6 +287,11 @@ function parseCommandLine() {
 
   if (coverage) {
     coverageExcludeGlobs = getOptionValue('--test-coverage-exclude');
+    if (!coverageExcludeGlobs || coverageExcludeGlobs.length === 0) {
+      // TODO(pmarchini): this default should follow something similar to c8 defaults
+      // Default exclusions should be also exported to be used by other tools / users
+      coverageExcludeGlobs = [kDefaultPattern];
+    }
     coverageIncludeGlobs = getOptionValue('--test-coverage-include');
 
     branchCoverage = getOptionValue('--test-coverage-branches');

diff --git a/test/fixtures/test-runner/output/lcov_reporter.js b/test/fixtures/test-runner/output/lcov_reporter.js
@@ -4,4 +4,13 @@ const fixtures = require('../../../common/fixtures');
 const spawn = require('node:child_process').spawn;
 
 spawn(process.execPath,
-      ['--no-warnings', '--experimental-test-coverage', '--test-reporter', 'lcov', fixtures.path('test-runner/output/output.js')], { stdio: 'inherit' });
+      [
+            '--no-warnings',
+            '--experimental-test-coverage',
+            '--test-coverage-exclude=!test/**',
+            '--test-reporter',
+            'lcov',
+            fixtures.path('test-runner/output/output.js')
+      ],
+      { stdio: 'inherit' }
+);
diff --git a/test/parallel/test-runner-coverage-source-map.js b/test/parallel/test-runner-coverage-source-map.js
@@ -20,7 +20,11 @@ function generateReport(report) {
 
 const flags = [
   '--enable-source-maps',
-  '--test', '--experimental-test-coverage', '--test-reporter', 'tap',
+  '--test',
+  '--experimental-test-coverage',
+  '--test-coverage-exclude=!test/**',
+  '--test-reporter',
+  'tap',
 ];
 
 describe('Coverage with source maps', async () => {

diff --git a/test/parallel/test-runner-coverage-thresholds.js b/test/parallel/test-runner-coverage-thresholds.js
@@ -61,6 +61,7 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--test-coverage-exclude=!test/**',
       `${coverage.flag}=25`,
       '--test-reporter', 'tap',
       fixture,
@@ -77,6 +78,7 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--test-coverage-exclude=!test/**',
       `${coverage.flag}=25`,
       '--test-reporter', reporter,
       fixture,
@@ -92,6 +94,7 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--test-coverage-exclude=!test/**',
       `${coverage.flag}=99`,
       '--test-reporter', 'tap',
       fixture,
@@ -108,6 +111,7 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--test-coverage-exclude=!test/**',
       `${coverage.flag}=99`,
       '--test-reporter', reporter,
       fixture,
@@ -123,6 +127,7 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--test-coverage-exclude=!test/**',
       `${coverage.flag}=101`,
       fixture,
     ]);
@@ -136,6 +141,7 @@ for (const coverage of coverages) {
     const result = spawnSync(process.execPath, [
       '--test',
       '--experimental-test-coverage',
+      '--test-coverage-exclude=!test/**',
       `${coverage.flag}=-1`,
       fixture,
     ]);

diff --git a/test/parallel/test-runner-coverage.js b/test/parallel/test-runner-coverage.js
@@ -84,7 +84,11 @@ test('test coverage report', async (t) => {
     }
 
     const fixture = fixtures.path('test-runner', 'coverage.js');
-    const args = ['--experimental-test-coverage', fixture];
+    const args = [
+      '--experimental-test-coverage',
+      '--test-coverage-exclude=!test/**',
+      fixture,
+    ];
     const result = spawnSync(process.execPath, args);
 
     assert(!result.stdout.toString().includes('# start of coverage report'));
@@ -97,7 +101,13 @@ test('test coverage report', 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) => {
     const fixture = fixtures.path('test-runner', 'coverage.js');
-    const args = ['--experimental-test-coverage', '--test-reporter', 'tap', fixture];
+    const args = [
+      '--experimental-test-coverage',
+      '--test-coverage-exclude=!test/**',
+      '--test-reporter',
+      'tap',
+      fixture,
+    ];
     const options = { env: { ...process.env, NODE_V8_COVERAGE: tmpdir.path } };
     const result = spawnSync(process.execPath, args, options);
     const report = getTapCoverageFixtureReport();
@@ -109,7 +119,13 @@ test('test tap coverage reporter', skipIfNoInspector, async (t) => {
 
   await t.test('coverage is reported without NODE_V8_COVERAGE present', (t) => {
     const fixture = fixtures.path('test-runner', 'coverage.js');
-    const args = ['--experimental-test-coverage', '--test-reporter', 'tap', fixture];
+    const args = [
+      '--experimental-test-coverage',
+      '--test-coverage-exclude=!test/**',
+      '--test-reporter',
+      'tap',
+      fixture,
+    ];
     const result = spawnSync(process.execPath, args);
     const report = getTapCoverageFixtureReport();
 
@@ -123,7 +139,12 @@ test('test tap coverage reporter', skipIfNoInspector, 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) => {
     const fixture = fixtures.path('test-runner', 'coverage.js');
-    const args = ['--experimental-test-coverage', '--test-reporter', 'spec', fixture];
+    const args = [
+      '--experimental-test-coverage',
+      '--test-coverage-exclude=!test/**',
+      '--test-reporter',
+      'spec',
+      fixture];
     const options = { env: { ...process.env, NODE_V8_COVERAGE: tmpdir.path } };
     const result = spawnSync(process.execPath, args, options);
     const report = getSpecCoverageFixtureReport();
@@ -136,7 +157,12 @@ test('test spec coverage reporter', skipIfNoInspector, async (t) => {
 
   await t.test('coverage is reported without NODE_V8_COVERAGE present', (t) => {
     const fixture = fixtures.path('test-runner', 'coverage.js');
-    const args = ['--experimental-test-coverage', '--test-reporter', 'spec', fixture];
+    const args = [
+      '--experimental-test-coverage',
+      '--test-coverage-exclude=!test/**',
+      '--test-reporter',
+      'spec',
+      fixture];
     const result = spawnSync(process.execPath, args);
     const report = getSpecCoverageFixtureReport();
 
@@ -150,7 +176,12 @@ 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',
+    '--test-coverage-exclude=!test/**',
+    '--test-reporter',
+    'tap',
+    fixture,
   ];
   const result = spawnSync(process.execPath, args);
   const report = getTapCoverageFixtureReport();
@@ -183,7 +214,11 @@ 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',
+    '--test-coverage-exclude=!test/**',
+    '--test-reporter',
+    'tap',
   ];
   const result = spawnSync(process.execPath, args, {
     env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
@@ -221,7 +256,11 @@ test.skip('coverage works with isolation=none', skipIfNoInspector, () => {
 
   const fixture = fixtures.path('v8-coverage', 'combined_coverage');
   const args = [
-    '--test', '--experimental-test-coverage', '--test-reporter', 'tap', '--experimental-test-isolation=none',
+    '--test',
+    '--experimental-test-coverage',
+    '--test-reporter',
+    'tap',
+    '--experimental-test-isolation=none',
   ];
   const result = spawnSync(process.execPath, args, {
     env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
@@ -236,9 +275,14 @@ 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',
-                           fixtures.fileURL('test-runner/custom_reporters/coverage.mjs'),
-                           fixture]);
+                          [
+                            '--test',
+                            '--experimental-test-coverage',
+                            '--test-coverage-exclude=!test/**',
+                            '--test-reporter',
+                            fixtures.fileURL('test-runner/custom_reporters/coverage.mjs'),
+                            fixture,
+                          ]);
   assert.strictEqual(child.stderr.toString(), '');
   const stdout = child.stdout.toString();
   const coverage = JSON.parse(stdout);
@@ -310,7 +354,14 @@ test('coverage with ESM hook - source irrelevant', skipIfNoInspector, () => {
 
   const fixture = fixtures.path('test-runner', 'coverage-loader');
   const args = [
-    '--import', './register-hooks.js', '--test', '--experimental-test-coverage', '--test-reporter', 'tap', 'virtual.js',
+    '--import',
+    './register-hooks.js',
+    '--test',
+    '--experimental-test-coverage',
+    '--test-coverage-exclude=!test/**',
+    '--test-reporter',
+    'tap',
+    'virtual.js',
   ];
   const result = spawnSync(process.execPath, args, { cwd: fixture });
 
@@ -341,7 +392,10 @@ test('coverage with ESM hook - source transpiled', skipIfNoInspector, () => {
 
   const fixture = fixtures.path('test-runner', 'coverage-loader');
   const args = [
-    '--import', './register-hooks.js', '--test', '--experimental-test-coverage',
+    '--import', './register-hooks.js',
+    '--test',
+    '--experimental-test-coverage',
+    '--test-coverage-exclude=!test/**',
     '--test-reporter', 'tap', 'sum.test.ts',
   ];
   const result = spawnSync(process.execPath, args, { cwd: fixture });
@@ -356,6 +410,7 @@ test('coverage with excluded files', skipIfNoInspector, () => {
   const args = [
     '--experimental-test-coverage', '--test-reporter', 'tap',
     '--test-coverage-exclude=test/*/test-runner/invalid-tap.js',
+    '--test-coverage-exclude=!test/**',
     fixture];
   const result = spawnSync(process.execPath, args);
   const report = [
@@ -391,6 +446,7 @@ test('coverage with included files', skipIfNoInspector, () => {
     '--experimental-test-coverage', '--test-reporter', 'tap',
     '--test-coverage-include=test/fixtures/test-runner/coverage.js',
     '--test-coverage-include=test/fixtures/v8-coverage/throw.js',
+    '--test-coverage-exclude=!test/**',
     fixture,
   ];
   const result = spawnSync(process.execPath, args);
@@ -478,7 +534,12 @@ test('correctly prints the coverage report of files contained in parent director
   }
   const fixture = fixtures.path('test-runner', 'coverage.js');
   const args = [
-    '--test', '--experimental-test-coverage', '--test-reporter', 'tap', fixture,
+    '--test',
+    '--experimental-test-coverage',
+    '--test-coverage-exclude=!test/**',
+    '--test-reporter',
+    'tap',
+    fixture,
   ];
   const result = spawnSync(process.execPath, args, {
     env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },