Skip to content

Commit cc09908

Browse files
LlorxLlorx
committed
test_runner: exclude test files from code coverage by default
Is not usual to test the code coverage of test files. Fixes: #53508
1 parent d37214b commit cc09908

File tree

8 files changed

+106
-19
lines changed

8 files changed

+106
-19
lines changed

‎doc/api/cli.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2352,6 +2352,9 @@ This option may be specified multiple times to include multiple glob patterns.
23522352
If both `--test-coverage-exclude` and `--test-coverage-include` are provided,
23532353
files must meet **both** criteria to be included in the coverage report.
23542354

2355+
By default, the files being tested are excluded from code coverage. They can be explicitly
2356+
included via this flag.
2357+
23552358
### `--test-coverage-lines=threshold`
23562359

23572360
<!-- YAML

‎doc/api/test.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1359,6 +1359,8 @@ changes:
13591359
This property is only applicable when `coverage` was set to `true`.
13601360
If both `coverageExcludeGlobs` and `coverageIncludeGlobs` are provided,
13611361
files must meet **both** criteria to be included in the coverage report.
1362+
By default, the files being tested are excluded from code coverage. They can be explicitly
1363+
included via this parameter.
13621364
**Default:** `undefined`.
13631365
* `lineCoverage` {number} Require a minimum percent of covered lines. If code
13641366
coverage does not reach the threshold specified, the process will exit with code `1`.

