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.

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 coverage by default #56060

Merged
merged 5 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2291,6 +2291,9 @@ This option may be specified multiple times to exclude 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 all the matching test files are excluded from the coverage report.
Specifying this option will override the default behavior.

### `--test-coverage-functions=threshold`

<!-- YAML
Expand Down
7 changes: 5 additions & 2 deletions doc/api/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -476,8 +476,10 @@ all tests have completed. If the [`NODE_V8_COVERAGE`][] environment variable is
used to specify a code coverage directory, the generated V8 coverage files are
written to that directory. Node.js core modules and files within
`node_modules/` directories are, by default, not included in the coverage report.
However, they can be explicitly included via the [`--test-coverage-include`][] flag. If
coverage is enabled, the coverage report is sent to any [test reporters][] via
However, they can be explicitly included via the [`--test-coverage-include`][] flag.
By default all the matching test files are excluded from the coverage report.
Exclusions can be overridden by using the [`--test-coverage-exclude`][] flag.
If coverage is enabled, the coverage report is sent to any [test reporters][] via
pmarchini marked this conversation as resolved.
Show resolved Hide resolved
the `'test:coverage'` event.

Coverage can be disabled on a series of lines using the following
Expand Down Expand Up @@ -3594,6 +3596,7 @@ Can be used to abort test subtasks when the test has been aborted.
[`--experimental-test-module-mocks`]: cli.md#--experimental-test-module-mocks
[`--import`]: cli.md#--importmodule
[`--test-concurrency`]: cli.md#--test-concurrency
[`--test-coverage-exclude`]: cli.md#--test-coverage-exclude
[`--test-coverage-include`]: cli.md#--test-coverage-include
[`--test-name-pattern`]: cli.md#--test-name-pattern
[`--test-only`]: cli.md#--test-only
Expand Down
23 changes: 23 additions & 0 deletions lib/internal/fs/glob.js
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,30 @@ class Glob {
}
}

/**
* Check if a path matches a glob pattern
* @param {string} path the path to check
* @param {string} pattern the glob pattern to match
* @param {boolean} windows whether the path is on a Windows system, defaults to `isWindows`
* @returns {boolean}
*/
function matchGlobPattern(path, pattern, windows = isWindows) {
validateString(path, 'path');
validateString(pattern, 'pattern');
return lazyMinimatch().minimatch(path, pattern, {
kEmptyObject,
nocase: isMacOS || isWindows,
windowsPathsNoEscape: true,
nonegate: true,
nocomment: true,
optimizationLevel: 2,
platform: windows ? 'win32' : 'posix',
nocaseMagicOnly: true,
});
}

module.exports = {
__proto__: null,
Glob,
matchGlobPattern,
};
17 changes: 12 additions & 5 deletions lib/internal/test_runner/coverage.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const {
} = require('fs');
const { setupCoverageHooks } = require('internal/util');
const { tmpdir } = require('os');
const { join, resolve, relative, matchesGlob } = require('path');
const { join, resolve, relative } = require('path');
const { fileURLToPath } = require('internal/url');
const { kMappings, SourceMap } = require('internal/source_map/source_map');
const {
Expand All @@ -36,6 +36,8 @@ const {
ERR_SOURCE_MAP_MISSING_SOURCE,
},
} = require('internal/errors');
const { matchGlobPattern } = require('internal/fs/glob');

const kCoverageFileRegex = /^coverage-(\d+)-(\d{13})-(\d+)\.json$/;
const kIgnoreRegex = /\/\* node:coverage ignore next (?<count>\d+ )?\*\//;
const kLineEndingRegex = /\r?\n$/u;
Expand Down Expand Up @@ -464,19 +466,24 @@ class TestCoverage {
coverageExcludeGlobs: excludeGlobs,
coverageIncludeGlobs: includeGlobs,
} = this.options;

