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

Jump to bottom

test_runner: omit filtered test from output #52221

Merged
merged 1 commit into from
Mar 28, 2024
Merged

test_runner: omit filtered test from output #52221

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
5 changes: 3 additions & 2 deletions doc/api/test.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ If Node.js is started with the [`--test-only`][] command-line option, it is
possible to skip all top level tests except for a selected subset by passing
the `only` option to the tests that should be run. When a test with the `only`
option set is run, all subtests are also run. The test context's `runOnly()`
method can be used to implement the same behavior at the subtest level.
method can be used to implement the same behavior at the subtest level. Tests
that are not executed are omitted from the test runner output.
Copy link
Member

Choose a reason for hiding this comment

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

Skip tests are not executed but do appear, don't they?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct (precedence for behavior is filtering > skip > todo). I wasn't sure on the best wording to describe that in these two paragraphs.


```js
// Assume Node.js is run with the --test-only command-line option.
Expand Down Expand Up @@ -239,7 +240,7 @@ 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.
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 Down
75 changes: 52 additions & 23 deletions lib/internal/test_runner/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ const {
} = parseCommandLine();
let kResistStopPropagation;
let findSourceMap;
let noopTestStream;

function lazyFindSourceMap(file) {
if (findSourceMap === undefined) {
Expand Down Expand Up @@ -252,13 +253,20 @@ class Test extends AsyncResource {
parent = null;
}

this.name = name;
this.parent = parent;
this.testNumber = 0;
this.outputSubtestCount = 0;
this.filteredSubtestCount = 0;
this.filtered = false;

if (parent === null) {
this.concurrency = 1;
this.nesting = 0;
this.only = testOnlyFlag;
this.reporter = new TestsStream();
this.runOnlySubtests = this.only;
this.testNumber = 0;
this.childNumber = 0;
this.timeout = kDefaultTimeout;
this.root = this;
this.hooks = {
Expand All @@ -277,7 +285,7 @@ class Test extends AsyncResource {
this.only = only ?? !parent.runOnlySubtests;
this.reporter = parent.reporter;
this.runOnlySubtests = !this.only;
this.testNumber = parent.subtests.length + 1;
this.childNumber = parent.subtests.length + 1;
this.timeout = parent.timeout;
this.root = parent.root;
this.hooks = {
Expand All @@ -287,6 +295,13 @@ class Test extends AsyncResource {
beforeEach: ArrayPrototypeSlice(parent.hooks.beforeEach),
afterEach: ArrayPrototypeSlice(parent.hooks.afterEach),
};

if ((testNamePatterns !== null && !this.matchesTestNamePatterns()) ||
(testOnlyFlag && !this.only)) {
skip = true;
this.filtered = true;
this.parent.filteredSubtestCount++;
}
}

switch (typeof concurrency) {
Expand Down Expand Up @@ -314,17 +329,6 @@ class Test extends AsyncResource {
this.timeout = timeout;
}

this.name = name;
this.parent = parent;

if (testNamePatterns !== null && !this.matchesTestNamePatterns()) {
skip = 'test name does not match pattern';
}

if (testOnlyFlag && !this.only) {
skip = '\'only\' option not set';
}

if (skip) {
fn = noop;
}
Expand Down Expand Up @@ -435,14 +439,14 @@ class Test extends AsyncResource {
while (this.pendingSubtests.length > 0 && this.hasConcurrency()) {
const deferred = ArrayPrototypeShift(this.pendingSubtests);
const test = deferred.test;
this.reporter.dequeue(test.nesting, test.loc, test.name);
test.reporter.dequeue(test.nesting, test.loc, test.name);
await test.run();
deferred.resolve();
}
}

addReadySubtest(subtest) {
this.readySubtests.set(subtest.testNumber, subtest);
this.readySubtests.set(subtest.childNumber, subtest);
}

processReadySubtestRange(canSend) {
Expand Down Expand Up @@ -503,7 +507,7 @@ class Test extends AsyncResource {
const test = new Factory({ __proto__: null, fn, name, parent, ...options, ...overrides });

if (parent.waitingOn === 0) {
parent.waitingOn = test.testNumber;
parent.waitingOn = test.childNumber;
}

if (preventAddingSubtests) {
Expand Down Expand Up @@ -591,6 +595,14 @@ class Test extends AsyncResource {
}

start() {
if (this.filtered) {
noopTestStream ??= new TestsStream();
this.reporter = noopTestStream;
this.run = this.filteredRun;
} else {
this.testNumber = ++this.parent.outputSubtestCount;
}

// If there is enough available concurrency to run the test now, then do
// it. Otherwise, return a Promise to the caller and mark the test as
// pending for later execution.
Expand Down Expand Up @@ -639,6 +651,13 @@ class Test extends AsyncResource {
}
}

async filteredRun() {
this.pass();
this.subtests = [];
this.report = noop;
this.postRun();
}

async run() {
if (this.parent !== null) {
this.parent.activeSubtests++;
Expand Down Expand Up @@ -784,11 +803,14 @@ class Test extends AsyncResource {
this.mock?.reset();

if (this.parent !== null) {
const report = this.getReportDetails();
report.details.passed = this.passed;
this.reporter.complete(this.nesting, this.loc, this.testNumber, this.name, report.details, report.directive);
if (!this.filtered) {
const report = this.getReportDetails();
report.details.passed = this.passed;
this.testNumber ||= ++this.parent.outputSubtestCount;
this.reporter.complete(this.nesting, this.loc, this.testNumber, this.name, report.details, report.directive);
this.parent.activeSubtests--;
}

this.parent.activeSubtests--;
this.parent.addReadySubtest(this);
this.parent.processReadySubtestRange(false);
this.parent.processPendingSubtests();
Expand Down Expand Up @@ -846,7 +868,7 @@ class Test extends AsyncResource {
isClearToSend() {
return this.parent === null ||
(
this.parent.waitingOn === this.testNumber && this.parent.isClearToSend()
this.parent.waitingOn === this.childNumber && this.parent.isClearToSend()
);
}

Expand Down Expand Up @@ -893,8 +915,8 @@ class Test extends AsyncResource {

report() {
countCompletedTest(this);
if (this.subtests.length > 0) {
this.reporter.plan(this.subtests[0].nesting, this.loc, this.subtests.length);
if (this.outputSubtestCount > 0) {
this.reporter.plan(this.subtests[0].nesting, this.loc, this.outputSubtestCount);
} else {
this.reportStarted();
}
Expand Down Expand Up @@ -996,6 +1018,13 @@ class Suite extends Test {
}),
() => {
this.buildPhaseFinished = true;

// A suite can transition from filtered to unfiltered based on the
// tests that it contains.
if (this.filtered && this.filteredSubtestCount !== this.subtests.length) {
this.filtered = false;
this.parent.filteredSubtestCount--;
}
},
);
} catch (err) {
Expand Down
Loading
Loading