test_runner: remove promises returned by t.test()

cjihrig · jasnell · commit 1a2eb15bc610 · 2025-01-31T12:56:03.000-08:00
This commit updates the TestContext.prototype.test() API 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
@@ -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
@@ -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
@@ -154,8 +154,6 @@ Failed tests:
       *
       *
       *
-      *
-      *
 &#10006; subtest sync throw fails (*ms)
   '2 subtests failed'
 &#10006; timed out async test (*ms)
diff --git 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
@@ -363,7 +363,6 @@
       *
       *
       *
-      *
 
 *
  1 (*ms)
@@ -376,8 +375,6 @@
       *
       *
       *
-      *
-      *
 
 *
  2 (*ms)
@@ -391,7 +388,6 @@
       *
       *
       *
-      *
 
 *
  1 (*ms)
@@ -439,7 +435,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
@@ -351,8 +351,7 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at first
 		</testcase>
 		<testcase name="sync throw fails at second" time="*" classname="test" failure="thrown from subtest sync throw fails at second">
 			<failure type="testCodeFailure" message="thrown from subtest sync throw fails at second">
-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
       *
       *
       *
-      *
-      *
 }
 			</failure>
 		</testcase>
diff --git 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
@@ -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
@@ -285,8 +285,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
@@ -288,8 +288,6 @@
       *
       *
       *
-      *
-      *
 
 *
  timed out async test (*ms)
diff --git a/test/parallel/test-runner-module-mocking.js b/test/parallel/test-runner-module-mocking.js
@@ -449,13 +449,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) => {

Original file line number	Diff line number	Diff line change
`@@ -304,7 +304,7 @@ class TestContext {`
`304`	`304`	`Test, name, options, fn, overrides,`
`305`	`305`	`);`
`306`	`306`
`307`		`- return subtest.start();`
	`307`	`+ subtest.start();`
`308`	`308`	`}`
`309`	`309`
`310`	`310`	`before(fn, options) {`
Original file line number	Diff line number	Diff line change
`@@ -154,8 +154,6 @@ Failed tests:`
`154`	`154`	`*`
`155`	`155`	`*`
`156`	`156`	`*`
`157`		`- *`
`158`		`- *`
`159`	`157`	`✖ subtest sync throw fails (*ms)`
`160`	`158`	`'2 subtests failed'`
`161`	`159`	`✖ timed out async test (*ms)`
Original file line number	Diff line number	Diff line change
`@@ -576,7 +576,6 @@ not ok 16 - t.after throws - no subtests`
`576`	`576`	`*`
`577`	`577`	`*`
`578`	`578`	`*`
`579`		`- *`
`580`	`579`	`...`
`581`	`580`	`1..2`
`582`	`581`	`not ok 17 - t.beforeEach throws`
`@@ -607,8 +606,6 @@ not ok 17 - t.beforeEach throws`
`607`	`606`	`*`
`608`	`607`	`*`
`609`	`608`	`*`
`610`		`- *`
`611`		`- *`
`612`	`609`	`...`
`613`	`610`	`# Subtest: 2`
`614`	`611`	`not ok 2 - 2`
`@@ -629,7 +626,6 @@ not ok 17 - t.beforeEach throws`
`629`	`626`	`*`
`630`	`627`	`*`
`631`	`628`	`*`
`632`		`- *`
`633`	`629`	`...`
`634`	`630`	`1..2`
`635`	`631`	`not ok 18 - t.afterEach throws`
`@@ -757,7 +753,6 @@ not ok 21 - afterEach context when test fails`
`757`	`753`	`*`
`758`	`754`	`*`
`759`	`755`	`*`
`760`		`- *`
`761`	`756`	`...`
`762`	`757`	`1..2`
`763`	`758`	`not ok 22 - afterEach throws and test fails`
Original file line number	Diff line number	Diff line change
`@@ -363,7 +363,6 @@`
`363`	`363`	`*`
`364`	`364`	`*`
`365`	`365`	`*`
`366`		`- *`
`367`	`366`
`368`	`367`	`*`
`369`	`368`	`1 (*ms)`
`@@ -376,8 +375,6 @@`
`376`	`375`	`*`
`377`	`376`	`*`
`378`	`377`	`*`
`379`		`- *`
`380`		`- *`
`381`	`378`
`382`	`379`	`*`
`383`	`380`	`2 (*ms)`
`@@ -391,7 +388,6 @@`
`391`	`388`	`*`
`392`	`389`	`*`
`393`	`390`	`*`
`394`		`- *`
`395`	`391`
`396`	`392`	`*`
`397`	`393`	`1 (*ms)`
`@@ -439,7 +435,6 @@`
`439`	`435`	`*`
`440`	`436`	`*`
`441`	`437`	`*`
`442`		`- *`
`443`	`438`
`444`	`439`	`*`
`445`	`440`	`t.after() is called if test body throws (*ms)`
Original file line number	Diff line number	Diff line change
`@@ -351,8 +351,7 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at first`
`351`	`351`	`</testcase>`
`352`	`352`	`<testcase name="sync throw fails at second" time="*" classname="test" failure="thrown from subtest sync throw fails at second">`
`353`	`353`	`<failure type="testCodeFailure" message="thrown from subtest sync throw fails at second">`
`354`		`-Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at second`
`355`		`- * {`
	`354`	`+[Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at second] {`
`356`	`355`	`code: 'ERR_TEST_FAILURE',`
`357`	`356`	`failureType: 'testCodeFailure',`
`358`	`357`	`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`
`362`	`361`	`*`
`363`	`362`	`*`
`364`	`363`	`*`
`365`		`- *`
`366`		`- *`
`367`	`364`	`}`
`368`	`365`	`</failure>`
`369`	`366`	`</testcase>`
Original file line number	Diff line number	Diff line change
`@@ -598,8 +598,6 @@ not ok 51 - custom inspect symbol that throws fail`
`598`	`598`	`*`
`599`	`599`	`*`
`600`	`600`	`*`
`601`		`- *`
`602`		`- *`
`603`	`601`	`...`
`604`	`602`	`1..2`
`605`	`603`	`not ok 52 - subtest sync throw fails`
Original file line number	Diff line number	Diff line change
`@@ -606,8 +606,6 @@ not ok 51 - custom inspect symbol that throws fail`
`606`	`606`	`*`
`607`	`607`	`*`
`608`	`608`	`*`
`609`		`- *`
`610`		`- *`
`611`	`609`	`...`
`612`	`610`	`1..2`
`613`	`611`	`not ok 52 - subtest sync throw fails`
Original file line number	Diff line number	Diff line change
`@@ -285,8 +285,6 @@`
`285`	`285`	`*`
`286`	`286`	`*`
`287`	`287`	`*`
`288`		`- *`
`289`		`- *`
`290`	`288`
`291`	`289`	`*`
`292`	`290`	`timed out async test (*ms)`
Original file line number	Diff line number	Diff line change
`@@ -288,8 +288,6 @@`
`288`	`288`	`*`
`289`	`289`	`*`
`290`	`290`	`*`
`291`		`- *`
`292`		`- *`
`293`	`291`
`294`	`292`	`*`
`295`	`293`	`timed out async test (*ms)`