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: add --test-skip-pattern cli option #52529

Merged
merged 6 commits into from
Apr 18, 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
17 changes: 17 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -1969,6 +1969,9 @@ A regular expression that configures the test runner to only execute tests
whose name matches the provided pattern. See the documentation on
[filtering tests by name][] for more details.

If both `--test-name-pattern` and `--test-skip-pattern` are supplied,
avivkeller marked this conversation as resolved.
Show resolved Hide resolved
tests must satisfy **both** requirements in order to be executed.

### `--test-only`

<!-- YAML
Expand Down Expand Up @@ -2037,6 +2040,20 @@ node --test --test-shard=2/3
node --test --test-shard=3/3
```

### `--test-skip-pattern`

<!-- YAML
added:
- REPLACEME
-->

A regular expression that configures the test runner to skip tests
whose name matches the provided pattern. See the documentation on
[filtering tests by name][] for more details.

If both `--test-name-pattern` and `--test-skip-pattern` are supplied,
tests must satisfy **both** requirements in order to be executed.

### `--test-timeout`

<!-- YAML
Expand Down
23 changes: 15 additions & 8 deletions doc/api/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -298,12 +298,15 @@ describe.only('a suite', () => {

## Filtering tests by name

The [`--test-name-pattern`][] command-line option can be used to only run tests
whose name matches the provided pattern. Test name patterns are interpreted as
JavaScript regular expressions. The `--test-name-pattern` option can be
specified multiple times in order to run nested tests. For each test that is
executed, any corresponding test hooks, such as `beforeEach()`, are also
run. Tests that are not executed are omitted from the test runner output.
The [`--test-name-pattern`][] command-line option can be used to only run
tests whose name matches the provided pattern, and the
[`--test-skip-pattern`][] option can be used to skip tests whose name
matches the provided pattern. Test name patterns are interpreted as
JavaScript regular expressions. The `--test-name-pattern` and
`--test-skip-pattern` options can be specified multiple times in order to run
nested tests. For each test that is executed, any corresponding test hooks,
such as `beforeEach()`, are also run. Tests that are not executed are omitted
from the test runner output.

Given the following test file, starting Node.js with the
`--test-name-pattern="test [1-3]"` option would cause the test runner to execute
Expand All @@ -327,8 +330,8 @@ test('Test 4', async (t) => {

Test name patterns can also be specified using regular expression literals. This
allows regular expression flags to be used. In the previous example, starting
Node.js with `--test-name-pattern="/test [4-5]/i"` would match `Test 4` and
`Test 5` because the pattern is case-insensitive.
Node.js with `--test-name-pattern="/test [4-5]/i"` (or `--test-skip-pattern="/test [4-5]/i"`)
would match `Test 4` and `Test 5` because the pattern is case-insensitive.

To match a single test with a pattern, you can prefix it with all its ancestor
test names separated by space, to ensure it is unique.
Expand All @@ -349,6 +352,9 @@ only `some test` in `test 1`.

Test name patterns do not change the set of files that the test runner executes.

If both `--test-name-pattern` and `--test-skip-pattern` are supplied,
tests must satisfy **both** requirements in order to be executed.

## Extraneous asynchronous activity

Once a test function finishes executing, the results are reported as quickly
Expand Down Expand Up @@ -3157,6 +3163,7 @@ Can be used to abort test subtasks when the test has been aborted.
[`--test-only`]: cli.md#--test-only
[`--test-reporter-destination`]: cli.md#--test-reporter-destination
[`--test-reporter`]: cli.md#--test-reporter
[`--test-skip-pattern`]: cli.md#--test-skip-pattern
[`--test`]: cli.md#--test
[`MockFunctionContext`]: #class-mockfunctioncontext
[`MockTimers`]: #class-mocktimers
Expand Down
6 changes: 5 additions & 1 deletion doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,11 @@ option set.
.
.It Fl -test-shard
Test suite shard to execute in a format of <index>/<total>.

.
.It Fl -test-skip-pattern
A regular expression that configures the test runner to skip tests
whose name matches the provided pattern.
.
.It Fl -test-timeout
A number of milliseconds the test execution will fail after.
.
Expand Down
24 changes: 22 additions & 2 deletions lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ function filterExecArgv(arg, i, arr) {
!ArrayPrototypeSome(kFilterArgValues, (p) => arg === p || (i > 0 && arr[i - 1] === p) || StringPrototypeStartsWith(arg, `${p}=`));
}

function getRunArgs(path, { forceExit, inspectPort, testNamePatterns, only }) {
function getRunArgs(path, { forceExit, inspectPort, testNamePatterns, testSkipPatterns, only }) {
avivkeller marked this conversation as resolved.
Show resolved Hide resolved
const argv = ArrayPrototypeFilter(process.execArgv, filterExecArgv);
if (forceExit === true) {
ArrayPrototypePush(argv, '--test-force-exit');
Expand All @@ -124,6 +124,9 @@ function getRunArgs(path, { forceExit, inspectPort, testNamePatterns, only }) {
if (testNamePatterns != null) {
ArrayPrototypeForEach(testNamePatterns, (pattern) => ArrayPrototypePush(argv, `--test-name-pattern=${pattern}`));
}
if (testSkipPatterns != null) {
avivkeller marked this conversation as resolved.
Show resolved Hide resolved
ArrayPrototypeForEach(testSkipPatterns, (pattern) => ArrayPrototypePush(argv, `--test-skip-pattern=${pattern}`));
}
if (only === true) {
ArrayPrototypePush(argv, '--test-only');
}
Expand Down Expand Up @@ -448,7 +451,7 @@ function watchFiles(testFiles, opts) {
function run(options = kEmptyObject) {
validateObject(options, 'options');

let { testNamePatterns, shard } = options;
let { testNamePatterns, testSkipPatterns, shard } = options;
const {
concurrency,
timeout,
Expand Down Expand Up @@ -514,6 +517,22 @@ function run(options = kEmptyObject) {
throw new ERR_INVALID_ARG_TYPE(name, ['string', 'RegExp'], value);
});
}
if (testSkipPatterns != null) {
if (!ArrayIsArray(testSkipPatterns)) {
testSkipPatterns = [testSkipPatterns];
}

testSkipPatterns = ArrayPrototypeMap(testSkipPatterns, (value, i) => {
if (isRegExp(value)) {
return value;
}
const name = `options.testSkipPatterns[${i}]`;
if (typeof value === 'string') {
return convertStringToRegExp(value, name);
}
throw new ERR_INVALID_ARG_TYPE(name, ['string', 'RegExp'], value);
});
}

const root = createTestTree({ __proto__: null, concurrency, timeout, signal });
root.harness.shouldColorizeTestFiles ||= shouldColorizeTestFiles(root);
Expand All @@ -537,6 +556,7 @@ function run(options = kEmptyObject) {
signal,
inspectPort,
testNamePatterns,
testSkipPatterns,
only,
forceExit,
};
Expand Down
50 changes: 33 additions & 17 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ const {
forceExit,
sourceMaps,
testNamePatterns,
testSkipPatterns,
testOnlyFlag,
} = parseCommandLine();
let kResistStopPropagation;
Expand Down Expand Up @@ -137,6 +138,19 @@ function stopTest(timeout, signal) {
return deferred.promise;
}

function testMatchesPattern(test, patterns) {
MoLow marked this conversation as resolved.
Show resolved Hide resolved
const matchesByNameOrParent = ArrayPrototypeSome(patterns, (re) =>
RegExpPrototypeExec(re, test.name) !== null,
) || (test.parent && testMatchesPattern(test.parent, patterns));
if (matchesByNameOrParent) return true;

const testNameWithAncestors = StringPrototypeTrim(test.getTestNameWithAncestors());

return ArrayPrototypeSome(patterns, (re) =>
RegExpPrototypeExec(re, testNameWithAncestors) !== null,
);
}

class TestContext {
#test;

Expand Down Expand Up @@ -300,8 +314,7 @@ class Test extends AsyncResource {
ownAfterEachCount: 0,
};

if ((testNamePatterns !== null && !this.matchesTestNamePatterns()) ||
(testOnlyFlag && !this.only)) {
if (this.willBeFiltered()) {
this.filtered = true;
this.parent.filteredSubtestCount++;
}
Expand Down Expand Up @@ -408,18 +421,16 @@ class Test extends AsyncResource {
}
}

matchesTestNamePatterns() {
const matchesByNameOrParent = ArrayPrototypeSome(testNamePatterns, (re) =>
RegExpPrototypeExec(re, this.name) !== null,
) ||
this.parent?.matchesTestNamePatterns();

if (matchesByNameOrParent) return true;
willBeFiltered() {
if (testOnlyFlag && !this.only) return true;

const testNameWithAncestors = StringPrototypeTrim(this.getTestNameWithAncestors());
if (!testNameWithAncestors) return false;

return ArrayPrototypeSome(testNamePatterns, (re) => RegExpPrototypeExec(re, testNameWithAncestors) !== null);
if (testNamePatterns && !testMatchesPattern(this, testNamePatterns)) {
return true;
}
if (testSkipPatterns && testMatchesPattern(this, testSkipPatterns)) {
return true;
}
return false;
}

/**
Expand Down Expand Up @@ -987,8 +998,8 @@ class TestHook extends Test {
getRunArgs() {
return this.#args;
}
matchesTestNamePatterns() {
return true;
willBeFiltered() {
return false;
}
postRun() {
const { error, loc, parentTest: parent } = this;
Expand Down Expand Up @@ -1016,7 +1027,7 @@ class Suite extends Test {
constructor(options) {
super(options);

if (testNamePatterns !== null && !options.skip) {
if (testNamePatterns !== null && testSkipPatterns !== null && !options.skip) {
this.fn = options.fn || this.fn;
this.skipped = false;
}
Expand Down Expand Up @@ -1050,7 +1061,12 @@ class Suite extends Test {
// tests that it contains - in case of children matching patterns.
this.filtered = false;
this.parent.filteredSubtestCount--;
} else if (testOnlyFlag && testNamePatterns == null && this.filteredSubtestCount === this.subtests.length) {
} else if (
testOnlyFlag &&
testNamePatterns == null &&
testSkipPatterns == null &&
avivkeller marked this conversation as resolved.
Show resolved Hide resolved
this.filteredSubtestCount === this.subtests.length
) {
// If no subtests are marked as "only", run them all
this.filteredSubtestCount = 0;
}
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 @@ -200,6 +200,7 @@ function parseCommandLine() {
let destinations;
let reporters;
let testNamePatterns;
let testSkipPatterns;
let testOnlyFlag;

if (isChildProcessV8) {
Expand Down Expand Up @@ -240,6 +241,9 @@ function parseCommandLine() {
testNamePatternFlag,
(re) => convertStringToRegExp(re, '--test-name-pattern'),
) : null;
const testSkipPatternFlag = getOptionValue('--test-skip-pattern');
testSkipPatterns = testSkipPatternFlag?.length > 0 ?
ArrayPrototypeMap(testSkipPatternFlag, (re) => convertStringToRegExp(re, '--test-skip-pattern')) : null;
avivkeller marked this conversation as resolved.
Show resolved Hide resolved
}

globalTestOptions = {
Expand All @@ -250,6 +254,7 @@ function parseCommandLine() {
sourceMaps,
testOnlyFlag,
testNamePatterns,
testSkipPatterns,
reporters,
destinations,
};
Expand Down
3 changes: 3 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,9 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"run test at specific shard",
&EnvironmentOptions::test_shard,
kAllowedInEnvvar);
AddOption("--test-skip-pattern",
avivkeller marked this conversation as resolved.
Show resolved Hide resolved
"run tests whose name do not match this regular expression",
&EnvironmentOptions::test_skip_pattern);
AddOption("--test-udp-no-try-send", "", // For testing only.
&EnvironmentOptions::test_udp_no_try_send);
AddOption("--throw-deprecation",
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ class EnvironmentOptions : public Options {
bool test_only = false;
bool test_udp_no_try_send = false;
std::string test_shard;
std::vector<std::string> test_skip_pattern;
bool throw_deprecation = false;
bool trace_atomics_wait = false;
bool trace_deprecation = false;
Expand Down
10 changes: 10 additions & 0 deletions test/fixtures/test-runner/output/name_and_skip_patterns.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Flags: --test-skip-pattern=disabled --test-name-pattern=enabled
'use strict';
const common = require('../../../common');
const {
test,
} = require('node:test');

test('disabled', common.mustNotCall());
test('enabled', common.mustCall());
test('enabled disabled', common.mustNotCall());
15 changes: 15 additions & 0 deletions test/fixtures/test-runner/output/name_and_skip_patterns.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
TAP version 13
# Subtest: enabled
ok 1 - enabled
---
duration_ms: *
...
1..1
# tests 1
# suites 0
# pass 1
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms *
20 changes: 20 additions & 0 deletions test/fixtures/test-runner/output/skip_pattern.js
avivkeller marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Flags: --test-skip-pattern=disabled --test-skip-pattern=/no/i
avivkeller marked this conversation as resolved.
Show resolved Hide resolved
'use strict';
const common = require('../../../common');
const {
describe,
it,
test,
} = require('node:test');

test('top level test disabled', common.mustNotCall());
test('top level skipped test disabled', { skip: true }, common.mustNotCall());
test('top level skipped test enabled', { skip: true }, common.mustNotCall());
it('top level it enabled', common.mustCall());
it('top level it disabled', common.mustNotCall());
it.skip('top level skipped it disabled', common.mustNotCall());
it.skip('top level skipped it enabled', common.mustNotCall());
describe('top level describe', common.mustCall());
describe.skip('top level skipped describe disabled', common.mustNotCall());
describe.skip('top level skipped describe enabled', common.mustNotCall());
test('this will NOt call', common.mustNotCall());
37 changes: 37 additions & 0 deletions test/fixtures/test-runner/output/skip_pattern.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
TAP version 13
# Subtest: top level skipped test enabled
ok 1 - top level skipped test enabled # SKIP
---
duration_ms: *
...
# Subtest: top level it enabled
ok 2 - top level it enabled
---
duration_ms: *
...
# Subtest: top level skipped it enabled
ok 3 - top level skipped it enabled # SKIP
---
duration_ms: *
...
# Subtest: top level describe
ok 4 - top level describe
---
duration_ms: *
type: 'suite'
...
# Subtest: top level skipped describe enabled
ok 5 - top level skipped describe enabled # SKIP
---
duration_ms: *
type: 'suite'
...
1..5
# tests 3
# suites 2
# pass 1
# fail 0
# cancelled 0
# skipped 2
# todo 0
# duration_ms *
2 changes: 2 additions & 0 deletions test/parallel/test-runner-output.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,10 @@ const tests = [
process.features.inspector ? { name: 'test-runner/output/lcov_reporter.js', transform: lcovTransform } : false,
{ name: 'test-runner/output/output.js' },
{ name: 'test-runner/output/output_cli.js' },
{ name: 'test-runner/output/name_and_skip_patterns.js' },
{ name: 'test-runner/output/name_pattern.js' },
{ name: 'test-runner/output/name_pattern_with_only.js' },
{ name: 'test-runner/output/skip_pattern.js' },
{ name: 'test-runner/output/unfinished-suite-async-error.js' },
{ name: 'test-runner/output/unresolved_promise.js' },
{ name: 'test-runner/output/default_output.js', transform: specTransform, tty: true },
Expand Down