// This check filters out files that match the exclude globs.
if (excludeGlobs?.length > 0) {
for (let i = 0; i < excludeGlobs.length; ++i) {
if (matchesGlob(relativePath, excludeGlobs[i]) ||
matchesGlob(absolutePath, excludeGlobs[i])) return true;
if (
matchGlobPattern(relativePath, excludeGlobs[i]) ||
matchGlobPattern(absolutePath, excludeGlobs[i])
) return true;
}
}

// This check filters out files that do not match the include globs.
if (includeGlobs?.length > 0) {
for (let i = 0; i < includeGlobs.length; ++i) {
if (matchesGlob(relativePath, includeGlobs[i]) ||
matchesGlob(absolutePath, includeGlobs[i])) return false;
if (
matchGlobPattern(relativePath, includeGlobs[i]) ||
matchGlobPattern(absolutePath, includeGlobs[i])
) return false;
}
return true;
}
Expand Down
5 changes: 5 additions & 0 deletions lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,11 @@ function parseCommandLine() {

if (coverage) {
coverageExcludeGlobs = getOptionValue('--test-coverage-exclude');
if (!coverageExcludeGlobs || coverageExcludeGlobs.length === 0) {
// TODO(pmarchini): this default should follow something similar to c8 defaults
// Default exclusions should be also exported to be used by other tools / users
coverageExcludeGlobs = [kDefaultPattern];
}
coverageIncludeGlobs = getOptionValue('--test-coverage-include');

branchCoverage = getOptionValue('--test-coverage-branches');
Expand Down
27 changes: 6 additions & 21 deletions lib/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,12 @@ const {
} = require('internal/validators');

const {
getLazy,
emitExperimentalWarning,
isWindows,
isMacOS,
getLazy,
} = require('internal/util');

const lazyMinimatch = getLazy(() => require('internal/deps/minimatch/index'));
const lazyMatchGlobPattern = getLazy(() => require('internal/fs/glob').matchGlobPattern);

function isPathSeparator(code) {
return code === CHAR_FORWARD_SLASH || code === CHAR_BACKWARD_SLASH;
Expand Down Expand Up @@ -164,22 +163,6 @@ function _format(sep, pathObject) {
return dir === pathObject.root ? `${dir}${base}` : `${dir}${sep}${base}`;
}

function glob(path, pattern, windows) {
emitExperimentalWarning('glob');
validateString(path, 'path');
validateString(pattern, 'pattern');
return lazyMinimatch().minimatch(path, pattern, {
__proto__: null,
nocase: isMacOS || isWindows,
windowsPathsNoEscape: true,
nonegate: true,
nocomment: true,
optimizationLevel: 2,
platform: windows ? 'win32' : 'posix',
nocaseMagicOnly: true,
});
}

const win32 = {
/**
* path.resolve([from ...], to)
Expand Down Expand Up @@ -1140,7 +1123,8 @@ const win32 = {
},

matchesGlob(path, pattern) {
return glob(path, pattern, true);
emitExperimentalWarning('glob');
return lazyMatchGlobPattern()(path, pattern, true);
},

sep: '\\',
Expand Down Expand Up @@ -1616,7 +1600,8 @@ const posix = {
},

matchesGlob(path, pattern) {
return glob(path, pattern, false);
emitExperimentalWarning('glob');
return lazyMatchGlobPattern()(path, pattern, false);
},

sep: '/',
Expand Down
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');

test('foo returns 1', () => {
assert.strictEqual(foo(), 1);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import test from 'node:test';
import assert from 'node:assert';
import { foo } from './logic-file.js';

test('foo returns 1', () => {
assert.strictEqual(foo(), 1);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import test from 'node:test';
import assert from 'node:assert';
import { foo } from './logic-file.js';

test('foo returns 1', () => {
assert.strictEqual(foo(), 1);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
function foo() {
return 1;
}

function bar() {
return 'bar';
}

module.exports = { foo, bar };
7 changes: 7 additions & 0 deletions test/fixtures/test-runner/coverage-default-exclusion/test.cjs
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', () => {
assert.strictEqual(foo(), 1);
});
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', () => {
assert.strictEqual(foo(), 1);
});
11 changes: 10 additions & 1 deletion test/fixtures/test-runner/output/lcov_reporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,13 @@ const fixtures = require('../../../common/fixtures');
const spawn = require('node:child_process').spawn;

spawn(process.execPath,
['--no-warnings', '--experimental-test-coverage', '--test-reporter', 'lcov', fixtures.path('test-runner/output/output.js')], { stdio: 'inherit' });
[
'--no-warnings',
'--experimental-test-coverage',
'--test-coverage-exclude=!test/**',
'--test-reporter',
'lcov',
fixtures.path('test-runner/output/output.js')
],
{ stdio: 'inherit' }
);
116 changes: 116 additions & 0 deletions test/parallel/test-runner-coverage-default-exclusion.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import '../common/index.mjs';
import { before, describe, it } from 'node:test';
import assert from 'node:assert';
import { spawnSync } from 'node:child_process';
import { cp } from 'node:fs/promises';
import tmpdir from '../common/tmpdir.js';
import fixtures from '../common/fixtures.js';
const skipIfNoInspector = {
skip: !process.features.inspector ? 'inspector disabled' : false
};

tmpdir.refresh();

async function setupFixtures() {
const fixtureDir = fixtures.path('test-runner', 'coverage-default-exclusion');
await cp(fixtureDir, tmpdir.path, { recursive: true });
}

describe('test runner coverage default exclusion', skipIfNoInspector, () => {
before(async () => {
await setupFixtures();
});

it('should override default exclusion setting --test-coverage-exclude', async () => {
const report = [
'# start of coverage report',
'# ---------------------------------------------------------------------------',
'# file | line % | branch % | funcs % | uncovered lines',
'# ---------------------------------------------------------------------------',
'# file-test.js | 100.00 | 100.00 | 100.00 | ',
'# file.test.mjs | 100.00 | 100.00 | 100.00 | ',
'# logic-file.js | 66.67 | 100.00 | 50.00 | 5-7',
'# test.cjs | 100.00 | 100.00 | 100.00 | ',
'# test | | | | ',
'# not-matching-test-name.js | 100.00 | 100.00 | 100.00 | ',
'# ---------------------------------------------------------------------------',
'# all files | 91.89 | 100.00 | 83.33 | ',
'# ---------------------------------------------------------------------------',
'# end of coverage report',
].join('\n');


const args = [
'--test',
'--experimental-test-coverage',
'--test-coverage-exclude=!test/**',
'--test-reporter=tap',
];
const result = spawnSync(process.execPath, args, {
env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
cwd: tmpdir.path
});

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

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 = [
'--test',
'--experimental-test-coverage',
'--test-reporter=tap',
];
const result = spawnSync(process.execPath, args, {
env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
cwd: tmpdir.path
});

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

it('should exclude ts test files when using --experimental-strip-types', 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',
'--experimental-strip-types',
'--disable-warning=ExperimentalWarning',
'--test-reporter=tap',
];
const result = spawnSync(process.execPath, args, {
env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
cwd: tmpdir.path
});

assert.strictEqual(result.stderr.toString(), '');
assert(result.stdout.toString().includes(report));
assert.strictEqual(result.status, 0);
});
});
6 changes: 5 additions & 1 deletion test/parallel/test-runner-coverage-source-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ function generateReport(report) {

const flags = [
'--enable-source-maps',
'--test', '--experimental-test-coverage', '--test-reporter', 'tap',
'--test',
'--experimental-test-coverage',
'--test-coverage-exclude=!test/**',
'--test-reporter',
'tap',
];

describe('Coverage with source maps', async () => {
Expand Down
Loading
Loading