‎lib/internal/test_runner/coverage.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22
const {
33
ArrayFrom,
4+
ArrayPrototypeIncludes,
45
ArrayPrototypeMap,
56
ArrayPrototypePush,
67
JSONParse,
@@ -463,6 +464,7 @@ class TestCoverage {
463464
const {
464465
coverageExcludeGlobs: excludeGlobs,
465466
coverageIncludeGlobs: includeGlobs,
467+
coverageExcludeFiles: excludeFiles,
466468
} = this.options;
467469
// This check filters out files that match the exclude globs.
468470
if (excludeGlobs?.length > 0) {
@@ -481,8 +483,8 @@ class TestCoverage {
481483
return true;
482484
}
483485

484-
// This check filters out the node_modules/ directory, unless it is explicitly included.
485-
return StringPrototypeIncludes(url, '/node_modules/');
486+
// This check filters out the node_modules/ directory and the test files, unless they are explicitly included.
487+
return StringPrototypeIncludes(url, '/node_modules/') || !excludeFiles || ArrayPrototypeIncludes(excludeFiles, absolutePath);
486488
}
487489
}
488490

‎lib/internal/test_runner/runner.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,7 @@ function run(options = kEmptyObject) {
666666
validateStringArray(execArgv, 'options.execArgv');
667667

668668
const rootTestOptions = { __proto__: null, concurrency, timeout, signal };
669+
let testFiles = files ?? createTestFileList(globPatterns, cwd);
669670
const globalOptions = {
670671
__proto__: null,
671672
// parseCommandLine() should not be used here. However, The existing run()
@@ -675,13 +676,13 @@ function run(options = kEmptyObject) {
675676
coverage,
676677
coverageExcludeGlobs,
677678
coverageIncludeGlobs,
679+
coverageExcludeFiles: ArrayPrototypeMap(testFiles, (file) => resolve(cwd, file)),
678680
lineCoverage: lineCoverage,
679681
branchCoverage: branchCoverage,
680682
functionCoverage: functionCoverage,
681683
cwd,
682684
};
683685
const root = createTestTree(rootTestOptions, globalOptions);
684-
let testFiles = files ?? createTestFileList(globPatterns, cwd);
685686

686687
if (shard) {
687688
testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);

‎test/parallel/test-runner-coverage-source-map.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ function generateReport(report) {
2020

2121
const flags = [
2222
'--enable-source-maps',
23+
'--no-warnings', '--test-coverage-include=**',
2324
'--test', '--experimental-test-coverage', '--test-reporter', 'tap',
2425
];
2526

‎test/parallel/test-runner-coverage-thresholds.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ for (const coverage of coverages) {
6161
const result = spawnSync(process.execPath, [
6262
'--test',
6363
'--experimental-test-coverage',
64+
'--no-warnings',
65+
'--test-coverage-include=**',
6466
`${coverage.flag}=25`,
6567
'--test-reporter', 'tap',
6668
fixture,
@@ -77,6 +79,8 @@ for (const coverage of coverages) {
7779
const result = spawnSync(process.execPath, [
7880
'--test',
7981
'--experimental-test-coverage',
82+
'--no-warnings',
83+
'--test-coverage-include=**',
8084
`${coverage.flag}=25`,
8185
'--test-reporter', reporter,
8286
fixture,
@@ -92,6 +96,8 @@ for (const coverage of coverages) {
9296
const result = spawnSync(process.execPath, [
9397
'--test',
9498
'--experimental-test-coverage',
99+
'--no-warnings',
100+
'--test-coverage-include=**',
95101
`${coverage.flag}=99`,
96102
'--test-reporter', 'tap',
97103
fixture,
@@ -108,6 +114,8 @@ for (const coverage of coverages) {
108114
const result = spawnSync(process.execPath, [
109115
'--test',
110116
'--experimental-test-coverage',
117+
'--no-warnings',
118+
'--test-coverage-include=**',
111119
`${coverage.flag}=99`,
112120
'--test-reporter', reporter,
113121
fixture,
@@ -123,6 +131,8 @@ for (const coverage of coverages) {
123131
const result = spawnSync(process.execPath, [
124132
'--test',
125133
'--experimental-test-coverage',
134+
'--no-warnings',
135+
'--test-coverage-include=**',
126136
`${coverage.flag}=101`,
127137
fixture,
128138
]);
@@ -136,6 +146,8 @@ for (const coverage of coverages) {
136146
const result = spawnSync(process.execPath, [
137147
'--test',
138148
'--experimental-test-coverage',
149+
'--no-warnings',
150+
'--test-coverage-include=**',
139151
`${coverage.flag}=-1`,
140152
fixture,
141153
]);

‎test/parallel/test-runner-coverage.js

Lines changed: 42 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,9 @@ test('test spec coverage reporter', skipIfNoInspector, async (t) => {
150150
test('single process coverage is the same with --test', skipIfNoInspector, () => {
151151
const fixture = fixtures.path('test-runner', 'coverage.js');
152152
const args = [
153-
'--test', '--experimental-test-coverage', '--test-reporter', 'tap', fixture,
153+
'--test', '--experimental-test-coverage',
154+
'--no-warnings', '--test-coverage-include=**',
155+
'--test-reporter', 'tap', fixture,
154156
];
155157
const result = spawnSync(process.execPath, args);
156158
const report = getTapCoverageFixtureReport();
@@ -183,7 +185,7 @@ test('coverage is combined for multiple processes', skipIfNoInspector, () => {
183185

184186
const fixture = fixtures.path('v8-coverage', 'combined_coverage');
185187
const args = [
186-
'--test', '--experimental-test-coverage', '--test-reporter', 'tap',
188+
'--test', '--experimental-test-coverage', '--no-warnings', '--test-coverage-include=**', '--test-reporter', 'tap',
187189
];
188190
const result = spawnSync(process.execPath, args, {
189191
env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
@@ -236,7 +238,8 @@ test.skip('coverage works with isolation=none', skipIfNoInspector, () => {
236238
test('coverage reports on lines, functions, and branches', skipIfNoInspector, async (t) => {
237239
const fixture = fixtures.path('test-runner', 'coverage.js');
238240
const child = spawnSync(process.execPath,
239-
['--test', '--experimental-test-coverage', '--test-reporter',
241+
['--test', '--experimental-test-coverage',
242+
'--no-warnings', '--test-coverage-include=**', '--test-reporter',
240243
fixtures.fileURL('test-runner/custom_reporters/coverage.mjs'),
241244
fixture]);
242245
assert.strictEqual(child.stderr.toString(), '');
@@ -297,7 +300,6 @@ test('coverage with ESM hook - source irrelevant', skipIfNoInspector, () => {
297300
'# ------------------------------------------------------------------',
298301
'# hooks.mjs | 100.00 | 100.00 | 100.00 | ',
299302
'# register-hooks.js | 100.00 | 100.00 | 100.00 | ',
300-
'# virtual.js | 100.00 | 100.00 | 100.00 | ',
301303
'# ------------------------------------------------------------------',
302304
'# all files | 100.00 | 100.00 | 100.00 | ',
303305
'# ------------------------------------------------------------------',
@@ -327,7 +329,6 @@ test('coverage with ESM hook - source transpiled', skipIfNoInspector, () => {
327329
'# ------------------------------------------------------------------',
328330
'# hooks.mjs | 100.00 | 100.00 | 100.00 | ',
329331
'# register-hooks.js | 100.00 | 100.00 | 100.00 | ',
330-
'# sum.test.ts | 100.00 | 100.00 | 100.00 | ',
331332
'# sum.ts | 100.00 | 100.00 | 100.00 | ',
332333
'# ------------------------------------------------------------------',
333334
'# all files | 100.00 | 100.00 | 100.00 | ',
@@ -384,7 +385,38 @@ test('coverage with excluded files', skipIfNoInspector, () => {
384385
assert.strictEqual(result.status, 0);
385386
assert(!findCoverageFileForPid(result.pid));
386387
});
388+
test('coverage should not include test files by default', skipIfNoInspector, () => {
389+
const fixture = fixtures.path('test-runner', 'coverage.js');
390+
const args = [
391+
'--experimental-test-coverage', '--test-reporter', 'tap',
392+
fixture,
393+
];
394+
const result = spawnSync(process.execPath, args);
395+
const report = [
396+
'# start of coverage report',
397+
'# ---------------------------------------------------------------',
398+
'# file | line % | branch % | funcs % | uncovered lines',
399+
'# ---------------------------------------------------------------',
400+
'# test | | | | ',
401+
'# fixtures | | | | ',
402+
'# test-runner | | | | ',
403+
'# v8-coverage | | | | ',
404+
'# throw.js | 71.43 | 50.00 | 100.00 | 5-6',
405+
'# ---------------------------------------------------------------',
406+
'# all files | 78.13 | 40.00 | 60.00 | ',
407+
'# ---------------------------------------------------------------',
408+
'# end of coverage report',
409+
].join('\n');
387410

411+
412+
if (common.isWindows) {
413+
return report.replaceAll('/', '\\');
414+
}
415+
416+
assert(result.stdout.toString().includes(report));
417+
assert.strictEqual(result.status, 0);
418+
assert(!findCoverageFileForPid(result.pid));
419+
});
388420
test('coverage with included files', skipIfNoInspector, () => {
389421
const fixture = fixtures.path('test-runner', 'coverage.js');
390422
const args = [
@@ -458,18 +490,17 @@ test('coverage with included and excluded files', skipIfNoInspector, () => {
458490
test('correctly prints the coverage report of files contained in parent directories', skipIfNoInspector, () => {
459491
let report = [
460492
'# start of coverage report',
461-
'# --------------------------------------------------------------------------------------------',
493+
'# ------------------------------------------------------------------',
462494
'# file | line % | branch % | funcs % | uncovered lines',
463-
'# --------------------------------------------------------------------------------------------',
495+
'# ------------------------------------------------------------------',
464496
'# .. | | | | ',
465-
'# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72',
466497
'# invalid-tap.js | 100.00 | 100.00 | 100.00 | ',
467498
'# .. | | | | ',
468499
'# v8-coverage | | | | ',
469500
'# throw.js | 71.43 | 50.00 | 100.00 | 5-6',
470-
'# --------------------------------------------------------------------------------------------',
471-
'# all files | 78.35 | 43.75 | 60.00 | ',
472-
'# --------------------------------------------------------------------------------------------',
501+
'# ------------------------------------------------------------------',
502+
'# all files | 75.00 | 66.67 | 100.00 | ',
503+
'# ------------------------------------------------------------------',
473504
'# end of coverage report',
474505
].join('\n');
475506

‎test/parallel/test-runner-run-coverage.mjs

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,21 @@ describe('require(\'node:test\').run coverage settings', { concurrency: true },
106106
// eslint-disable-next-line no-unused-vars
107107
for await (const _ of stream);
108108
});
109-
109+
await it('should not include test files with complex paths', async () => {
110+
const coverageFiles = [];
111+
const stream = run({ files: [fixtures.path('no-folder', '..', 'test-runner', 'coverage.js')], coverage: true });
112+
stream.on('test:fail', common.mustNotCall());
113+
stream.on('test:pass', common.mustCall());
114+
stream.on('test:coverage', msg => {
115+
coverageFiles.push(...msg.summary.files.map(file => file.path));
116+
});
117+
// eslint-disable-next-line no-unused-vars
118+
for await (const _ of stream);
119+
assert.deepStrictEqual(coverageFiles.sort(), [
120+
fixtures.path('test-runner', 'invalid-tap.js'),
121+
fixtures.path('v8-coverage', 'throw.js')
122+
]);
123+
});
110124
await it('should run with coverage and exclude by glob', async () => {
111125
const stream = run({ files, coverage: true, coverageExcludeGlobs: ['test/*/test-runner/invalid-tap.js'] });
112126
stream.on('test:fail', common.mustNotCall());
@@ -137,13 +151,13 @@ describe('require(\'node:test\').run coverage settings', { concurrency: true },
137151

138152
await it('should run while including and excluding globs', async () => {
139153
const stream = run({
140-
files: [...files, fixtures.path('test-runner/invalid-tap.js')],
154+
files: files,
141155
coverage: true,
142156
coverageIncludeGlobs: ['test/fixtures/test-runner/*.js'],
143157
coverageExcludeGlobs: ['test/fixtures/test-runner/*-tap.js']
144158
});
145159
stream.on('test:fail', common.mustNotCall());
146-
stream.on('test:pass', common.mustCall(2));
160+
stream.on('test:pass', common.mustCall(1));
147161
stream.on('test:coverage', common.mustCall(({ summary: { files } }) => {
148162
const filesPaths = files.map(({ path }) => path);
149163
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 },
153167
for await (const _ of stream);
154168
});
155169

156-
await it('should run with coverage and fail when below line threshold', async () => {
170+
await it('should run with coverage and coverageIncludeGlobs and fail when below thresholds', async () => {
157171
const thresholdErrors = [];
158172
const originalExitCode = process.exitCode;
159173
assert.notStrictEqual(originalExitCode, 1);
160-
const stream = run({ files, coverage: true, lineCoverage: 99, branchCoverage: 99, functionCoverage: 99 });
174+
const stream = run({ files, coverageIncludeGlobs: '**', coverage: true,
175+
lineCoverage: 99, branchCoverage: 99, functionCoverage: 99 });
161176
stream.on('test:fail', common.mustNotCall());
162177
stream.on('test:pass', common.mustCall(1));
163178
stream.on('test:diagnostic', ({ message }) => {
@@ -172,6 +187,26 @@ describe('require(\'node:test\').run coverage settings', { concurrency: true },
172187
assert.strictEqual(process.exitCode, 1);
173188
process.exitCode = originalExitCode;
174189
});
190+
await it('should run with coverage and fail when below thresholds', async () => {
191+
const thresholdErrors = [];
192+
const originalExitCode = process.exitCode;
193+
assert.notStrictEqual(originalExitCode, 1);
194+
const stream = run({ files, coverage: true,
195+
lineCoverage: 99, branchCoverage: 99, functionCoverage: 99 });
196+
stream.on('test:fail', common.mustNotCall());
197+
stream.on('test:pass', common.mustCall(1));
198+
stream.on('test:diagnostic', ({ message }) => {
199+
const match = message.match(/Error: \d{2}\.\d{2}% (line|branch|function) coverage does not meet threshold of 99%/);
200+
if (match) {
201+
thresholdErrors.push(match[1]);
202+
}
203+
});
204+
// eslint-disable-next-line no-unused-vars
205+
for await (const _ of stream);
206+
assert.deepStrictEqual(thresholdErrors.sort(), ['branch', 'line']);
207+
assert.strictEqual(process.exitCode, 1);
208+
process.exitCode = originalExitCode;
209+
});
175210
});
176211
});
177212

0 commit comments

Comments
 (0)