test_runner: exclude test files from coverage by default

PR-URL: #56060
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>

Author: pmarchini

doc/api/cli.md

@@ -2270,6 +2270,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

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

lib/internal/fs/glob.js

@@ -650,7 +650,30 @@ class Glob {
   }
 }
 
+/**
+ * Check if a path matches a glob pattern
+ * @param {string} path the path to check
+ * @param {string} pattern the glob pattern to match
+ * @param {boolean} windows whether the path is on a Windows system, defaults to `isWindows`
+ * @returns {boolean}
+ */
+function matchGlobPattern(path, pattern, windows = isWindows) {
+  validateString(path, 'path');
+  validateString(pattern, 'pattern');
+  return lazyMinimatch().minimatch(path, pattern, {
+    kEmptyObject,
+    nocase: isMacOS || isWindows,
+    windowsPathsNoEscape: true,
+    nonegate: true,
+    nocomment: true,
+    optimizationLevel: 2,
+    platform: windows ? 'win32' : 'posix',
+    nocaseMagicOnly: true,
+  });
+}
+
 module.exports = {
   __proto__: null,
   Glob,
+  matchGlobPattern,
 };

lib/internal/test_runner/coverage.js

@@ -27,7 +27,7 @@ const {
 } = require('fs');
 const { setupCoverageHooks } = 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 {
@@ -36,6 +36,8 @@ const {
     ERR_SOURCE_MAP_MISSING_SOURCE,
   },
 } = require('internal/errors');
+const { matchGlobPattern } = require('internal/fs/glob');
+
 const kCoverageFileRegex = /^coverage-(\d+)-(\d{13})-(\d+)\.json$/;
 const kIgnoreRegex = /\/\* node:coverage ignore next (?<count>\d+ )?\*\//;
 const kLineEndingRegex = /\r?\n$/u;
@@ -464,19 +466,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 (
+          matchGlobPattern(relativePath, excludeGlobs[i]) ||
+          matchGlobPattern(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 (
+          matchGlobPattern(relativePath, includeGlobs[i]) ||
+          matchGlobPattern(absolutePath, includeGlobs[i])
+        ) return false;
       }
       return true;
     }

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');

lib/path.js

@@ -52,13 +52,12 @@ const {
 } = require('internal/validators');
 
 const {
-  getLazy,
   emitExperimentalWarning,
   isWindows,
-  isMacOS,
+  getLazy,
 } = require('internal/util');
 
-const lazyMinimatch = getLazy(() => require('internal/deps/minimatch/index'));
+const lazyMatchGlobPattern = getLazy(() => require('internal/fs/glob').matchGlobPattern);
 
 function isPathSeparator(code) {
   return code === CHAR_FORWARD_SLASH || code === CHAR_BACKWARD_SLASH;
@@ -164,22 +163,6 @@ function _format(sep, pathObject) {
   return dir === pathObject.root ? `${dir}${base}` : `${dir}${sep}${base}`;
 }
 
-function glob(path, pattern, windows) {
-  emitExperimentalWarning('glob');
-  validateString(path, 'path');
-  validateString(pattern, 'pattern');
-  return lazyMinimatch().minimatch(path, pattern, {
-    __proto__: null,
-    nocase: isMacOS || isWindows,
-    windowsPathsNoEscape: true,
-    nonegate: true,
-    nocomment: true,
-    optimizationLevel: 2,
-    platform: windows ? 'win32' : 'posix',
-    nocaseMagicOnly: true,
-  });
-}
-
 const win32 = {
   /**
    * path.resolve([from ...], to)
@@ -1140,7 +1123,8 @@ const win32 = {
   },
 
   matchesGlob(path, pattern) {
-    return glob(path, pattern, true);
+    emitExperimentalWarning('glob');
+    return lazyMatchGlobPattern()(path, pattern, true);
   },
 
   sep: '\\',
@@ -1616,7 +1600,8 @@ const posix = {
   },
 
   matchesGlob(path, pattern) {
-    return glob(path, pattern, false);
+    emitExperimentalWarning('glob');
+    return lazyMatchGlobPattern()(path, pattern, false);
   },
 
   sep: '/',

test/fixtures/test-runner/coverage-default-exclusion/file-test.js

@@ -0,0 +1,7 @@
+const test = require('node:test');
+const assert = require('node:assert');
+const { foo } = require('./logic-file');
+
+test('foo returns 1', () => {
+    assert.strictEqual(foo(), 1);
+});

test/fixtures/test-runner/coverage-default-exclusion/file.test.mjs

@@ -0,0 +1,7 @@
+import test from 'node:test';
+import assert from 'node:assert';
+import { foo } from './logic-file.js';
+
+test('foo returns 1', () => {
+    assert.strictEqual(foo(), 1);
+});

test/fixtures/test-runner/coverage-default-exclusion/file.test.ts

@@ -0,0 +1,7 @@
+import test from 'node:test';
+import assert from 'node:assert';
+import { foo } from './logic-file.js';
+
+test('foo returns 1', () => {
+    assert.strictEqual(foo(), 1);
+});

test/fixtures/test-runner/coverage-default-exclusion/logic-file.js

@@ -0,0 +1,9 @@
+function foo() {
+    return 1;
+}
+
+function bar() {
+    return 'bar';
+}
+
+module.exports = { foo, bar };