Skip to content

Commit

Permalink
test_runner: omit filtered test from output
Browse files Browse the repository at this point in the history
This commit updates the test runner to suppress any output for
filtered tests. Filtered tests no longer generate reporter events,
and the unfiltered tests are renumbered in the output as though
the filtered tests were not present. Skipped tests that are not
filtered are still included in the output.

This change is particularly useful when filtering a large number
of tests, as the previously displayed skip output could be
distracting.

Fixes: #51383
PR-URL: #52221
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
  • Loading branch information
cjihrig committed Mar 28, 2024
1 parent 27493a1 commit 29de7f8
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 274 deletions.
5 changes: 3 additions & 2 deletions doc/api/test.md
Expand Up @@ -233,7 +233,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.

```js
// Assume Node.js is run with the --test-only command-line option.
Expand Down Expand Up @@ -270,7 +271,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
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

0 comments on commit 29de7f8

Please sign in to comment.