test_runner: avoid running twice tests in describe

PR-URL: https://github.com/nodejs/node/pull/46888
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

3 files changed, 29 insertions, 28 deletions

diff --git a/doc/api/test.md b/doc/api/test.md
index 1b39f0c683..00cd263690 100644
--- a/doc/api/test.md
+++ b/doc/api/test.md
@@ -767,7 +767,8 @@ 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} Resolved with `undefined` once the test completes.
+* Returns: {Promise} Resolved with `undefined` once
+  the test completes, or immediately if the test runs within [`describe()`][].
 
 The `test()` function is the value imported from the `test` module. Each
 invocation of this function results in reporting the test to the {TestsStream}.
@@ -776,10 +777,12 @@ 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 resolves once the test completes. 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.
+`test()` returns a `Promise` that resolves once the test completes.
+if `test()` is called within a `describe()` block, it resolve 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) => {
@@ -1700,6 +1703,7 @@ added: v18.7.0
 [`context.diagnostic`]: #contextdiagnosticmessage
 [`context.skip`]: #contextskipmessage
 [`context.todo`]: #contexttodomessage
+[`describe()`]: #describename-options-fn
 [`run()`]: #runoptions
 [`test()`]: #testname-options-fn
 [describe options]: #describename-options-fn
diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
index 748edd9fa2..874f1c253e 100644
--- a/lib/internal/test_runner/harness.js
+++ b/lib/internal/test_runner/harness.js
@@ -1,6 +1,7 @@
 'use strict';
 const {
   ArrayPrototypeForEach,
+  PromiseResolve,
   SafeMap,
 } = primordials;
 const {
@@ -185,31 +186,27 @@ async function startSubtest(subtest) {
   await subtest.start();
 }
 
-function test(name, options, fn) {
-  const parent = testResources.get(executionAsyncId()) || getGlobalRoot();
-  const subtest = parent.createSubtest(Test, name, options, fn);
-  return startSubtest(subtest);
-}
-
-function runInParentContext(Factory) {
+function runInParentContext(Factory, addShorthands = true) {
   function run(name, options, fn, overrides) {
     const parent = testResources.get(executionAsyncId()) || getGlobalRoot();
     const subtest = parent.createSubtest(Factory, name, options, fn, overrides);
-    if (parent === getGlobalRoot()) {
-      startSubtest(subtest);
+    if (!(parent instanceof Suite)) {
+      return startSubtest(subtest);
     }
+    return PromiseResolve();
   }
 
-  const cb = (name, options, fn) => {
-    run(name, options, fn);
-  };
+  const test = (name, options, fn) => run(name, options, fn);
+  if (!addShorthands) {
+    return test;
+  }
 
   ArrayPrototypeForEach(['skip', 'todo', 'only'], (keyword) => {
-    cb[keyword] = (name, options, fn) => {
+    test[keyword] = (name, options, fn) => {
       run(name, options, fn, { [keyword]: true });
     };
   });
-  return cb;
+  return test;
 }
 
 function hook(hook) {
@@ -221,7 +218,7 @@ function hook(hook) {
 
 module.exports = {
   createTestTree,
-  test,
+  test: runInParentContext(Test, false),
   describe: runInParentContext(Suite),
   it: runInParentContext(Test),
   before: hook('before'),
diff --git a/test/message/test_runner_hooks.js b/test/message/test_runner_hooks.js
index dab607f862..bd5adb762d 100644
--- a/test/message/test_runner_hooks.js
+++ b/test/message/test_runner_hooks.js
@@ -32,7 +32,7 @@ describe('describe hooks', () => {
   });
 
   it('1', () => testArr.push('1'));
-  it('2', () => testArr.push('2'));
+  test('2', () => testArr.push('2'));
 
   describe('nested', () => {
     before(function() {
@@ -48,44 +48,44 @@ describe('describe hooks', () => {
       testArr.push('afterEach ' + this.name);
     });
     it('nested 1', () => testArr.push('nested 1'));
-    it('nested 2', () => testArr.push('nested 2'));
+    test('nested 2', () => testArr.push('nested 2'));
   });
 });
 
 describe('before throws', () => {
   before(() => { throw new Error('before'); });
   it('1', () => {});
-  it('2', () => {});
+  test('2', () => {});
 });
 
 describe('after throws', () => {
   after(() => { throw new Error('after'); });
   it('1', () => {});
-  it('2', () => {});
+  test('2', () => {});
 });
 
 describe('beforeEach throws', () => {
   beforeEach(() => { throw new Error('beforeEach'); });
   it('1', () => {});
-  it('2', () => {});
+  test('2', () => {});
 });
 
 describe('afterEach throws', () => {
   afterEach(() => { throw new Error('afterEach'); });
   it('1', () => {});
-  it('2', () => {});
+  test('2', () => {});
 });
 
 describe('afterEach when test fails', () => {
   afterEach(common.mustCall(2));
   it('1', () => { throw new Error('test'); });
-  it('2', () => {});
+  test('2', () => {});
 });
 
 describe('afterEach throws and test fails', () => {
   afterEach(() => { throw new Error('afterEach'); });
   it('1', () => { throw new Error('test'); });
-  it('2', () => {});
+  test('2', () => {});
 });
 
 test('test hooks', async (t) => {