test_runner: remove promises returned by test()

cjihrig · jasnell · commit 96718268fec7 · 2025-01-31T12:56:02.000-08:00
This commit updates the test() and suite() APIs to no longer return a Promise. Fixes: #51292 PR-URL: #56664 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Raz Luvaton <rluvaton@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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);
   });
 });

Original file line number	Diff line number	Diff line change
`@@ -122,4 +122,4 @@ describe('Coverage with source maps', async () => {`
`122`	`122`	`t.assert.strictEqual(spawned.code, 1);`
`123`	`123`	`});`
`124`	`124`	`}`
`125`		`-}).then(common.mustCall());`
	`125`	`+});`