Skip to content

Commit 2df9e6e

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 2df9e6e

File tree

9 files changed

+129
-25
lines changed

9 files changed

+129
-25
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+
testFiles,
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/') || ArrayPrototypeIncludes(testFiles, absolutePath);
486488
}
487489
}
488490

‎lib/internal/test_runner/harness.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const {
1313
createHook,
1414
executionAsyncId,
1515
} = require('async_hooks');
16-
const { relative } = require('path');
16+
const { relative, resolve } = require('path');
1717
const {
1818
codes: {
1919
ERR_TEST_FAILURE,
@@ -261,6 +261,7 @@ function lazyBootstrapRoot() {
261261
};
262262
const globalOptions = parseCommandLine();
263263
globalOptions.cwd = process.cwd();
264+
globalOptions.testFiles = entryFile ? [resolve(globalOptions.cwd, entryFile)] : [];
264265
createTestTree(rootTestOptions, globalOptions);
265266
globalRoot.reporter.on('test:summary', (data) => {
266267
if (!data.success) {

‎lib/internal/test_runner/runner.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,8 @@ 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);
670+
const absoluteTestFiles = ArrayPrototypeMap(testFiles, (file) => resolve(cwd, file));
669671
const globalOptions = {
670672
__proto__: null,
671673
// parseCommandLine() should not be used here. However, The existing run()
@@ -675,13 +677,13 @@ function run(options = kEmptyObject) {
675677
coverage,
676678
coverageExcludeGlobs,
677679
coverageIncludeGlobs,
680+
testFiles: absoluteTestFiles,
678681
lineCoverage: lineCoverage,
679682
branchCoverage: branchCoverage,
680683
functionCoverage: functionCoverage,
681684
cwd,
682685
};
683686
const root = createTestTree(rootTestOptions, globalOptions);
684-
let testFiles = files ?? createTestFileList(globPatterns, cwd);
685687

686688
if (shard) {
687689
testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
@@ -733,7 +735,6 @@ function run(options = kEmptyObject) {
733735
};
734736
} else if (isolation === 'none') {
735737
if (watch) {
736-
const absoluteTestFiles = ArrayPrototypeMap(testFiles, (file) => (isAbsolute(file) ? file : resolve(cwd, file)));
737738
filesWatcher = watchFiles(absoluteTestFiles, opts);
738739
runFiles = async () => {
739740
root.harness.bootstrapPromise = null;

‎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: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,11 @@ test('test coverage report', async (t) => {
9797
test('test tap coverage reporter', skipIfNoInspector, async (t) => {
9898
await t.test('coverage is reported and dumped to NODE_V8_COVERAGE if present', (t) => {
9999
const fixture = fixtures.path('test-runner', 'coverage.js');
100-
const args = ['--experimental-test-coverage', '--test-reporter', 'tap', fixture];
100+
const args = [
101+
'--experimental-test-coverage',
102+
'--no-warnings', '--test-coverage-include=**',
103+
'--test-reporter', 'tap', fixture,
104+
];
101105
const options = { env: { ...process.env, NODE_V8_COVERAGE: tmpdir.path } };
102106
const result = spawnSync(process.execPath, args, options);
103107
const report = getTapCoverageFixtureReport();
@@ -109,7 +113,11 @@ test('test tap coverage reporter', skipIfNoInspector, async (t) => {
109113

110114
await t.test('coverage is reported without NODE_V8_COVERAGE present', (t) => {
111115
const fixture = fixtures.path('test-runner', 'coverage.js');
112-
const args = ['--experimental-test-coverage', '--test-reporter', 'tap', fixture];
116+
const args = [
117+
'--experimental-test-coverage',
118+
'--no-warnings', '--test-coverage-include=**',
119+
'--test-reporter', 'tap', fixture,
120+
];
113121
const result = spawnSync(process.execPath, args);
114122
const report = getTapCoverageFixtureReport();
115123

@@ -123,7 +131,11 @@ test('test tap coverage reporter', skipIfNoInspector, async (t) => {
123131
test('test spec coverage reporter', skipIfNoInspector, async (t) => {
124132
await t.test('coverage is reported and dumped to NODE_V8_COVERAGE if present', (t) => {
125133
const fixture = fixtures.path('test-runner', 'coverage.js');
126-
const args = ['--experimental-test-coverage', '--test-reporter', 'spec', fixture];
134+
const args = [
135+
'--experimental-test-coverage',
136+
'--no-warnings', '--test-coverage-include=**',
137+
'--test-reporter', 'spec', fixture,
138+
];
127139
const options = { env: { ...process.env, NODE_V8_COVERAGE: tmpdir.path } };
128140
const result = spawnSync(process.execPath, args, options);
129141
const report = getSpecCoverageFixtureReport();
@@ -136,7 +148,11 @@ test('test spec coverage reporter', skipIfNoInspector, async (t) => {
136148

137149
await t.test('coverage is reported without NODE_V8_COVERAGE present', (t) => {
138150
const fixture = fixtures.path('test-runner', 'coverage.js');
139-
const args = ['--experimental-test-coverage', '--test-reporter', 'spec', fixture];
151+
const args = [
152+
'--experimental-test-coverage',
153+
'--no-warnings', '--test-coverage-include=**',
154+
'--test-reporter', 'spec', fixture,
155+
];
140156
const result = spawnSync(process.execPath, args);
141157
const report = getSpecCoverageFixtureReport();
142158

@@ -150,7 +166,9 @@ test('test spec coverage reporter', skipIfNoInspector, async (t) => {
150166
test('single process coverage is the same with --test', skipIfNoInspector, () => {
151167
const fixture = fixtures.path('test-runner', 'coverage.js');
152168
const args = [
153-
'--test', '--experimental-test-coverage', '--test-reporter', 'tap', fixture,
169+
'--test', '--experimental-test-coverage',
170+
'--no-warnings', '--test-coverage-include=**',
171+
'--test-reporter', 'tap', fixture,
154172
];
155173
const result = spawnSync(process.execPath, args);
156174
const report = getTapCoverageFixtureReport();
@@ -183,7 +201,7 @@ test('coverage is combined for multiple processes', skipIfNoInspector, () => {
183201

184202
const fixture = fixtures.path('v8-coverage', 'combined_coverage');
185203
const args = [
186-
'--test', '--experimental-test-coverage', '--test-reporter', 'tap',
204+
'--test', '--experimental-test-coverage', '--no-warnings', '--test-coverage-include=**', '--test-reporter', 'tap',
187205
];
188206
const result = spawnSync(process.execPath, args, {
189207
env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
@@ -236,7 +254,8 @@ test.skip('coverage works with isolation=none', skipIfNoInspector, () => {
236254
test('coverage reports on lines, functions, and branches', skipIfNoInspector, async (t) => {
237255
const fixture = fixtures.path('test-runner', 'coverage.js');
238256
const child = spawnSync(process.execPath,
239-
['--test', '--experimental-test-coverage', '--test-reporter',
257+
['--test', '--experimental-test-coverage',
258+
'--no-warnings', '--test-coverage-include=**', '--test-reporter',
240259
fixtures.fileURL('test-runner/custom_reporters/coverage.mjs'),
241260
fixture]);
242261
assert.strictEqual(child.stderr.toString(), '');
@@ -297,7 +316,6 @@ test('coverage with ESM hook - source irrelevant', skipIfNoInspector, () => {
297316
'# ------------------------------------------------------------------',
298317
'# hooks.mjs | 100.00 | 100.00 | 100.00 | ',
299318
'# register-hooks.js | 100.00 | 100.00 | 100.00 | ',
300-
'# virtual.js | 100.00 | 100.00 | 100.00 | ',
301319
'# ------------------------------------------------------------------',
302320
'# all files | 100.00 | 100.00 | 100.00 | ',
303321
'# ------------------------------------------------------------------',
@@ -327,7 +345,6 @@ test('coverage with ESM hook - source transpiled', skipIfNoInspector, () => {
327345
'# ------------------------------------------------------------------',
328346
'# hooks.mjs | 100.00 | 100.00 | 100.00 | ',
329347
'# register-hooks.js | 100.00 | 100.00 | 100.00 | ',
330-
'# sum.test.ts | 100.00 | 100.00 | 100.00 | ',
331348
'# sum.ts | 100.00 | 100.00 | 100.00 | ',
332349
'# ------------------------------------------------------------------',
333350
'# all files | 100.00 | 100.00 | 100.00 | ',
@@ -384,7 +401,38 @@ test('coverage with excluded files', skipIfNoInspector, () => {
384401
assert.strictEqual(result.status, 0);
385402
assert(!findCoverageFileForPid(result.pid));
386403
});
404+
test('coverage should not include test files by default', skipIfNoInspector, () => {
405+
const fixture = fixtures.path('test-runner', 'coverage.js');
406+
const args = [
407+
'--experimental-test-coverage', '--test-reporter', 'tap',
408+
fixture,
409+
];
410+
const result = spawnSync(process.execPath, args);
411+
const report = [
412+
'# start of coverage report',
413+
'# ---------------------------------------------------------------',
414+
'# file | line % | branch % | funcs % | uncovered lines',
415+
'# ---------------------------------------------------------------',
416+
'# test | | | | ',
417+
'# fixtures | | | | ',
418+
'# test-runner | | | | ',
419+
'# v8-coverage | | | | ',
420+
'# throw.js | 71.43 | 50.00 | 100.00 | 5-6',
421+
'# ---------------------------------------------------------------',
422+
'# all files | 78.13 | 40.00 | 60.00 | ',
423+
'# ---------------------------------------------------------------',
424+
'# end of coverage report',
425+
].join('\n');
387426

427+
428+
if (common.isWindows) {
429+
return report.replaceAll('/', '\\');
430+
}
431+
432+
assert(result.stdout.toString().includes(report));
433+
assert.strictEqual(result.status, 0);
434+
assert(!findCoverageFileForPid(result.pid));
435+
});
388436
test('coverage with included files', skipIfNoInspector, () => {
389437
const fixture = fixtures.path('test-runner', 'coverage.js');
390438
const args = [
@@ -458,18 +506,17 @@ test('coverage with included and excluded files', skipIfNoInspector, () => {
458506
test('correctly prints the coverage report of files contained in parent directories', skipIfNoInspector, () => {
459507
let report = [
460508
'# start of coverage report',
461-
'# --------------------------------------------------------------------------------------------',
509+
'# ------------------------------------------------------------------',
462510
'# file | line % | branch % | funcs % | uncovered lines',
463-
'# --------------------------------------------------------------------------------------------',
511+
'# ------------------------------------------------------------------',
464512
'# .. | | | | ',
465-
'# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72',
466513
'# invalid-tap.js | 100.00 | 100.00 | 100.00 | ',
467514
'# .. | | | | ',
468515
'# v8-coverage | | | | ',
469516
'# throw.js | 71.43 | 50.00 | 100.00 | 5-6',
470-
'# --------------------------------------------------------------------------------------------',
471-
'# all files | 78.35 | 43.75 | 60.00 | ',
472-
'# --------------------------------------------------------------------------------------------',
517+
'# ------------------------------------------------------------------',
518+
'# all files | 75.00 | 66.67 | 100.00 | ',
519+
'# ------------------------------------------------------------------',
473520
'# end of coverage report',
474521
].join('\n');
475522

‎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)