Revert "test_runner: remove promises returned by t.test()"

This reverts commit 1a2eb15.

PR-URL: #58282
Fixes: #58227
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>

Author: romainmenke

doc/api/test.md

@@ -105,11 +105,11 @@ top level test with two subtests.
 
 ```js
 test('top level test', async (t) => {
-  t.test('subtest 1', (t) => {
+  await t.test('subtest 1', (t) => {
     assert.strictEqual(1, 1);
   });
 
-  t.test('subtest 2', (t) => {
+  await t.test('subtest 2', (t) => {
     assert.strictEqual(2, 2);
   });
 });
@@ -118,7 +118,12 @@ test('top level test', async (t) => {
 > **Note:** `beforeEach` and `afterEach` hooks are triggered
 > between each subtest execution.
 
-Any subtest failures cause the parent test to fail.
+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.
 
 ## Skipping tests
 
@@ -236,20 +241,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.
-  t.test('running subtest');
+  await t.test('running subtest');
 
   // The test context can be updated to run subtests with the 'only' option.
   t.runOnly(true);
-  t.test('this subtest is now skipped');
-  t.test('this subtest is run', { only: true });
+  await t.test('this subtest is now skipped');
+  await t.test('this subtest is run', { only: true });
 
   // Switch the context back to execute all tests.
   t.runOnly(false);
-  t.test('this subtest is now run');
+  await t.test('this subtest is now run');
 
   // Explicitly do not run these tests.
-  t.test('skipped subtest 3', { only: false });
-  t.test('skipped subtest 4', { skip: true });
+  await t.test('skipped subtest 3', { only: false });
+  await t.test('skipped subtest 4', { skip: true });
 });
 
 // The 'only' option is not set, so this test is skipped.
@@ -304,13 +309,13 @@ multiple times (e.g. `--test-name-pattern="test 1"`,
 
 ```js
 test('test 1', async (t) => {
-  t.test('test 2');
-  t.test('test 3');
+  await t.test('test 2');
+  await t.test('test 3');
 });
 
 test('Test 4', async (t) => {
-  t.test('Test 5');
-  t.test('test 6');
+  await t.test('Test 5');
+  await t.test('test 6');
 });
 ```
 
@@ -3388,9 +3393,12 @@ before each subtest of the current test.
 ```js
 test('top level test', async (t) => {
   t.beforeEach((t) => t.diagnostic(`about to run ${t.name}`));
-  t.test('This is a subtest', (t) => {
-    assert.ok('some relevant assertion here');
-  });
+  await t.test(
+    'This is a subtest',
+    (t) => {
+      assert.ok('some relevant assertion here');
+    },
+  );
 });
 ```
 
@@ -3448,9 +3456,12 @@ after each subtest of the current test.
 ```js
 test('top level test', async (t) => {
   t.afterEach((t) => t.diagnostic(`finished running ${t.name}`));
-  t.test('This is a subtest', (t) => {
-    assert.ok('some relevant assertion here');
-  });
+  await t.test(
+    'This is a subtest',
+    (t) => {
+      assert.ok('some relevant assertion here');
+    },
+  );
 });
 ```
 
@@ -3702,8 +3713,10 @@ no-op.
 test('top level test', (t) => {
   // The test context can be set to run subtests with the 'only' option.
   t.runOnly(true);
-  t.test('this subtest is now skipped');
-  t.test('this subtest is run', { only: true });
+  return Promise.all([
+    t.test('this subtest is now skipped'),
+    t.test('this subtest is run', { only: true }),
+  ]);
 });
 ```
 
@@ -3775,10 +3788,6 @@ added:
   - v18.0.0
   - v16.17.0
 changes:
-  - version:
-    - v24.0.0
-    pr-url: https://github.com/nodejs/node/pull/56664
-    description: This function no longer returns a `Promise`.
   - version:
     - v18.8.0
     - v16.18.0
@@ -3823,13 +3832,14 @@ 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) => {
-  t.test(
+  await t.test(
     'This is a subtest',
     { only: false, skip: false, concurrency: 1, todo: false, plan: 1 },
     (t) => {

lib/internal/test_runner/test.js

@@ -370,7 +370,7 @@ class TestContext {
       Test, name, options, fn, overrides,
     );
 
-    subtest.start();
+    return subtest.start();
   }
 
   before(fn, options) {

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

@@ -158,6 +158,8 @@ Failed tests:
       *
       *
       *
+      *
+      *
 ✖ subtest sync throw fails (*ms)
   '2 subtests failed'
 ✖ timed out async test (*ms)

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

@@ -576,6 +576,7 @@ not ok 16 - t.after throws - no subtests
         *
         *
         *
+        *
       ...
     1..2
 not ok 17 - t.beforeEach throws
@@ -606,6 +607,8 @@ not ok 17 - t.beforeEach throws
         *
         *
         *
+        *
+        *
       ...
     # Subtest: 2
     not ok 2 - 2
@@ -626,6 +629,7 @@ not ok 17 - t.beforeEach throws
         *
         *
         *
+        *
       ...
     1..2
 not ok 18 - t.afterEach throws
@@ -753,6 +757,7 @@ not ok 21 - afterEach context when test fails
         *
         *
         *
+        *
       ...
     1..2
 not ok 22 - afterEach throws and test fails

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

@@ -363,6 +363,7 @@
       *
       *
       *
+      *
 
 *
  1 (*ms)
@@ -375,6 +376,8 @@
       *
       *
       *
+      *
+      *
 
 *
  2 (*ms)
@@ -388,6 +391,7 @@
       *
       *
       *
+      *
 
 *
  1 (*ms)
@@ -435,6 +439,7 @@
       *
       *
       *
+      *
 
 *
  t.after() is called if test body throws (*ms)

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

@@ -355,7 +355,8 @@ 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
@@ -365,6 +366,8 @@ Error [ERR_TEST_FAILURE]: thrown from subtest sync throw fails at first
       *
       *
       *
+      *
+      *
 }
 			</failure>
 		</testcase>

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

@@ -606,6 +606,8 @@ not ok 51 - custom inspect symbol that throws fail
         *
         *
         *
+        *
+        *
       ...
     1..2
 not ok 52 - subtest sync throw fails

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

@@ -614,6 +614,8 @@ not ok 51 - custom inspect symbol that throws fail
         *
         *
         *
+        *
+        *
       ...
     1..2
 not ok 52 - subtest sync throw fails

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

@@ -289,6 +289,8 @@
       *
       *
       *
+      *
+      *
 
 *
  timed out async test (*ms)

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

@@ -292,6 +292,8 @@
       *
       *
       *
+      *
+      *
 
 *
  timed out async test (*ms)