Skip to content

test_runner: match dotfiles in default coverage exclude #63401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
semimikoh wants to merge 1 commit into
base: main
Choose a base branch
from
+59 −31
Open

test_runner: match dotfiles in default coverage exclude #63401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions lib/internal/test_runner/coverage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ const kIgnoreRegex = /\/\* node:coverage ignore next (?<count>\d+ )?\*\//;
const kLineEndingRegex = /\r?\n$/u;
const kLineSplitRegex = /(?<=\r?\n)/u;
const kStatusRegex = /\/\* node:coverage (?<status>enable|disable) \*\//;
// Match dotfiles (e.g. `test/.foo.js`) when applying coverage globs so the
// default exclude patterns cover them.
const kMatchGlobPatternOptions = { __proto__: null, dot: true };
const kTypeOnlyImportRegex = /^\s*import\s+type\b/u;
const kTypeScriptSourceRegex = /\.(?:cts|mts|ts)$/u;

Expand All @@ -61,6 +64,14 @@ function getStripTypeScriptTypesForCoverage() {
return stripTypeScriptTypesForCoverage;
}

function createCoverageMatcher(pattern) {
return {
__proto__: null,
relative: createMatcher(pattern, kMatchGlobPatternOptions),
absolute: createMatcher(pattern),
};
}
Comment on lines +67 to +73

@avivkeller avivkeller Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we pass the same options and thus and use the same matcher for both or no?

@semimikoh semimikoh Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional — relative needs dot: true so the default exclude globs match project dotfiles like test/.foo.test.js (the point of this PR), but absolute deliberately leaves it off. Enabling dot: true there could make a glob unintentionally match a dot segment that shows up in the absolute path for unrelated reasons (e.g. an OS temp dir like test/.tmp.0), which isn't something the user's pattern was written to target.

@avivkeller avivkeller Aug 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't they be on par? As in, if we are changing this to include dots locally, wouldn't it make sense to also include them globally?

@semimikoh semimikoh Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite symmetric on purpose — I actually tested this by flipping absolute to use dot: true too and running it against the exact test/**/* default pattern.

Node's own test runner puts NODE_TEST_TMPDIR under a dot-prefixed directory (test/.tmp.<n>/, see test/common/tmpdir.js). With dot: true on absolute, ** is allowed to traverse into .tmp.<n>, so the default test/**/* exclude pattern ends up matching the absolute path of any file running under that tmp dir — e.g. .../test/.tmp.0/logic-file.js matches test/**/*.js, even though logic-file.js is a normal source file that should stay covered.

absolute (dot:false, current) match: false   // logic-file.js correctly stays covered
absolute (dot:true)          match: true    // logic-file.js gets wrongly excluded

Since basically every Node test that uses a tmpdir runs under test/.tmp.N/, making absolute symmetric with dot: true would cause widespread false-exclusions whenever coverage runs inside the Node source tree itself. relative only needs dot: true for * to match a leading dot in the basename (e.g. .foo.test.js); it doesn't need ** to traverse arbitrary dot directories the way absolute would.


class CoverageLine {
constructor(line, startOffset, src, length = src?.length) {
const newlineLength = src == null ? 0 :
Expand Down Expand Up @@ -557,23 +568,28 @@ class TestCoverage {
// TestCoverage instance, so compile each glob to a matcher once and reuse
// it for every file. Building a fresh Minimatch per call (the previous
// behavior) dominated the coverage report time, scaling with
// files * globs.
// files * globs. Each glob compiles to a matcher pair: `relative` enables
// dot:true so globs match dotfiles within the project, while `absolute`
// keeps the default behavior to avoid misinterpreting dot segments in the
// absolute filesystem path (e.g. tmp dirs like `test/.tmp.0`).
this.#excludeMatchers ??= ArrayPrototypeMap(
this.options.coverageExcludeGlobs ?? [], (pattern) => createMatcher(pattern));
this.options.coverageExcludeGlobs ?? [], createCoverageMatcher);
this.#includeMatchers ??= ArrayPrototypeMap(
this.options.coverageIncludeGlobs ?? [], (pattern) => createMatcher(pattern));
this.options.coverageIncludeGlobs ?? [], createCoverageMatcher);

