Skip to content

Commit 22e99dc

Browse files
authored
test_runner: match dotfiles in default coverage exclude
The default coverage exclude globs did not match dotfiles, so test files such as `test/.foo.test.js` were incorrectly included in coverage reports. Apply the `dot: true` minimatch option when matching the relative path so the default exclude patterns cover dotfiles, while keeping plain matching for the absolute path to avoid misinterpreting dot segments in the filesystem path (e.g. tmp dirs like `test/.tmp.0`). Fixes: #63397 Signed-off-by: semimikoh <ejffjeosms@gmail.com> PR-URL: #63401 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent a89fc17 commit 22e99dc

3 files changed

Lines changed: 59 additions & 31 deletions

File tree

lib/internal/test_runner/coverage.js

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ const kIgnoreRegex = /\/\* node:coverage ignore next (?<count>\d+ )?\*\//;
4747
const kLineEndingRegex = /\r?\n$/u;
4848
const kLineSplitRegex = /(?<=\r?\n)/u;
4949
const kStatusRegex = /\/\* node:coverage (?<status>enable|disable) \*\//;
50+
// Match dotfiles (e.g. `test/.foo.js`) when applying coverage globs so the
51+
// default exclude patterns cover them.
52+
const kMatchGlobPatternOptions = { __proto__: null, dot: true };
5053
const kTypeOnlyImportRegex = /^\s*import\s+type\b/u;
5154
const kTypeScriptSourceRegex = /\.(?:cts|mts|ts)$/u;
5255
const kSourceFileGlob = '**/*.{cjs,cts,js,mjs,mts,ts}';
@@ -63,6 +66,14 @@ function getStripTypeScriptTypesForCoverage() {
6366
return stripTypeScriptTypesForCoverage;
6467
}
6568

69+
function createCoverageMatcher(pattern) {
70+
return {
71+
__proto__: null,
72+
relative: createMatcher(pattern, kMatchGlobPatternOptions),
73+
absolute: createMatcher(pattern),
74+
};
75+
}
76+
6677
class CoverageLine {
6778
constructor(line, startOffset, src, length = src?.length) {
6879
const newlineLength = src == null ? 0 :
@@ -605,23 +616,28 @@ class TestCoverage {
605616
// TestCoverage instance, so compile each glob to a matcher once and reuse
606617
// it for every file. Building a fresh Minimatch per call (the previous
607618
// behavior) dominated the coverage report time, scaling with
608-
// files * globs.
619+
// files * globs. Each glob compiles to a matcher pair: `relative` enables
620+
// dot:true so globs match dotfiles within the project, while `absolute`
621+
// keeps the default behavior to avoid misinterpreting dot segments in the
622+
// absolute filesystem path (e.g. tmp dirs like `test/.tmp.0`).
609623
this.#excludeMatchers ??= ArrayPrototypeMap(
610-
this.options.coverageExcludeGlobs ?? [], (pattern) => createMatcher(pattern));
624+
this.options.coverageExcludeGlobs ?? [], createCoverageMatcher);
611625
this.#includeMatchers ??= ArrayPrototypeMap(
612-
this.options.coverageIncludeGlobs ?? [], (pattern) => createMatcher(pattern));
626+
this.options.coverageIncludeGlobs ?? [], createCoverageMatcher);
613627

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

