From bb5088d8c766af81093a6bdcb6e78ce96d38ca02 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 19 Jan 2025 17:46:37 -0500 Subject: [PATCH 1/3] test_runner: automatically wait for subtests to finish This commit updates the test runner to automatically wait for subtests to finish. This makes the experience more consistent with suites and removes the need to await anything. --- lib/internal/test_runner/harness.js | 17 +++++++++++++--- lib/internal/test_runner/test.js | 14 +++++++++++++ .../output/default_output.snapshot | 12 +++-------- .../test-runner/output/dot_reporter.snapshot | 6 +----- .../output/junit_reporter.snapshot | 14 +++++-------- .../test-runner/output/no_refs.snapshot | 20 ++++--------------- .../test-runner/output/output.snapshot | 18 +++++------------ .../test-runner/output/output_cli.snapshot | 18 +++++------------ .../test-runner/output/spec_reporter.snapshot | 12 +++-------- .../output/spec_reporter_cli.snapshot | 12 +++-------- test/parallel/test-runner-output.mjs | 4 ---- 11 files changed, 57 insertions(+), 90 deletions(-) diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index e9356cbc0100bb..b4f46ecdec5586 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -27,6 +27,8 @@ const { shouldColorizeTestFiles, } = require('internal/test_runner/utils'); const { queueMicrotask } = require('internal/process/task_queues'); +const { TIMEOUT_MAX } = require('internal/timers'); +const { clearInterval, setInterval } = require('timers'); const { bigint: hrtime } = process.hrtime; const resolvedPromise = PromiseResolve(); const testResources = new SafeMap(); @@ -212,11 +214,20 @@ function setupProcessState(root, globalOptions) { const rejectionHandler = createProcessEventHandler('unhandledRejection', root); const coverage = configureCoverage(root, globalOptions); - const exitHandler = async () => { + const exitHandler = async (kill) => { if (root.subtests.length === 0 && (root.hooks.before.length > 0 || root.hooks.after.length > 0)) { // Run global before/after hooks in case there are no tests await root.run(); } + + if (kill !== true && root.subtestsPromise !== null) { + // Wait for all subtests to finish, but keep the process alive in case + // there are no ref'ed handles left. + const keepAlive = setInterval(() => {}, TIMEOUT_MAX); + await root.subtestsPromise.promise; + clearInterval(keepAlive); + } + root.postRun(new ERR_TEST_FAILURE( 'Promise resolution is still pending but the event loop has already resolved', kCancelledByParent)); @@ -231,8 +242,8 @@ function setupProcessState(root, globalOptions) { } }; - const terminationHandler = () => { - exitHandler(); + const terminationHandler = async () => { + await exitHandler(true); process.exit(); }; diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index b13d13e105e9ba..9bc090b3836116 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -579,6 +579,8 @@ class Test extends AsyncResource { this.activeSubtests = 0; this.pendingSubtests = []; this.readySubtests = new SafeMap(); + this.unfinishedSubtests = new SafeSet(); + this.subtestsPromise = null; this.subtests = []; this.waitingOn = 0; this.finished = false; @@ -682,6 +684,11 @@ class Test extends AsyncResource { addReadySubtest(subtest) { this.readySubtests.set(subtest.childNumber, subtest); + + if (this.unfinishedSubtests.delete(subtest) && + this.unfinishedSubtests.size === 0) { + this.subtestsPromise.resolve(); + } } processReadySubtestRange(canSend) { @@ -743,6 +750,7 @@ class Test extends AsyncResource { if (parent.waitingOn === 0) { parent.waitingOn = test.childNumber; + parent.subtestsPromise = PromiseWithResolvers(); } if (preventAddingSubtests) { @@ -865,6 +873,7 @@ class Test extends AsyncResource { // 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. + this.parent.unfinishedSubtests.add(this); this.reporter.enqueue(this.nesting, this.loc, this.name, this.reportedType); if (this.root.harness.buildPromise || !this.parent.hasConcurrency()) { const deferred = PromiseWithResolvers(); @@ -982,6 +991,11 @@ class Test extends AsyncResource { } this[kShouldAbort](); + + if (this.subtestsPromise !== null) { + await SafePromiseRace([this.subtestsPromise.promise, stopPromise]); + } + this.plan?.check(); this.pass(); await afterEach(); diff --git a/test/fixtures/test-runner/output/default_output.snapshot b/test/fixtures/test-runner/output/default_output.snapshot index 73bfc1da5e92e9..2d6a0fd6dfe823 100644 --- a/test/fixtures/test-runner/output/default_output.snapshot +++ b/test/fixtures/test-runner/output/default_output.snapshot @@ -24,15 +24,13 @@ *[39m *[39m - [31m✖ should pass but parent fail [90m(*ms)[39m[39m - [32m'test did not finish before its parent and was cancelled'[39m - + [32m✔ should pass but parent fail [90m(*ms)[39m[39m [31m✖ parent [90m(*ms)[39m[39m [34mℹ tests 6[39m [34mℹ suites 0[39m -[34mℹ pass 1[39m +[34mℹ pass 2[39m [34mℹ fail 3[39m -[34mℹ cancelled 1[39m +[34mℹ cancelled 0[39m [34mℹ skipped 1[39m [34mℹ todo 0[39m [34mℹ duration_ms *[39m @@ -63,7 +61,3 @@ *[39m *[39m *[39m - -* -[31m✖ should pass but parent fail [90m(*ms)[39m[39m - [32m'test did not finish before its parent and was cancelled'[39m diff --git a/test/fixtures/test-runner/output/dot_reporter.snapshot b/test/fixtures/test-runner/output/dot_reporter.snapshot index fc2b58cfef8428..4ef804951dc99f 100644 --- a/test/fixtures/test-runner/output/dot_reporter.snapshot +++ b/test/fixtures/test-runner/output/dot_reporter.snapshot @@ -1,5 +1,5 @@ ..XX...X..XXX.X..... -XXX.....X..X...X.... +XXX............X.... .....X...XXX.XX..... XXXXXXX...XXXXX @@ -93,10 +93,6 @@ Failed tests: '1 subtest failed' ✖ sync throw non-error fail (*ms) Symbol(thrown symbol from sync throw non-error fail) -✖ +long running (*ms) - 'test did not finish before its parent and was cancelled' -✖ top level (*ms) - '1 subtest failed' ✖ sync skip option is false fail (*ms) Error: this should be executed * diff --git a/test/fixtures/test-runner/output/junit_reporter.snapshot b/test/fixtures/test-runner/output/junit_reporter.snapshot index aaa5dcd6ff9963..d1868bd4b6eaa9 100644 --- a/test/fixtures/test-runner/output/junit_reporter.snapshot +++ b/test/fixtures/test-runner/output/junit_reporter.snapshot @@ -186,12 +186,8 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fail - - - -[Error [ERR_TEST_FAILURE]: test did not finish before its parent and was cancelled] { code: 'ERR_TEST_FAILURE', failureType: 'cancelledByParent', cause: 'test did not finish before its parent and was cancelled' } - - + + @@ -519,9 +515,9 @@ Error [ERR_TEST_FAILURE]: test could not be started because its parent finished - - - + + + diff --git a/test/fixtures/test-runner/output/no_refs.snapshot b/test/fixtures/test-runner/output/no_refs.snapshot index 310094947f9f96..8014b0343892f7 100644 --- a/test/fixtures/test-runner/output/no_refs.snapshot +++ b/test/fixtures/test-runner/output/no_refs.snapshot @@ -1,35 +1,23 @@ TAP version 13 # Subtest: does not keep event loop alive # Subtest: +does not keep event loop alive - not ok 1 - +does not keep event loop alive + ok 1 - +does not keep event loop alive --- duration_ms: * type: 'test' - location: '/test/fixtures/test-runner/output/no_refs.js:(LINE):11' - failureType: 'cancelledByParent' - error: 'Promise resolution is still pending but the event loop has already resolved' - code: 'ERR_TEST_FAILURE' - stack: |- - * ... 1..1 -not ok 1 - does not keep event loop alive +ok 1 - does not keep event loop alive --- duration_ms: * type: 'test' - location: '/test/fixtures/test-runner/output/no_refs.js:(LINE):1' - failureType: 'cancelledByParent' - error: 'Promise resolution is still pending but the event loop has already resolved' - code: 'ERR_TEST_FAILURE' - stack: |- - * ... 1..1 # tests 2 # suites 0 -# pass 0 +# pass 2 # fail 0 -# cancelled 2 +# cancelled 0 # skipped 0 # todo 0 # duration_ms * diff --git a/test/fixtures/test-runner/output/output.snapshot b/test/fixtures/test-runner/output/output.snapshot index 36d9c289fa1615..044ac4137fa78d 100644 --- a/test/fixtures/test-runner/output/output.snapshot +++ b/test/fixtures/test-runner/output/output.snapshot @@ -288,14 +288,10 @@ ok 23 - level 0a ... # Subtest: top level # Subtest: +long running - not ok 1 - +long running + ok 1 - +long running --- duration_ms: * type: 'test' - location: '/test/fixtures/test-runner/output/output.js:(LINE):5' - failureType: 'cancelledByParent' - error: 'test did not finish before its parent and was cancelled' - code: 'ERR_TEST_FAILURE' ... # Subtest: +short running # Subtest: ++short running @@ -311,14 +307,10 @@ ok 23 - level 0a type: 'test' ... 1..2 -not ok 24 - top level +ok 24 - top level --- duration_ms: * type: 'test' - location: '/test/fixtures/test-runner/output/output.js:(LINE):1' - failureType: 'subtestsFailed' - error: '1 subtest failed' - code: 'ERR_TEST_FAILURE' ... # Subtest: invalid subtest - pass but subtest fails ok 25 - invalid subtest - pass but subtest fails @@ -787,9 +779,9 @@ not ok 62 - invalid subtest fail # Error: Test "callback async throw after done" at test/fixtures/test-runner/output/output.js:(LINE):1 generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. # tests 75 # suites 0 -# pass 34 -# fail 25 -# cancelled 3 +# pass 36 +# fail 24 +# cancelled 2 # skipped 9 # todo 4 # duration_ms * diff --git a/test/fixtures/test-runner/output/output_cli.snapshot b/test/fixtures/test-runner/output/output_cli.snapshot index 4546269836e9ca..eaa085d97d06d1 100644 --- a/test/fixtures/test-runner/output/output_cli.snapshot +++ b/test/fixtures/test-runner/output/output_cli.snapshot @@ -288,14 +288,10 @@ ok 23 - level 0a ... # Subtest: top level # Subtest: +long running - not ok 1 - +long running + ok 1 - +long running --- duration_ms: * type: 'test' - location: '/test/fixtures/test-runner/output/output.js:(LINE):5' - failureType: 'cancelledByParent' - error: 'test did not finish before its parent and was cancelled' - code: 'ERR_TEST_FAILURE' ... # Subtest: +short running # Subtest: ++short running @@ -311,14 +307,10 @@ ok 23 - level 0a type: 'test' ... 1..2 -not ok 24 - top level +ok 24 - top level --- duration_ms: * type: 'test' - location: '/test/fixtures/test-runner/output/output.js:(LINE):1' - failureType: 'subtestsFailed' - error: '1 subtest failed' - code: 'ERR_TEST_FAILURE' ... # Subtest: invalid subtest - pass but subtest fails ok 25 - invalid subtest - pass but subtest fails @@ -801,9 +793,9 @@ ok 63 - last test 1..63 # tests 77 # suites 0 -# pass 36 -# fail 25 -# cancelled 3 +# pass 38 +# fail 24 +# cancelled 2 # skipped 9 # todo 4 # duration_ms * diff --git a/test/fixtures/test-runner/output/spec_reporter.snapshot b/test/fixtures/test-runner/output/spec_reporter.snapshot index f3aebbd7fe5aae..d04ed7b62c5768 100644 --- a/test/fixtures/test-runner/output/spec_reporter.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter.snapshot @@ -118,8 +118,6 @@ level 0a (*ms) top level +long running (*ms) - 'test did not finish before its parent and was cancelled' - +short running ++short running (*ms) +short running (*ms) @@ -302,9 +300,9 @@ Error: Test "callback async throw after done" at test/fixtures/test-runner/output/output.js:269:1 generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. tests 75 suites 0 - pass 34 - fail 25 - cancelled 3 + pass 36 + fail 24 + cancelled 2 skipped 9 todo 4 duration_ms * @@ -415,10 +413,6 @@ sync throw non-error fail (*ms) Symbol(thrown symbol from sync throw non-error fail) -* - +long running (*ms) - 'test did not finish before its parent and was cancelled' - * sync skip option is false fail (*ms) Error: this should be executed diff --git a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot index 2e5f263e1a5e3a..c205253f257e01 100644 --- a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot @@ -118,8 +118,6 @@ level 0a (*ms) top level +long running (*ms) - 'test did not finish before its parent and was cancelled' - +short running ++short running (*ms) +short running (*ms) @@ -305,9 +303,9 @@ Error: Test "callback async throw after done" at test/fixtures/test-runner/output/output.js:269:1 generated asynchronous activity after the test ended. This activity created the error "Error: thrown from callback async throw after done" and would have caused the test to fail, but instead triggered an uncaughtException event. tests 76 suites 0 - pass 35 - fail 25 - cancelled 3 + pass 37 + fail 24 + cancelled 2 skipped 9 todo 4 duration_ms * @@ -418,10 +416,6 @@ sync throw non-error fail (*ms) Symbol(thrown symbol from sync throw non-error fail) -* - +long running (*ms) - 'test did not finish before its parent and was cancelled' - * sync skip option is false fail (*ms) Error: this should be executed diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs index 3bc4b0ad3d1a2e..767c08334843c3 100644 --- a/test/parallel/test-runner-output.mjs +++ b/test/parallel/test-runner-output.mjs @@ -199,10 +199,6 @@ const tests = [ name: 'test-runner/output/unfinished-suite-async-error.js', flags: ['--test-reporter=tap'], }, - { - name: 'test-runner/output/unresolved_promise.js', - flags: ['--test-reporter=tap'], - }, { name: 'test-runner/output/default_output.js', transform: specTransform, tty: true }, { name: 'test-runner/output/arbitrary-output.js', From e35af6746ae03e5ac5b1eb18f29a69b39851906d Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 19 Jan 2025 19:42:22 -0500 Subject: [PATCH 2/3] test_runner: remove promises returned by test() This commit updates the test() and suite() APIs to no longer return a Promise. Fixes: https://github.com/nodejs/node/issues/51292 --- doc/api/test.md | 32 ++++++------------- lib/internal/test_runner/harness.js | 9 +++--- .../test-runner-coverage-source-map.js | 2 +- test/parallel/test-runner-misc.js | 3 +- test/parallel/test-runner-typechecking.js | 24 ++++---------- 5 files changed, 23 insertions(+), 47 deletions(-) diff --git a/doc/api/test.md b/doc/api/test.md index 58a945df02b88f..c165efeb48d1b8 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -1407,6 +1407,11 @@ run({ files: [path.resolve('./tests/test.js')] }) added: - v22.0.0 - v20.13.0 +changes: + - version: + - REPLACEME + pr-url: https://github.com/nodejs/node/pull/56664 + description: This function no longer returns a `Promise`. --> * `name` {string} The name of the suite, which is displayed when reporting test @@ -1417,7 +1422,6 @@ added: * `fn` {Function|AsyncFunction} The suite function declaring nested tests and suites. The first argument to this function is a [`SuiteContext`][] object. **Default:** A no-op function. -* Returns: {Promise} Immediately fulfilled with `undefined`. The `suite()` function is imported from the `node:test` module. @@ -1461,6 +1465,10 @@ added: - v18.0.0 - v16.17.0 changes: + - version: + - REPLACEME + pr-url: https://github.com/nodejs/node/pull/56664 + description: This function no longer returns a `Promise`. - version: - v20.2.0 - v18.17.0 @@ -1510,8 +1518,6 @@ changes: to this function is a [`TestContext`][] object. If the test uses callbacks, the callback function is passed as the second argument. **Default:** A no-op function. -* Returns: {Promise} Fulfilled with `undefined` once - the test completes, or immediately if the test runs within a suite. The `test()` function is the value imported from the `test` module. Each invocation of this function results in reporting the test to the {TestsStream}. @@ -1520,26 +1526,6 @@ The `TestContext` object passed to the `fn` argument can be used to perform actions related to the current test. Examples include skipping the test, adding additional diagnostic information, or creating subtests. -`test()` returns a `Promise` that fulfills once the test completes. -if `test()` is called within a suite, it fulfills immediately. -The return value can usually be discarded for top level tests. -However, the return value from subtests should be used to prevent the parent -test from finishing first and cancelling the subtest -as shown in the following example. - -```js -test('top level test', async (t) => { - // The setTimeout() in the following subtest would cause it to outlive its - // parent test if 'await' is removed on the next line. Once the parent test - // completes, it will cancel any outstanding subtests. - await t.test('longer running subtest', async (t) => { - return new Promise((resolve, reject) => { - setTimeout(resolve, 1000); - }); - }); -}); -``` - The `timeout` option can be used to fail the test if it takes longer than `timeout` milliseconds to complete. However, it is not a reliable mechanism for canceling tests because a running test might block the application thread and diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index b4f46ecdec5586..ab8d1c24ae6d52 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -312,11 +312,12 @@ function runInParentContext(Factory) { function run(name, options, fn, overrides) { const parent = testResources.get(executionAsyncId()) || lazyBootstrapRoot(); const subtest = parent.createSubtest(Factory, name, options, fn, overrides); + if (parent instanceof Suite) { - return PromiseResolve(); + return; } - return startSubtestAfterBootstrap(subtest); + startSubtestAfterBootstrap(subtest); } const test = (name, options, fn) => { @@ -325,7 +326,7 @@ function runInParentContext(Factory) { loc: getCallerLocation(), }; - return run(name, options, fn, overrides); + run(name, options, fn, overrides); }; ArrayPrototypeForEach(['skip', 'todo', 'only'], (keyword) => { test[keyword] = (name, options, fn) => { @@ -335,7 +336,7 @@ function runInParentContext(Factory) { loc: getCallerLocation(), }; - return run(name, options, fn, overrides); + run(name, options, fn, overrides); }; }); return test; diff --git a/test/parallel/test-runner-coverage-source-map.js b/test/parallel/test-runner-coverage-source-map.js index e3b0676a557a9f..3b481f457dd3e6 100644 --- a/test/parallel/test-runner-coverage-source-map.js +++ b/test/parallel/test-runner-coverage-source-map.js @@ -122,4 +122,4 @@ describe('Coverage with source maps', async () => { t.assert.strictEqual(spawned.code, 1); }); } -}).then(common.mustCall()); +}); diff --git a/test/parallel/test-runner-misc.js b/test/parallel/test-runner-misc.js index cea115493249a1..28e626d182e899 100644 --- a/test/parallel/test-runner-misc.js +++ b/test/parallel/test-runner-misc.js @@ -18,9 +18,8 @@ if (process.argv[2] === 'child') { assert.strictEqual(signal.aborted, false); testSignal = signal; await setTimeout(50); - })).finally(common.mustCall(() => { - test(() => assert.strictEqual(testSignal.aborted, true)); })); + test(() => assert.strictEqual(testSignal.aborted, true)); // TODO(benjamingr) add more tests to describe + AbortSignal // this just tests the parameter is passed diff --git a/test/parallel/test-runner-typechecking.js b/test/parallel/test-runner-typechecking.js index e96761b1a054bd..8568b4cb39218b 100644 --- a/test/parallel/test-runner-typechecking.js +++ b/test/parallel/test-runner-typechecking.js @@ -6,7 +6,6 @@ require('../common'); const assert = require('assert'); const { test, describe, it } = require('node:test'); -const { isPromise } = require('util/types'); const testOnly = test('only test', { only: true }); const testTodo = test('todo test', { todo: true }); @@ -16,21 +15,12 @@ const testTodoShorthand = test.todo('todo test shorthand'); const testSkipShorthand = test.skip('skip test shorthand'); describe('\'node:test\' and its shorthands should return the same', () => { - it('should return a Promise', () => { - assert(isPromise(testOnly)); - assert(isPromise(testTodo)); - assert(isPromise(testSkip)); - assert(isPromise(testOnlyShorthand)); - assert(isPromise(testTodoShorthand)); - assert(isPromise(testSkipShorthand)); - }); - - it('should resolve undefined', async () => { - assert.strictEqual(await testOnly, undefined); - assert.strictEqual(await testTodo, undefined); - assert.strictEqual(await testSkip, undefined); - assert.strictEqual(await testOnlyShorthand, undefined); - assert.strictEqual(await testTodoShorthand, undefined); - assert.strictEqual(await testSkipShorthand, undefined); + it('should return undefined', () => { + assert.strictEqual(testOnly, undefined); + assert.strictEqual(testTodo, undefined); + assert.strictEqual(testSkip, undefined); + assert.strictEqual(testOnlyShorthand, undefined); + assert.strictEqual(testTodoShorthand, undefined); + assert.strictEqual(testSkipShorthand, undefined); }); }); From 2b34ffef76efc7f07864204fd307b26c217358ba Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 19 Jan 2025 20:06:50 -0500 Subject: [PATCH 3/3] test_runner: remove promises returned by t.test() This commit updates the TestContext.prototype.test() API to no longer return a Promise. Fixes: https://github.com/nodejs/node/issues/51292 --- doc/api/test.md | 62 ++++++++----------- lib/internal/test_runner/test.js | 2 +- .../test-runner/output/dot_reporter.snapshot | 2 - .../test-runner/output/hooks.snapshot | 5 -- .../output/hooks_spec_reporter.snapshot | 10 --- .../output/junit_reporter.snapshot | 5 +- .../test-runner/output/output.snapshot | 2 - .../test-runner/output/output_cli.snapshot | 2 - .../test-runner/output/spec_reporter.snapshot | 4 -- .../output/spec_reporter_cli.snapshot | 4 -- test/parallel/test-runner-module-mocking.js | 14 +++-- 11 files changed, 36 insertions(+), 76 deletions(-) diff --git a/doc/api/test.md b/doc/api/test.md index c165efeb48d1b8..7040fa1397d4c9 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -105,11 +105,11 @@ top level test with two subtests. ```js test('top level test', async (t) => { - await t.test('subtest 1', (t) => { + t.test('subtest 1', (t) => { assert.strictEqual(1, 1); }); - await t.test('subtest 2', (t) => { + t.test('subtest 2', (t) => { assert.strictEqual(2, 2); }); }); @@ -118,12 +118,7 @@ test('top level test', async (t) => { > **Note:** `beforeEach` and `afterEach` hooks are triggered > between each subtest execution. -In this example, `await` is used to ensure that both subtests have completed. -This is necessary because tests do not wait for their subtests to -complete, unlike tests created within suites. -Any subtests that are still outstanding when their parent finishes -are cancelled and treated as failures. Any subtest failures cause the parent -test to fail. +Any subtest failures cause the parent test to fail. ## Skipping tests @@ -241,20 +236,20 @@ that are not executed are omitted from the test runner output. // 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'); + t.test('running subtest'); // The test context can be updated to run subtests with the 'only' option. t.runOnly(true); - await t.test('this subtest is now skipped'); - await t.test('this subtest is run', { only: true }); + t.test('this subtest is now skipped'); + t.test('this subtest is run', { only: true }); // Switch the context back to execute all tests. t.runOnly(false); - await t.test('this subtest is now run'); + t.test('this subtest is now run'); // Explicitly do not run these tests. - await t.test('skipped subtest 3', { only: false }); - await t.test('skipped subtest 4', { skip: true }); + t.test('skipped subtest 3', { only: false }); + t.test('skipped subtest 4', { skip: true }); }); // The 'only' option is not set, so this test is skipped. @@ -309,13 +304,13 @@ multiple times (e.g. `--test-name-pattern="test 1"`, ```js test('test 1', async (t) => { - await t.test('test 2'); - await t.test('test 3'); + t.test('test 2'); + t.test('test 3'); }); test('Test 4', async (t) => { - await t.test('Test 5'); - await t.test('test 6'); + t.test('Test 5'); + t.test('test 6'); }); ``` @@ -3175,12 +3170,9 @@ before each subtest of the current test. ```js test('top level test', async (t) => { t.beforeEach((t) => t.diagnostic(`about to run ${t.name}`)); - await t.test( - 'This is a subtest', - (t) => { - assert.ok('some relevant assertion here'); - }, - ); + t.test('This is a subtest', (t) => { + assert.ok('some relevant assertion here'); + }); }); ``` @@ -3238,12 +3230,9 @@ after each subtest of the current test. ```js test('top level test', async (t) => { t.afterEach((t) => t.diagnostic(`finished running ${t.name}`)); - await t.test( - 'This is a subtest', - (t) => { - assert.ok('some relevant assertion here'); - }, - ); + t.test('This is a subtest', (t) => { + assert.ok('some relevant assertion here'); + }); }); ``` @@ -3458,10 +3447,8 @@ no-op. test('top level test', (t) => { // The test context can be set to run subtests with the 'only' option. t.runOnly(true); - return Promise.all([ - t.test('this subtest is now skipped'), - t.test('this subtest is run', { only: true }), - ]); + t.test('this subtest is now skipped'); + t.test('this subtest is run', { only: true }); }); ``` @@ -3533,6 +3520,10 @@ added: - v18.0.0 - v16.17.0 changes: + - version: + - REPLACEME + pr-url: https://github.com/nodejs/node/pull/56664 + description: This function no longer returns a `Promise`. - version: - v18.8.0 - v16.18.0 @@ -3577,14 +3568,13 @@ changes: to this function is a [`TestContext`][] object. If the test uses callbacks, the callback function is passed as the second argument. **Default:** A no-op function. -* Returns: {Promise} Fulfilled with `undefined` once the test completes. This function is used to create subtests under the current test. This function behaves in the same fashion as the top level [`test()`][] function. ```js test('top level test', async (t) => { - await t.test( + t.test( 'This is a subtest', { only: false, skip: false, concurrency: 1, todo: false, plan: 1 }, (t) => { diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 9bc090b3836116..489490baef32c6 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -304,7 +304,7 @@ class TestContext { Test, name, options, fn, overrides, ); - return subtest.start(); + subtest.start(); } before(fn, options) { diff --git a/test/fixtures/test-runner/output/dot_reporter.snapshot b/test/fixtures/test-runner/output/dot_reporter.snapshot index 4ef804951dc99f..5abbb979667cfd 100644 --- a/test/fixtures/test-runner/output/dot_reporter.snapshot +++ b/test/fixtures/test-runner/output/dot_reporter.snapshot @@ -154,8 +154,6 @@ Failed tests: * * * - * - * ✖ subtest sync throw fails (*ms) '2 subtests failed' ✖ timed out async test (*ms) diff --git a/test/fixtures/test-runner/output/hooks.snapshot b/test/fixtures/test-runner/output/hooks.snapshot index 4ba957d539ce70..f1f5b7573c9814 100644 --- a/test/fixtures/test-runner/output/hooks.snapshot +++ b/test/fixtures/test-runner/output/hooks.snapshot @@ -576,7 +576,6 @@ not ok 16 - t.after throws - no subtests * * * - * ... 1..2 not ok 17 - t.beforeEach throws @@ -607,8 +606,6 @@ not ok 17 - t.beforeEach throws * * * - * - * ... # Subtest: 2 not ok 2 - 2 @@ -629,7 +626,6 @@ not ok 17 - t.beforeEach throws * * * - * ... 1..2 not ok 18 - t.afterEach throws @@ -757,7 +753,6 @@ not ok 21 - afterEach context when test fails * * * - * ... 1..2 not ok 22 - afterEach throws and test fails diff --git a/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot b/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot index ea916c2ee754c4..c30fc8e7d98002 100644 --- a/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot +++ b/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot @@ -282,7 +282,6 @@ * * * - * t.beforeEach throws (*ms) t.afterEach throws @@ -296,8 +295,6 @@ * * * - * - * 2 (*ms) Error: afterEach @@ -310,7 +307,6 @@ * * * - * t.afterEach throws (*ms) afterEach when test fails @@ -364,7 +360,6 @@ * * * - * afterEach throws and test fails (*ms) t.after() is called if test body throws (*ms) @@ -674,7 +669,6 @@ * * * - * * 1 (*ms) @@ -687,8 +681,6 @@ * * * - * - * * 2 (*ms) @@ -702,7 +694,6 @@ * * * - * * 1 (*ms) @@ -750,7 +741,6 @@ * * * - * * t.after() is called if test body throws (*ms) diff --git a/test/fixtures/test-runner/output/junit_reporter.snapshot b/test/fixtures/test-runner/output/junit_reporter.snapshot index d1868bd4b6eaa9..3b1d15022af704 100644 --- a/test/fixtures/test-runner/output/junit_reporter.snapshot +++ b/test/fixtures/test-runner/output/junit_reporter.snapshot @@ -351,8 +351,7 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at first -Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at second - * { +[Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at second] { code: 'ERR_TEST_FAILURE', failureType: 'testCodeFailure', cause: Error: thrown from subtest sync throw fails at second @@ -362,8 +361,6 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at second * * * - * - * } diff --git a/test/fixtures/test-runner/output/output.snapshot b/test/fixtures/test-runner/output/output.snapshot index 044ac4137fa78d..ffbe91759bb859 100644 --- a/test/fixtures/test-runner/output/output.snapshot +++ b/test/fixtures/test-runner/output/output.snapshot @@ -598,8 +598,6 @@ not ok 51 - custom inspect symbol that throws fail * * * - * - * ... 1..2 not ok 52 - subtest sync throw fails diff --git a/test/fixtures/test-runner/output/output_cli.snapshot b/test/fixtures/test-runner/output/output_cli.snapshot index eaa085d97d06d1..7f989f14c619cf 100644 --- a/test/fixtures/test-runner/output/output_cli.snapshot +++ b/test/fixtures/test-runner/output/output_cli.snapshot @@ -606,8 +606,6 @@ not ok 51 - custom inspect symbol that throws fail * * * - * - * ... 1..2 not ok 52 - subtest sync throw fails diff --git a/test/fixtures/test-runner/output/spec_reporter.snapshot b/test/fixtures/test-runner/output/spec_reporter.snapshot index d04ed7b62c5768..13640ff837f736 100644 --- a/test/fixtures/test-runner/output/spec_reporter.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter.snapshot @@ -216,8 +216,6 @@ * * * - * - * subtest sync throw fails (*ms) timed out async test (*ms) @@ -495,8 +493,6 @@ * * * - * - * * timed out async test (*ms) diff --git a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot index c205253f257e01..1993d409737bb8 100644 --- a/test/fixtures/test-runner/output/spec_reporter_cli.snapshot +++ b/test/fixtures/test-runner/output/spec_reporter_cli.snapshot @@ -219,8 +219,6 @@ * * * - * - * subtest sync throw fails (*ms) timed out async test (*ms) @@ -498,8 +496,6 @@ * * * - * - * * timed out async test (*ms) diff --git a/test/parallel/test-runner-module-mocking.js b/test/parallel/test-runner-module-mocking.js index 803a566a3b4d28..a0f915fdd4c532 100644 --- a/test/parallel/test-runner-module-mocking.js +++ b/test/parallel/test-runner-module-mocking.js @@ -448,13 +448,15 @@ test('mocks are automatically restored', async (t) => { assert.strictEqual(mocked.fn(), 43); }); - const cjsMock = require(cjsFixture); - const esmMock = await import(esmFixture); + t.test('checks original behavior', async () => { + const cjsMock = require(cjsFixture); + const esmMock = await import(esmFixture); - assert.strictEqual(cjsMock.string, 'original cjs string'); - assert.strictEqual(cjsMock.fn, undefined); - assert.strictEqual(esmMock.string, 'original esm string'); - assert.strictEqual(esmMock.fn, undefined); + assert.strictEqual(cjsMock.string, 'original cjs string'); + assert.strictEqual(cjsMock.fn, undefined); + assert.strictEqual(esmMock.string, 'original esm string'); + assert.strictEqual(esmMock.fn, undefined); + }); }); test('mocks can be restored independently', async (t) => {