// This check filters out files that match the exclude globs.
for (let i = 0; i < this.#excludeMatchers.length; ++i) {
const matcher = this.#excludeMatchers[i];
if (matcher.match(relativePath) || matcher.match(absolutePath)) return true;
if (matcher.relative.match(relativePath) ||
matcher.absolute.match(absolutePath)) return true;
}

// This check filters out files that do not match the include globs.
if (this.#includeMatchers.length > 0) {
for (let i = 0; i < this.#includeMatchers.length; ++i) {
const matcher = this.#includeMatchers[i];
if (matcher.match(relativePath) || matcher.match(absolutePath)) return false;
if (matcher.relative.match(relativePath) ||
matcher.absolute.match(absolutePath)) return false;
}
return true;
}
Expand Down
7 changes: 7 additions & 0 deletions test/fixtures/test-runner/coverage-default-exclusion/test/.dotfile.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const test = require('node:test');
const assert = require('node:assert');
const { foo } = require('../logic-file.js');

test('foo returns 1 from a dotfile test', () => {
assert.strictEqual(foo(), 1);
});
57 changes: 31 additions & 26 deletions test/parallel/test-runner-coverage-default-exclusion.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ async function setupFixtures() {
await cp(fixtureDir, tmpdir.path, { recursive: true });
}

function assertDefaultExclusions(stdout) {
assert.match(stdout, /# start of coverage report/);
assert.doesNotMatch(stdout, /# file-test\.js\s+\|/);
assert.doesNotMatch(stdout, /# file\.test\.mjs\s+\|/);
assert.doesNotMatch(stdout, /# file\.test\.ts\s+\|/);
assert.doesNotMatch(stdout, /# test\.cjs\s+\|/);
assert.doesNotMatch(stdout, /#\s+not-matching-test-name\.js\s+\|/);
assert.match(stdout, /# end of coverage report/);
}

describe('test runner coverage default exclusion', skipIfNoInspector, () => {
before(async () => {
await setupFixtures();
Expand Down Expand Up @@ -58,18 +68,6 @@ describe('test runner coverage default exclusion', skipIfNoInspector, () => {
});

it('should exclude test files from coverage by default', async () => {
const report = [
'# start of coverage report',
'# --------------------------------------------------------------',
'# file | line % | branch % | funcs % | uncovered lines',
'# --------------------------------------------------------------',
'# logic-file.js | 66.67 | 100.00 | 50.00 | 5-7',
'# --------------------------------------------------------------',
'# all files | 66.67 | 100.00 | 50.00 | ',
'# --------------------------------------------------------------',
'# end of coverage report',
].join('\n');

const args = [
'--no-experimental-strip-types',
'--test',
Expand All @@ -82,23 +80,11 @@ describe('test runner coverage default exclusion', skipIfNoInspector, () => {
});

assert.strictEqual(result.stderr.toString(), '');
assert(result.stdout.toString().includes(report));
assertDefaultExclusions(result.stdout.toString());
assert.strictEqual(result.status, 0);
});

it('should exclude ts test files', async () => {
const report = [
'# start of coverage report',
'# --------------------------------------------------------------',
'# file | line % | branch % | funcs % | uncovered lines',
'# --------------------------------------------------------------',
'# logic-file.js | 66.67 | 100.00 | 50.00 | 5-7',
'# --------------------------------------------------------------',
'# all files | 66.67 | 100.00 | 50.00 | ',
'# --------------------------------------------------------------',
'# end of coverage report',
].join('\n');

const args = [
'--test',
'--experimental-test-coverage',
Expand All @@ -111,7 +97,26 @@ describe('test runner coverage default exclusion', skipIfNoInspector, () => {
});

assert.strictEqual(result.stderr.toString(), '');
assert(result.stdout.toString().includes(report));
assertDefaultExclusions(result.stdout.toString());
assert.strictEqual(result.status, 0);
});

it('should exclude dotfile test files from coverage by default', async () => {
const args = [
'--no-experimental-strip-types',
'--test',
'--experimental-test-coverage',
'--test-reporter=tap',
'test/.dotfile.cjs',
];
const result = spawnSync(process.execPath, args, {
env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
cwd: tmpdir.path
});

assert.strictEqual(result.stderr.toString(), '');
assertDefaultExclusions(result.stdout.toString());
assert.doesNotMatch(result.stdout.toString(), /#\s+\.dotfile\.cjs\s+\|/);
assert.strictEqual(result.status, 0);
});
});
Loading