Skip to content
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

test_runner: exclude test files from code coverage by default #55633

Closed
wants to merge 1 commit into from
Closed

test_runner: exclude test files from code coverage by default #55633

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
3 changes: 3 additions & 0 deletions doc/api/cli.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2352,6 +2352,9 @@ This option may be specified multiple times to include 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, the files being tested are excluded from code coverage. They can be explicitly
included via this flag.

Comment on lines +2355 to +2357
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
By default, the files being tested are excluded from code coverage. They can be explicitly
included via this flag.
By default, the test files are excluded from code coverage. They can be explicitly
included via this flag.

### `--test-coverage-lines=threshold`

<!-- YAML
Expand Down
2 changes: 2 additions & 0 deletions doc/api/test.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1359,6 +1359,8 @@ changes:
This property is only applicable when `coverage` was set to `true`.
If both `coverageExcludeGlobs` and `coverageIncludeGlobs` are provided,
files must meet **both** criteria to be included in the coverage report.
By default, the files being tested are excluded from code coverage. They can be explicitly
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
By default, the files being tested are excluded from code coverage. They can be explicitly
By default, the test files are excluded from code coverage. They can be explicitly

included via this parameter.
**Default:** `undefined`.
* `lineCoverage` {number} Require a minimum percent of covered lines. If code
coverage does not reach the threshold specified, the process will exit with code `1`.
Expand Down
6 changes: 4 additions & 2 deletions lib/internal/test_runner/coverage.js
View file
Open in desktop
Llorx marked this conversation as resolved.
Show resolved Hide resolved
Llorx marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';
const {
ArrayFrom,
ArrayPrototypeIncludes,
ArrayPrototypeMap,
ArrayPrototypePush,
JSONParse,
Expand Down Expand Up @@ -463,6 +464,7 @@ class TestCoverage {
const {
coverageExcludeGlobs: excludeGlobs,
coverageIncludeGlobs: includeGlobs,
testFiles,
} = this.options;
// This check filters out files that match the exclude globs.
if (excludeGlobs?.length > 0) {
Expand All @@ -481,8 +483,8 @@ class TestCoverage {
return true;
}

// This check filters out the node_modules/ directory, unless it is explicitly included.
return StringPrototypeIncludes(url, '/node_modules/');
// This check filters out the node_modules/ directory and the test files, unless they are explicitly included.
return StringPrototypeIncludes(url, '/node_modules/') || ArrayPrototypeIncludes(testFiles, absolutePath);
}
}

Expand Down
3 changes: 2 additions & 1 deletion lib/internal/test_runner/harness.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const {
createHook,
executionAsyncId,
} = require('async_hooks');
const { relative } = require('path');
const { relative, resolve } = require('path');
const {
codes: {
ERR_TEST_FAILURE,
Expand Down Expand Up @@ -261,6 +261,7 @@ function lazyBootstrapRoot() {
};
const globalOptions = parseCommandLine();
globalOptions.cwd = process.cwd();
globalOptions.testFiles = entryFile ? [resolve(globalOptions.cwd, entryFile)] : [];
Llorx marked this conversation as resolved.
Show resolved Hide resolved
createTestTree(rootTestOptions, globalOptions);
globalRoot.reporter.on('test:summary', (data) => {
if (!data.success) {
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/test_runner/runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,8 @@ function run(options = kEmptyObject) {
validateStringArray(execArgv, 'options.execArgv');

const rootTestOptions = { __proto__: null, concurrency, timeout, signal };
let testFiles = files ?? createTestFileList(globPatterns, cwd);
const absoluteTestFiles = ArrayPrototypeMap(testFiles, (file) => resolve(cwd, file));
const globalOptions = {
__proto__: null,
// parseCommandLine() should not be used here. However, The existing run()
Expand All @@ -675,13 +677,13 @@ function run(options = kEmptyObject) {
coverage,
coverageExcludeGlobs,
coverageIncludeGlobs,
testFiles: absoluteTestFiles,
lineCoverage: lineCoverage,
branchCoverage: branchCoverage,
functionCoverage: functionCoverage,
cwd,
};
const root = createTestTree(rootTestOptions, globalOptions);
let testFiles = files ?? createTestFileList(globPatterns, cwd);

if (shard) {
testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
Expand Down Expand Up @@ -733,7 +735,6 @@ function run(options = kEmptyObject) {
};
} else if (isolation === 'none') {
if (watch) {
const absoluteTestFiles = ArrayPrototypeMap(testFiles, (file) => (isAbsolute(file) ? file : resolve(cwd, file)));
Llorx marked this conversation as resolved.
Show resolved Hide resolved
filesWatcher = watchFiles(absoluteTestFiles, opts);
runFiles = async () => {
root.harness.bootstrapPromise = null;
Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-runner-coverage-source-map.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ function generateReport(report) {

const flags = [
'--enable-source-maps',
'--no-warnings', '--test-coverage-include=**',
'--test', '--experimental-test-coverage', '--test-reporter', 'tap',
];

Expand Down
12 changes: 12 additions & 0 deletions test/parallel/test-runner-coverage-thresholds.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ for (const coverage of coverages) {
const result = spawnSync(process.execPath, [
'--test',
'--experimental-test-coverage',
'--no-warnings',
'--test-coverage-include=**',
Copy link
Member

Choose a reason for hiding this comment

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

I have mixed feelings about all these --test-coverage-include=** flags.
IMHO, since we're changing the default behaviour, it might make sense to avoid a large number of tests focused on a 'non-default' behaviour.
Although we could address this in a separate PR, I think it would be better, if it makes sense to you, to handle it here to avoid a "test-refactor only" PR later.

WDYT?

cc @nodejs/test_runner @redyetidev

Copy link
Member

@avivkeller avivkeller Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I also have mixed feelings about it, as ** would also include anything in node_modules. I'm honestly okay with keeping the test files in the coverage report, and having the user exclude them manually with the exclude flag.

Copy link
Author

@Llorx Llorx Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

--test-coverage-include=** was a quick update here as for testing I prefer to update the arrange/act to behave exactly equal as before the update, than modify the assert to handle a different result than before. In this case easier to understand the arrange/act and update it accordingly so it behaves equally as before, than understanding the internals and what result it should output to be correct.

The thing with the filters is that I can't think of any reason a user would want the test files in the coverage report, so we are going to force all the developers to handle a dirty output that they don't actually want, or force them to use the --test-coverage-include flag, but either way is not comfortable. I always liked the premise of nodejs and how convenient it is without too much hassle, not like other languages/frameworks. It usually aims for the majority of user needs, so it is not usual to find yourself fiddling too much as there's high chance that the defaults fit your needs. In the case of the code coverage is the contrary right now.

Copy link
Member

@avivkeller avivkeller Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In the Node.js test files, the ** is fine.

I've self-addressed my concerns:
My concern was that users that want to include test files would use --test-coverage-include=**, which will also include node_modules/, which they may not want. I suppose it's not that bad for users who, for some reason, need the test files (and only the test files) to use --test-coverage-include=** --test-coverage-exclude=node_modules/.

Copy link
Author

@Llorx Llorx Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Ahhh I see, that's true, and that's not comfortable neither... I think that the more we are talking, the more this is heading to have an exclusive --coverage-include-test-files flag (or similar) that is disabled by default... Makes sense as right now test files is an exclusive option named "testFiles" in the TestCoverage class not linked to the include or exclude in any direct way, but just when applying the filters.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe, but that could be done in a followup if users show that they need it. For now, maybe this is okay?

Copy link
Author

Choose a reason for hiding this comment

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

Well, for me it is okay obviosuly because it fits my needs haha. We can see how people reacts to this. This is my first PR so I don't know how this works. This first goes to the non LTS and when people complain/debate/whatever it goes to the LTS if okay, right?

Copy link
Member

Choose a reason for hiding this comment

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

This first goes to the non LTS and when people complain/debate/whatever it goes to the LTS if okay, right?

Yes, that's how it works. I'm happy to see where this goes.

`${coverage.flag}=25`,
'--test-reporter', 'tap',
fixture,
Expand All @@ -77,6 +79,8 @@ for (const coverage of coverages) {
const result = spawnSync(process.execPath, [
'--test',
'--experimental-test-coverage',
'--no-warnings',
'--test-coverage-include=**',
`${coverage.flag}=25`,
'--test-reporter', reporter,
fixture,
Expand All @@ -92,6 +96,8 @@ for (const coverage of coverages) {
const result = spawnSync(process.execPath, [
'--test',
'--experimental-test-coverage',
'--no-warnings',
'--test-coverage-include=**',
`${coverage.flag}=99`,
'--test-reporter', 'tap',
fixture,
Expand All @@ -108,6 +114,8 @@ for (const coverage of coverages) {
const result = spawnSync(process.execPath, [
'--test',
'--experimental-test-coverage',
'--no-warnings',
'--test-coverage-include=**',
`${coverage.flag}=99`,
'--test-reporter', reporter,
fixture,
Expand All @@ -123,6 +131,8 @@ for (const coverage of coverages) {
const result = spawnSync(process.execPath, [
'--test',
'--experimental-test-coverage',
'--no-warnings',
'--test-coverage-include=**',
`${coverage.flag}=101`,
fixture,
]);
Expand All @@ -136,6 +146,8 @@ for (const coverage of coverages) {
const result = spawnSync(process.execPath, [
'--test',
'--experimental-test-coverage',
'--no-warnings',
'--test-coverage-include=**',
`${coverage.flag}=-1`,
fixture,
]);
Expand Down
77 changes: 62 additions & 15 deletions test/parallel/test-runner-coverage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,11 @@
test('test tap coverage reporter', skipIfNoInspector, async (t) => {
await t.test('coverage is reported and dumped to NODE_V8_COVERAGE if present', (t) => {
const fixture = fixtures.path('test-runner', 'coverage.js');
const args = ['--experimental-test-coverage', '--test-reporter', 'tap', fixture];
const args = [
'--experimental-test-coverage',
'--no-warnings', '--test-coverage-include=**',
'--test-reporter', 'tap', fixture,
];
const options = { env: { ...process.env, NODE_V8_COVERAGE: tmpdir.path } };
const result = spawnSync(process.execPath, args, options);
const report = getTapCoverageFixtureReport();
Expand All @@ -109,7 +113,11 @@

await t.test('coverage is reported without NODE_V8_COVERAGE present', (t) => {
const fixture = fixtures.path('test-runner', 'coverage.js');
const args = ['--experimental-test-coverage', '--test-reporter', 'tap', fixture];
const args = [
'--experimental-test-coverage',
'--no-warnings', '--test-coverage-include=**',
'--test-reporter', 'tap', fixture,
];
const result = spawnSync(process.execPath, args);
const report = getTapCoverageFixtureReport();

Expand All @@ -123,7 +131,11 @@
test('test spec coverage reporter', skipIfNoInspector, async (t) => {
await t.test('coverage is reported and dumped to NODE_V8_COVERAGE if present', (t) => {
const fixture = fixtures.path('test-runner', 'coverage.js');
const args = ['--experimental-test-coverage', '--test-reporter', 'spec', fixture];
const args = [
'--experimental-test-coverage',
'--no-warnings', '--test-coverage-include=**',
'--test-reporter', 'spec', fixture,
];
const options = { env: { ...process.env, NODE_V8_COVERAGE: tmpdir.path } };
const result = spawnSync(process.execPath, args, options);
const report = getSpecCoverageFixtureReport();
Expand All @@ -136,7 +148,11 @@

await t.test('coverage is reported without NODE_V8_COVERAGE present', (t) => {
const fixture = fixtures.path('test-runner', 'coverage.js');
const args = ['--experimental-test-coverage', '--test-reporter', 'spec', fixture];
const args = [
'--experimental-test-coverage',
'--no-warnings', '--test-coverage-include=**',
'--test-reporter', 'spec', fixture,
];
const result = spawnSync(process.execPath, args);
const report = getSpecCoverageFixtureReport();

Expand All @@ -150,7 +166,9 @@
test('single process coverage is the same with --test', skipIfNoInspector, () => {
const fixture = fixtures.path('test-runner', 'coverage.js');
const args = [
'--test', '--experimental-test-coverage', '--test-reporter', 'tap', fixture,
'--test', '--experimental-test-coverage',
'--no-warnings', '--test-coverage-include=**',
'--test-reporter', 'tap', fixture,
];
const result = spawnSync(process.execPath, args);
const report = getTapCoverageFixtureReport();
Expand Down Expand Up @@ -183,7 +201,7 @@

const fixture = fixtures.path('v8-coverage', 'combined_coverage');
const args = [
'--test', '--experimental-test-coverage', '--test-reporter', 'tap',
'--test', '--experimental-test-coverage', '--no-warnings', '--test-coverage-include=**', '--test-reporter', 'tap',
];
const result = spawnSync(process.execPath, args, {
env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
Expand Down Expand Up @@ -236,7 +254,8 @@
test('coverage reports on lines, functions, and branches', skipIfNoInspector, async (t) => {
const fixture = fixtures.path('test-runner', 'coverage.js');
const child = spawnSync(process.execPath,
['--test', '--experimental-test-coverage', '--test-reporter',
['--test', '--experimental-test-coverage',
'--no-warnings', '--test-coverage-include=**', '--test-reporter',
fixtures.fileURL('test-runner/custom_reporters/coverage.mjs'),
fixture]);
assert.strictEqual(child.stderr.toString(), '');
Expand Down Expand Up @@ -297,7 +316,6 @@
'# ------------------------------------------------------------------',
'# hooks.mjs | 100.00 | 100.00 | 100.00 | ',
'# register-hooks.js | 100.00 | 100.00 | 100.00 | ',
'# virtual.js | 100.00 | 100.00 | 100.00 | ',
pmarchini marked this conversation as resolved.
Show resolved Hide resolved
'# ------------------------------------------------------------------',
'# all files | 100.00 | 100.00 | 100.00 | ',
'# ------------------------------------------------------------------',
Expand Down Expand Up @@ -327,7 +345,6 @@
'# ------------------------------------------------------------------',
'# hooks.mjs | 100.00 | 100.00 | 100.00 | ',
'# register-hooks.js | 100.00 | 100.00 | 100.00 | ',
'# sum.test.ts | 100.00 | 100.00 | 100.00 | ',
'# sum.ts | 100.00 | 100.00 | 100.00 | ',
'# ------------------------------------------------------------------',
'# all files | 100.00 | 100.00 | 100.00 | ',
Expand Down Expand Up @@ -380,11 +397,42 @@
return report.replaceAll('/', '\\');
}

assert(result.stdout.toString().includes(report));

Check failure on line 400 in test/parallel/test-runner-coverage.js

View workflow job for this annotation

GitHub Actions / test-linux 

--- stdout ---
▶ test coverage report
  ✔ handles the inspector not being available (1.172183ms)
✔ test coverage report (2.205265ms)
▶ test tap coverage reporter
  ✔ coverage is reported and dumped to NODE_V8_COVERAGE if present (122.532738ms)
::debug::starting to run test coverage report
::debug::starting to run handles the inspector not being available
::debug::completed running handles the inspector not being available
::debug::completed running test coverage report
::debug::starting to run test tap coverage reporter
::debug::starting to run coverage is reported and dumped to NODE_V8_COVERAGE if present
::debug::completed running coverage is reported and dumped to NODE_V8_COVERAGE if present
  ✔ coverage is reported without NODE_V8_COVERAGE present (124.629752ms)
✔ test tap coverage reporter (247.744438ms)
▶ test spec coverage reporter
  ✔ coverage is reported and dumped to NODE_V8_COVERAGE if present (140.992164ms)
::debug::starting to run coverage is reported without NODE_V8_COVERAGE present
::debug::completed running coverage is reported without NODE_V8_COVERAGE present
::debug::completed running test tap coverage reporter
::debug::starting to run test spec coverage reporter
::debug::starting to run coverage is reported and dumped to NODE_V8_COVERAGE if present
::debug::completed running coverage is reported and dumped to NODE_V8_COVERAGE if present
  ✔ coverage is reported without NODE_V8_COVERAGE present (140.682963ms)
✔ test spec coverage reporter (282.322587ms)
✔ single process coverage is the same with --test (243.073981ms)
✔ coverage is combined for multiple processes (369.666596ms)
::debug::starting to run coverage is reported without NODE_V8_COVERAGE present
::debug::completed running coverage is reported without NODE_V8_COVERAGE present
::debug::completed running test spec coverage reporter
::debug::starting to run single process coverage is the same with --test
::debug::completed running single process coverage is the same with --test
::debug::starting to run coverage is combined for multiple processes
::debug::completed running coverage is combined for multiple processes
﹣ coverage works with isolation=none (0.255758ms) # SKIP
▶ coverage reports on lines, functions, and branches
  ✔ does not include node_modules (0.426788ms)
  ✔ reports on function coverage (0.234168ms)
::debug::starting to run coverage works with isolation=none
::debug::completed running coverage works with isolation=none
::debug::starting to run coverage reports on lines, functions, and branches
::debug::starting to run does not include node_modules
::debug::completed running does not include node_modules
::debug::starting to run reports on function coverage
::debug::completed running reports on function coverage
  ✔ reports on branch coverage (0.339755ms)
  ✔ reports on line coverage (0.406049ms)
✔ coverage reports on lines, functions, and branches (265.077639ms)
✔ coverage with ESM hook - source irrelevant (306.262757ms)
::debug::starting to run reports on branch coverage
::debug::completed running reports on branch coverage
::debug::starting to run reports on line coverage
::debug::completed running reports on line coverage
::debug::completed running coverage reports on lines, functions, and branches
::debug::starting to run coverage with ESM hook - source irrelevant
::debug::completed running coverage with ESM hook - source irrelevant
✔ coverage with ESM hook - source transpiled (293.221036ms)
✖ coverage with excluded files (164.982286ms)
  AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
  
    assert(result.stdout.toString().includes(report))
  
      at TestContext.<anonymous> (/home/runner/work/node/node/test/parallel/test-runner-coverage.js:400:3)
      at Test.runInAsyncScope (node:async_hooks:211:14)
      at Test.run (node:internal/test_runner/test:934:25)
      at Test.processPendingSubtests (node:internal/test_runner/test:633:18)
      at Test.postRun (node:internal/test_runner/test:1045:19)
      at Test.run (node:internal/test_runner/test:973:12)
      at async Test.processPendingSubtests (node

Check failure on line 400 in test/parallel/test-runner-coverage.js

View workflow job for this annotation

GitHub Actions / test-macOS 

--- stdout ---
▶ test coverage report
  ✔ handles the inspector not being available (0.407667ms)
✔ test coverage report (0.760875ms)
▶ test tap coverage reporter
  ✔ coverage is reported and dumped to NODE_V8_COVERAGE if present (138.9365ms)
::debug::starting to run test coverage report
::debug::starting to run handles the inspector not being available
::debug::completed running handles the inspector not being available
::debug::completed running test coverage report
::debug::starting to run test tap coverage reporter
::debug::starting to run coverage is reported and dumped to NODE_V8_COVERAGE if present
::debug::completed running coverage is reported and dumped to NODE_V8_COVERAGE if present
  ✔ coverage is reported without NODE_V8_COVERAGE present (133.390792ms)
✔ test tap coverage reporter (272.566417ms)
▶ test spec coverage reporter
  ✔ coverage is reported and dumped to NODE_V8_COVERAGE if present (78.830625ms)
::debug::starting to run coverage is reported without NODE_V8_COVERAGE present
::debug::completed running coverage is reported without NODE_V8_COVERAGE present
::debug::completed running test tap coverage reporter
::debug::starting to run test spec coverage reporter
::debug::starting to run coverage is reported and dumped to NODE_V8_COVERAGE if present
::debug::completed running coverage is reported and dumped to NODE_V8_COVERAGE if present
  ✔ coverage is reported without NODE_V8_COVERAGE present (96.094125ms)
✔ test spec coverage reporter (175.143333ms)
✔ single process coverage is the same with --test (206.648ms)
✔ coverage is combined for multiple processes (197.38675ms)
::debug::starting to run coverage is reported without NODE_V8_COVERAGE present
::debug::completed running coverage is reported without NODE_V8_COVERAGE present
::debug::completed running test spec coverage reporter
::debug::starting to run single process coverage is the same with --test
::debug::completed running single process coverage is the same with --test
::debug::starting to run coverage is combined for multiple processes
::debug::completed running coverage is combined for multiple processes
﹣ coverage works with isolation=none (0.120875ms) # SKIP
▶ coverage reports on lines, functions, and branches
  ✔ does not include node_modules (0.183791ms)
  ✔ reports on function coverage (0.068334ms)
::debug::starting to run coverage works with isolation=none
::debug::completed running coverage works with isolation=none
::debug::starting to run coverage reports on lines, functions, and branches
::debug::starting to run does not include node_modules
::debug::completed running does not include node_modules
::debug::starting to run reports on function coverage
::debug::completed running reports on function coverage
  ✔ reports on branch coverage (0.060375ms)
  ✔ reports on line coverage (0.139625ms)
✔ coverage reports on lines, functions, and branches (139.080167ms)
✔ coverage with ESM hook - source irrelevant (184.939916ms)
::debug::starting to run reports on branch coverage
::debug::completed running reports on branch coverage
::debug::starting to run reports on line coverage
::debug::completed running reports on line coverage
::debug::completed running coverage reports on lines, functions, and branches
::debug::starting to run coverage with ESM hook - source irrelevant
::debug::completed running coverage with ESM hook - source irrelevant
✔ coverage with ESM hook - source transpiled (169.959417ms)
✖ coverage with excluded files (71.792875ms)
  AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
  
    assert(result.stdout.toString().includes(report))
  
      at TestContext.<anonymous> (/Users/runner/work/node/node/test/parallel/test-runner-coverage.js:400:3)
      at Test.runInAsyncScope (node:async_hooks:211:14)
      at Test.run (node:internal/test_runner/test:934:25)
      at Test.processPendingSubtests (node:internal/test_runner/test:633:18)
      at Test.postRun (node:internal/test_runner/test:1045:19)
      at Test.run (node:internal/test_runner/test:973:12)
      at async Test.processPendingSubtests (node:interna
assert.strictEqual(result.status, 0);
assert(!findCoverageFileForPid(result.pid));
});
test('coverage should not include test files by default', skipIfNoInspector, () => {
const fixture = fixtures.path('test-runner', 'coverage.js');
const args = [
'--experimental-test-coverage', '--test-reporter', 'tap',
fixture,
];
const result = spawnSync(process.execPath, args);
const report = [
'# start of coverage report',
'# ---------------------------------------------------------------',
'# file | line % | branch % | funcs % | uncovered lines',
'# ---------------------------------------------------------------',
'# test | | | | ',
'# fixtures | | | | ',
'# test-runner | | | | ',
'# v8-coverage | | | | ',
'# throw.js | 71.43 | 50.00 | 100.00 | 5-6',
'# ---------------------------------------------------------------',
'# all files | 78.13 | 40.00 | 60.00 | ',
'# ---------------------------------------------------------------',
'# end of coverage report',
].join('\n');


if (common.isWindows) {
return report.replaceAll('/', '\\');
}

assert(result.stdout.toString().includes(report));
assert.strictEqual(result.status, 0);
assert(!findCoverageFileForPid(result.pid));
});
test('coverage with included files', skipIfNoInspector, () => {
const fixture = fixtures.path('test-runner', 'coverage.js');
const args = [
Expand Down Expand Up @@ -458,18 +506,17 @@
test('correctly prints the coverage report of files contained in parent directories', skipIfNoInspector, () => {
let report = [
'# start of coverage report',
'# --------------------------------------------------------------------------------------------',
'# ------------------------------------------------------------------',
'# file | line % | branch % | funcs % | uncovered lines',
'# --------------------------------------------------------------------------------------------',
'# ------------------------------------------------------------------',
'# .. | | | | ',
'# coverage.js | 78.65 | 38.46 | 60.00 | 12-13 16-22 27 39 43-44 61-62 66-67 71-72',
'# invalid-tap.js | 100.00 | 100.00 | 100.00 | ',
'# .. | | | | ',
'# v8-coverage | | | | ',
'# throw.js | 71.43 | 50.00 | 100.00 | 5-6',
'# --------------------------------------------------------------------------------------------',
'# all files | 78.35 | 43.75 | 60.00 | ',
'# --------------------------------------------------------------------------------------------',
'# ------------------------------------------------------------------',
'# all files | 75.00 | 66.67 | 100.00 | ',
'# ------------------------------------------------------------------',
'# end of coverage report',
].join('\n');

Expand Down
45 changes: 40 additions & 5 deletions test/parallel/test-runner-run-coverage.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,21 @@
// eslint-disable-next-line no-unused-vars
for await (const _ of stream);
});

await it('should not include test files with complex paths', async () => {
const coverageFiles = [];
const stream = run({ files: [fixtures.path('no-folder', '..', 'test-runner', 'coverage.js')], coverage: true });
stream.on('test:fail', common.mustNotCall());
stream.on('test:pass', common.mustCall());
stream.on('test:coverage', msg => {

Check failure on line 114 in test/parallel/test-runner-run-coverage.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md 

Expected parentheses around arrow function argument
coverageFiles.push(...msg.summary.files.map(file => file.path));

Check failure on line 115 in test/parallel/test-runner-run-coverage.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md 

Expected parentheses around arrow function argument
});
// eslint-disable-next-line no-unused-vars
for await (const _ of stream);
assert.deepStrictEqual(coverageFiles.sort(), [
fixtures.path('test-runner', 'invalid-tap.js'),
fixtures.path('v8-coverage', 'throw.js')

Check failure on line 121 in test/parallel/test-runner-run-coverage.mjs

View workflow job for this annotation

GitHub Actions / lint-js-and-md 

Missing trailing comma
]);
});
await it('should run with coverage and exclude by glob', async () => {
const stream = run({ files, coverage: true, coverageExcludeGlobs: ['test/*/test-runner/invalid-tap.js'] });
stream.on('test:fail', common.mustNotCall());
Expand Down Expand Up @@ -137,13 +151,13 @@

await it('should run while including and excluding globs', async () => {
const stream = run({
files: [...files, fixtures.path('test-runner/invalid-tap.js')],
files: files,
pmarchini marked this conversation as resolved.
Show resolved Hide resolved
coverage: true,
coverageIncludeGlobs: ['test/fixtures/test-runner/*.js'],
coverageExcludeGlobs: ['test/fixtures/test-runner/*-tap.js']
});
stream.on('test:fail', common.mustNotCall());
stream.on('test:pass', common.mustCall(2));
stream.on('test:pass', common.mustCall(1));
stream.on('test:coverage', common.mustCall(({ summary: { files } }) => {
const filesPaths = files.map(({ path }) => path);
assert.strictEqual(filesPaths.every((path) => !path.includes(`test-runner${sep}invalid-tap.js`)), true);
Expand All @@ -153,11 +167,12 @@
for await (const _ of stream);
});

await it('should run with coverage and fail when below line threshold', async () => {
await it('should run with coverage and coverageIncludeGlobs and fail when below thresholds', async () => {
const thresholdErrors = [];
const originalExitCode = process.exitCode;
assert.notStrictEqual(originalExitCode, 1);
const stream = run({ files, coverage: true, lineCoverage: 99, branchCoverage: 99, functionCoverage: 99 });
const stream = run({ files, coverageIncludeGlobs: '**', coverage: true,
Llorx marked this conversation as resolved.
Show resolved Hide resolved
lineCoverage: 99, branchCoverage: 99, functionCoverage: 99 });
stream.on('test:fail', common.mustNotCall());
stream.on('test:pass', common.mustCall(1));
stream.on('test:diagnostic', ({ message }) => {
Expand All @@ -172,6 +187,26 @@
assert.strictEqual(process.exitCode, 1);
process.exitCode = originalExitCode;
});
await it('should run with coverage and fail when below thresholds', async () => {
const thresholdErrors = [];
const originalExitCode = process.exitCode;
assert.notStrictEqual(originalExitCode, 1);
const stream = run({ files, coverage: true,
lineCoverage: 99, branchCoverage: 99, functionCoverage: 99 });
stream.on('test:fail', common.mustNotCall());
stream.on('test:pass', common.mustCall(1));
stream.on('test:diagnostic', ({ message }) => {
const match = message.match(/Error: \d{2}\.\d{2}% (line|branch|function) coverage does not meet threshold of 99%/);
if (match) {
thresholdErrors.push(match[1]);
}
});
// eslint-disable-next-line no-unused-vars
for await (const _ of stream);
assert.deepStrictEqual(thresholdErrors.sort(), ['branch', 'line']);
assert.strictEqual(process.exitCode, 1);
process.exitCode = originalExitCode;
});
});
});

Expand Down
Loading