test_runner: improve describe.only behavior

doc/api/test.md

@@ -230,15 +230,24 @@ const { describe, it } = require('node:test');
 ## `only` tests
 
 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()`
+possible to skip all tests except for a selected subset by passing
+the `only` option to the tests that should run. When a test with the `only`
+option is set, all subtests are also run.
+If a suite has the `only` option set, all tests within the suite are run,
+unless it has descendants with the `only` option set, in which case only those
+tests are run.
+
+When using [subtests][] within a `test()`/`it()`, it is required to mark
+all ancestor tests with the `only` option to run only a
+selected subset of tests.
+
+The test context's `runOnly()`
 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.
-// The 'only' option is set, so this test is run.
+// The suite's 'only' option is set, so these tests are run.
 test('this test is run', { only: true }, async (t) => {
  // Within this test, all subtests are run by default.
  await t.test('running subtest');
@@ -262,6 +271,29 @@ test('this test is not run', () => {
  // This code is not run.
  throw new Error('fail');
 });
+
+describe('a suite', () => {
+ // The 'only' option is set, so this test is run.
+ it('this test is run', { only: true }, () => {
+ // This code is run.
+ });
+
+ it('this test is not run', () => {
+ // This code is not run.
+ throw new Error('fail');
+ });
+});
+
+describe.only('a suite', () => {
+ // The 'only' option is set, so this test is run.
+ it('this test is run', () => {
+ // This code is run.
+ });
+
+ it('this test is run', () => {
+ // This code is run.
+ });
+});
 ```
 
 ## Filtering tests by name
@@ -3145,6 +3177,7 @@ Can be used to abort test subtasks when the test has been aborted.
 [describe options]: #describename-options-fn
 [it options]: #testname-options-fn
 [stream.compose]: stream.md#streamcomposestreams
+[subtests]: #subtests
 [suite options]: #suitename-options-fn
 [test reporters]: #test-reporters
 [test runner execution model]: #test-runner-execution-model

lib/internal/test_runner/test.js