620635
// This check filters out files that do not match the include globs.
621636
if (this.#includeMatchers.length > 0) {
622637
for (let i = 0; i < this.#includeMatchers.length; ++i) {
623638
const matcher = this.#includeMatchers[i];
624-
if (matcher.match(relativePath) || matcher.match(absolutePath)) return false;
639+
if (matcher.relative.match(relativePath) ||
640+
matcher.absolute.match(absolutePath)) return false;
625641
}
626642
return true;
627643
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
const test = require('node:test');
2+
const assert = require('node:assert');
3+
const { foo } = require('../logic-file.js');
4+
5+
test('foo returns 1 from a dotfile test', () => {
6+
assert.strictEqual(foo(), 1);
7+
});

test/parallel/test-runner-coverage-default-exclusion.mjs

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,16 @@ async function setupFixtures() {
1616
await cp(fixtureDir, tmpdir.path, { recursive: true });
1717
}
1818

19+
function assertDefaultExclusions(stdout) {
20+
assert.match(stdout, /# start of coverage report/);
21+
assert.doesNotMatch(stdout, /# file-test\.js\s+\|/);
22+
assert.doesNotMatch(stdout, /# file\.test\.mjs\s+\|/);
23+
assert.doesNotMatch(stdout, /# file\.test\.ts\s+\|/);
24+
assert.doesNotMatch(stdout, /# test\.cjs\s+\|/);
25+
assert.doesNotMatch(stdout, /#\s+not-matching-test-name\.js\s+\|/);
26+
assert.match(stdout, /# end of coverage report/);
27+
}
28+
1929
describe('test runner coverage default exclusion', skipIfNoInspector, () => {
2030
before(async () => {
2131
await setupFixtures();
@@ -58,18 +68,6 @@ describe('test runner coverage default exclusion', skipIfNoInspector, () => {
5868
});
5969

6070
it('should exclude test files from coverage by default', async () => {
61-
const report = [
62-
'# start of coverage report',
63-
'# --------------------------------------------------------------',
64-
'# file | line % | branch % | funcs % | uncovered lines',
65-
'# --------------------------------------------------------------',
66-
'# logic-file.js | 66.67 | 100.00 | 50.00 | 5-7',
67-
'# --------------------------------------------------------------',
68-
'# all files | 66.67 | 100.00 | 50.00 | ',
69-
'# --------------------------------------------------------------',
70-
'# end of coverage report',
71-
].join('\n');
72-
7371
const args = [
7472
'--no-experimental-strip-types',
7573
'--test',
@@ -82,23 +80,11 @@ describe('test runner coverage default exclusion', skipIfNoInspector, () => {
8280
});
8381

8482
assert.strictEqual(result.stderr.toString(), '');
85-
assert(result.stdout.toString().includes(report));
83+
assertDefaultExclusions(result.stdout.toString());
8684
assert.strictEqual(result.status, 0);
8785
});
8886

8987
it('should exclude ts test files', async () => {
90-
const report = [
91-
'# start of coverage report',
92-
'# --------------------------------------------------------------',
93-
'# file | line % | branch % | funcs % | uncovered lines',
94-
'# --------------------------------------------------------------',
95-
'# logic-file.js | 66.67 | 100.00 | 50.00 | 5-7',
96-
'# --------------------------------------------------------------',
97-
'# all files | 66.67 | 100.00 | 50.00 | ',
98-
'# --------------------------------------------------------------',
99-
'# end of coverage report',
100-
].join('\n');
101-
10288
const args = [
10389
'--test',
10490
'--experimental-test-coverage',
@@ -111,7 +97,26 @@ describe('test runner coverage default exclusion', skipIfNoInspector, () => {
11197
});
11298

11399
assert.strictEqual(result.stderr.toString(), '');
114-
assert(result.stdout.toString().includes(report));
100+
assertDefaultExclusions(result.stdout.toString());
101+
assert.strictEqual(result.status, 0);
102+
});
103+
104+
it('should exclude dotfile test files from coverage by default', async () => {
105+
const args = [
106+
'--no-experimental-strip-types',
107+
'--test',
108+
'--experimental-test-coverage',
109+
'--test-reporter=tap',
110+
'test/.dotfile.cjs',
111+
];
112+
const result = spawnSync(process.execPath, args, {
113+
env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
114+
cwd: tmpdir.path
115+
});
116+
117+
assert.strictEqual(result.stderr.toString(), '');
118+
assertDefaultExclusions(result.stdout.toString());
119+
assert.doesNotMatch(result.stdout.toString(), /#\s+\.dotfile\.cjs\s+\|/);
115120
assert.strictEqual(result.status, 0);
116121
});
117122
});

0 commit comments

Comments
 (0)