@@ -239,8 +239,8 @@ class Test extends AsyncResource {
  constructor(options) {
  super('Test');
 
- let { fn, name, parent, skip } = options;
- const { concurrency, loc, only, timeout, todo, signal } = options;
+ let { fn, name, parent } = options;
+ const { concurrency, loc, only, timeout, todo, skip, signal } = options;
 
  if (typeof fn !== 'function') {
  fn = noop;
@@ -301,10 +301,13 @@ class Test extends AsyncResource {
 
  if ((testNamePatterns !== null && !this.matchesTestNamePatterns()) ||
  (testOnlyFlag && !this.only)) {
- skip = true;
  this.filtered = true;
  this.parent.filteredSubtestCount++;
  }
+
+ if (testOnlyFlag && only === false) {
+ fn = noop;
+ }
  }
 
  switch (typeof concurrency) {
@@ -605,8 +608,12 @@ class Test extends AsyncResource {
  ArrayPrototypePush(this.diagnostics, message);
  }
 
+ get shouldFilter() {
+ return this.filtered && this.parent?.filteredSubtestCount > 0;
+ }
+
  start() {
- if (this.filtered) {
+ if (this.shouldFilter) {
  noopTestStream ??= new TestsStream();
  this.reporter = noopTestStream;
  this.run = this.filteredRun;
@@ -814,7 +821,7 @@ class Test extends AsyncResource {
  this.mock?.reset();
 
  if (this.parent !== null) {
- if (!this.filtered) {
+ if (!this.shouldFilter) {
  const report = this.getReportDetails();
  report.details.passed = this.passed;
  this.testNumber ||= ++this.parent.outputSubtestCount;
@@ -1027,23 +1034,27 @@ class Suite extends Test {
  (err) => {
  this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
  }),
- () => {
- 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--;
- }
- },
+ () => this.postBuild(),
  );
  } catch (err) {
  this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
 
  this.buildPhaseFinished = true;
  }
- this.fn = () => {};
+ this.fn = noop;
+ }
+
+ postBuild() {
+ this.buildPhaseFinished = true;
+ if (this.filtered && this.filteredSubtestCount !== this.subtests.length) {
+ // A suite can transition from filtered to unfiltered based on the
+ // 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) {
+ // If no subtests are marked as "only", run them all
+ this.filteredSubtestCount = 0;
+ }
  }
 
  getRunArgs() {

test/fixtures/test-runner/output/only_tests.js

@@ -1,100 +1,121 @@
 // Flags: --test-only
 'use strict';
-require('../../../common');
+const common = require('../../../common');
 const { test, describe, it } = require('node:test');
 
 // These tests should be skipped based on the 'only' option.
-test('only = undefined');
-test('only = undefined, skip = string', { skip: 'skip message' });
-test('only = undefined, skip = true', { skip: true });
-test('only = undefined, skip = false', { skip: false });
-test('only = false', { only: false });
-test('only = false, skip = string', { only: false, skip: 'skip message' });
-test('only = false, skip = true', { only: false, skip: true });
-test('only = false, skip = false', { only: false, skip: false });
+test('only = undefined', common.mustNotCall());
+test('only = undefined, skip = string', { skip: 'skip message' }, common.mustNotCall());
+test('only = undefined, skip = true', { skip: true }, common.mustNotCall());
+test('only = undefined, skip = false', { skip: false }, common.mustNotCall());
+test('only = false', { only: false }, common.mustNotCall());
+test('only = false, skip = string', { only: false, skip: 'skip message' }, common.mustNotCall());
+test('only = false, skip = true', { only: false, skip: true }, common.mustNotCall());
+test('only = false, skip = false', { only: false, skip: false }, common.mustNotCall());
 
 // These tests should be skipped based on the 'skip' option.
-test('only = true, skip = string', { only: true, skip: 'skip message' });
-test('only = true, skip = true', { only: true, skip: true });
+test('only = true, skip = string', { only: true, skip: 'skip message' }, common.mustNotCall());
+test('only = true, skip = true', { only: true, skip: true }, common.mustNotCall());
 
 // An 'only' test with subtests.
-test('only = true, with subtests', { only: true }, async (t) => {
+test('only = true, with subtests', { only: true }, common.mustCall(async (t) => {
  // These subtests should run.
- await t.test('running subtest 1');
- await t.test('running subtest 2');
+ await t.test('running subtest 1', common.mustCall());
+ await t.test('running subtest 2', common.mustCall());
 
  // Switch the context to only execute 'only' tests.
  t.runOnly(true);
- await t.test('skipped subtest 1');
- await t.test('skipped subtest 2');
- await t.test('running subtest 3', { only: true });
+ await t.test('skipped subtest 1', common.mustNotCall());
+ await t.test('skipped subtest 2'), common.mustNotCall();
+ await t.test('running subtest 3', { only: true }, common.mustCall());
 
  // Switch the context back to execute all tests.
  t.runOnly(false);
- await t.test('running subtest 4', async (t) => {
+ await t.test('running subtest 4', common.mustCall(async (t) => {
  // These subtests should run.
- await t.test('running sub-subtest 1');
- await t.test('running sub-subtest 2');
+ await t.test('running sub-subtest 1', common.mustCall());
+ await t.test('running sub-subtest 2', common.mustCall());
 
  // Switch the context to only execute 'only' tests.
  t.runOnly(true);
- await t.test('skipped sub-subtest 1');
- await t.test('skipped sub-subtest 2');
- });
+ await t.test('skipped sub-subtest 1', common.mustNotCall());
+ await t.test('skipped sub-subtest 2', common.mustNotCall());
+ }));
 
  // Explicitly do not run these tests.
- await t.test('skipped subtest 3', { only: false });
- await t.test('skipped subtest 4', { skip: true });
-});
+ await t.test('skipped subtest 3', { only: false }, common.mustNotCall());
+ await t.test('skipped subtest 4', { skip: true }, common.mustNotCall());
+}));
 
-describe.only('describe only = true, with subtests', () => {
- it.only('`it` subtest 1 should run', () => {});
+describe.only('describe only = true, with subtests', common.mustCall(() => {
+ it.only('`it` subtest 1 should run', common.mustCall());
 
- it('`it` subtest 2 should not run', async () => {});
-});
+ it('`it` subtest 2 should not run', common.mustNotCall());
+}));
 
-describe.only('describe only = true, with a mixture of subtests', () => {
- it.only('`it` subtest 1', () => {});
+describe.only('describe only = true, with a mixture of subtests', common.mustCall(() => {
+ it.only('`it` subtest 1', common.mustCall());
 
- it.only('`it` async subtest 1', async () => {});
+ it.only('`it` async subtest 1', common.mustCall(async () => {}));
 
- it('`it` subtest 2 only=true', { only: true });
+ it('`it` subtest 2 only=true', { only: true }, common.mustCall());
 
- it('`it` subtest 2 only=false', { only: false }, () => {
- throw new Error('This should not run');
- });
+ it('`it` subtest 2 only=false', { only: false }, common.mustNotCall());
 
- it.skip('`it` subtest 3 skip', () => {
- throw new Error('This should not run');
- });
+ it.skip('`it` subtest 3 skip', common.mustNotCall());
 
- it.todo('`it` subtest 4 todo', { only: false }, () => {
- throw new Error('This should not run');
- });
+ it.todo('`it` subtest 4 todo', { only: false }, common.mustNotCall());
 
- test.only('`test` subtest 1', () => {});
+ test.only('`test` subtest 1', common.mustCall());
 
- test.only('`test` async subtest 1', async () => {});
+ test.only('`test` async subtest 1', common.mustCall(async () => {}));
 
- test('`test` subtest 2 only=true', { only: true });
+ test('`test` subtest 2 only=true', { only: true }, common.mustCall());
 
- test('`test` subtest 2 only=false', { only: false }, () => {
- throw new Error('This should not run');
- });
+ test('`test` subtest 2 only=false', { only: false }, common.mustNotCall());
 
- test.skip('`test` subtest 3 skip', () => {
- throw new Error('This should not run');
- });
+ test.skip('`test` subtest 3 skip', common.mustNotCall());
 
- test.todo('`test` subtest 4 todo', { only: false }, () => {
- throw new Error('This should not run');
- });
-});
+ test.todo('`test` subtest 4 todo', { only: false }, common.mustNotCall());
+}));
 
-describe.only('describe only = true, with subtests', () => {
- test.only('subtest should run', () => {});
+describe.only('describe only = true, with subtests', common.mustCall(() => {
+ test.only('subtest should run', common.mustCall());
 
- test('async subtest should not run', async () => {});
+ test('async subtest should not run', common.mustNotCall());
 
- test('subtest should be skipped', { only: false }, () => {});
-});
+ test('subtest should be skipped', { only: false }, common.mustNotCall());
+}));
+
+
+describe('describe only = undefined, with nested only subtest', common.mustCall(() => {
+ test('subtest should not run', common.mustNotCall());
+ describe('nested describe', common.mustCall(() => {
+ test('subtest should not run', common.mustNotCall());
+ test.only('nested test should run', common.mustCall());
+ }));
+}));
+
+
+describe('describe only = undefined, with subtests', common.mustCall(() => {
+ test('async subtest should not run', common.mustNotCall());
+}));
+
+describe('describe only = false, with subtests', { only: false }, common.mustNotCall(() => {
+ test('async subtest should not run', common.mustNotCall());
+}));
+
+
+describe.only('describe only = true, with nested subtests', common.mustCall(() => {
+ test('async subtest should run', common.mustCall());
+ describe('nested describe', common.mustCall(() => {
+ test('nested test should run', common.mustCall());
+ }));
+}));
+
+describe('describe only = false, with nested only subtests', { only: false }, common.mustNotCall(() => {
+ test('async subtest should not run', common.mustNotCall());
+ describe('nested describe', common.mustNotCall(() => {
+ test.only('nested test should run', common.mustNotCall());
+ }));
+}));

test/fixtures/test-runner/output/only_tests.snapshot

@@ -112,10 +112,53 @@ ok 6 - describe only = true, with subtests
  duration_ms: *
  type: 'suite'
  ...
-1..6
-# tests 18
-# suites 3
-# pass 15
+# Subtest: describe only = undefined, with nested only subtest
+ # Subtest: nested describe
+ # Subtest: nested test should run
+ ok 1 - nested test should run
+ ---
+ duration_ms: *
+ ...
+ 1..1
+ ok 1 - nested describe
+ ---
+ duration_ms: *
+ type: 'suite'
+ ...
+ 1..1
+ok 7 - describe only = undefined, with nested only subtest
+ ---
+ duration_ms: *
+ type: 'suite'
+ ...
+# Subtest: describe only = true, with nested subtests
+ # Subtest: async subtest should run
+ ok 1 - async subtest should run
+ ---
+ duration_ms: *
+ ...
+ # Subtest: nested describe
+ # Subtest: nested test should run
+ ok 1 - nested test should run
+ ---
+ duration_ms: *
+ ...
+ 1..1
+ ok 2 - nested describe
+ ---
+ duration_ms: *
+ type: 'suite'
+ ...
+ 1..2
+ok 8 - describe only = true, with nested subtests
+ ---
+ duration_ms: *
+ type: 'suite'
+ ...
+1..8
+# tests 21
+# suites 7
+# pass 18
 # fail 0
 # cancelled 0
 # skipped 3