test_runner: fix top level describe queuing

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

lib/internal/test_runner/test.js

@@ -561,7 +561,12 @@ class Suite extends Test {
 
     try {
       const context = { signal: this.signal };
-      this.buildSuite = this.runInAsyncScope(this.fn, context, [context]);
+      this.buildSuite = PromisePrototypeThen(
+        PromiseResolve(this.runInAsyncScope(this.fn, context, [context])),
+        undefined,
+        (err) => {
+          this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
+        });
     } catch (err) {
       this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
     }
@@ -569,17 +574,9 @@ class Suite extends Test {
     this.buildPhaseFinished = true;
   }
 
-  start() {
-    return this.run();
-  }
-
   async run() {
-    try {
-      await this.buildSuite;
-    } catch (err) {
-      this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
-    }
     this.parent.activeSubtests++;
+    await this.buildSuite;
     this.startTime = hrtime();
 
     if (this[kShouldAbort]()) {

test/message/test_runner_describe_it.js

@@ -149,17 +149,6 @@ describe('level 0a', { concurrency: 4 }, () => {
   return p0a;
 });
 
-describe('top level', { concurrency: 2 }, () => {
-  it('+long running', async () => {
-    return new Promise((resolve, reject) => {
-      setTimeout(resolve, 3000).unref();
-    });
-  });
-
-  describe('+short running', async () => {
-    it('++short running', async () => {});
-  });
-});
 
 describe('invalid subtest - pass but subtest fails', () => {
   setImmediate(() => {
@@ -339,3 +328,47 @@ describe('timeouts', () => {
     setTimeout(done, 10);
   });
 });
+
+describe('successful thenable', () => {
+  it('successful thenable', () => {
+    let thenCalled = false;
+    return {
+      get then() {
+        if (thenCalled) throw new Error();
+        thenCalled = true;
+        return (successHandler) => successHandler();
+      },
+    };
+  });
+
+  it('rejected thenable', () => {
+    let thenCalled = false;
+    return {
+      get then() {
+        if (thenCalled) throw new Error();
+        thenCalled = true;
+        return (_, errorHandler) => errorHandler(new Error('custom error'));
+      },
+    };
+  });
+
+  let thenCalled = false;
+  return {
+    get then() {
+      if (thenCalled) throw new Error();
+      thenCalled = true;
+      return (successHandler) => successHandler();
+    },
+  };
+});
+
+describe('rejected thenable', () => {
+  let thenCalled = false;
+  return {
+    get then() {
+      if (thenCalled) throw new Error();
+      thenCalled = true;
+      return (_, errorHandler) => errorHandler(new Error('custom error'));
+    },
+  };
+});

test/message/test_runner_describe_it.out

@@ -24,6 +24,7 @@ not ok 3 - sync fail todo # TODO
     *
     *
     *
+    *
   ...
 # Subtest: sync fail todo with message
 not ok 4 - sync fail todo with message # TODO this is a failing todo
@@ -74,6 +75,7 @@ not ok 8 - sync throw fail
     *
     *
     *
+    *
   ...
 # Subtest: async skip pass
 ok 9 - async skip pass # SKIP
@@ -100,6 +102,7 @@ not ok 11 - async throw fail
     *
     *
     *
+    *
   ...
 # Subtest: async skip fail
 not ok 12 - async skip fail
@@ -128,6 +131,7 @@ not ok 13 - async assertion fail
     *
     *
     *
+    *
   ...
 # Subtest: resolve pass
 ok 14 - resolve pass
@@ -149,6 +153,7 @@ not ok 15 - reject fail
     *
     *
     *
+    *
   ...
 # Subtest: unhandled rejection - passes but warns
 ok 16 - unhandled rejection - passes but warns
@@ -237,45 +242,23 @@ ok 23 - level 0a
   ---
   duration_ms: *
   ...
-# Subtest: top level
-    # Subtest: +long running
-    ok 1 - +long running
-      ---
-      duration_ms: *
-      ...
-    # Subtest: +short running
-        # Subtest: ++short running
-        ok 1 - ++short running
-          ---
-          duration_ms: *
-          ...
-        1..1
-    ok 2 - +short running
-      ---
-      duration_ms: *
-      ...
-    1..2
-ok 24 - top level
-  ---
-  duration_ms: *
-  ...
 # Subtest: invalid subtest - pass but subtest fails
-ok 25 - invalid subtest - pass but subtest fails
+ok 24 - invalid subtest - pass but subtest fails
   ---
   duration_ms: *
   ...
 # Subtest: sync skip option
-ok 26 - sync skip option # SKIP
+ok 25 - sync skip option # SKIP
   ---
   duration_ms: *
   ...
 # Subtest: sync skip option with message
-ok 27 - sync skip option with message # SKIP this is skipped
+ok 26 - sync skip option with message # SKIP this is skipped
   ---
   duration_ms: *
   ...
 # Subtest: sync skip option is false fail
-not ok 28 - sync skip option is false fail
+not ok 27 - sync skip option is false fail
   ---
   duration_ms: *
   failureType: 'testCodeFailure'
@@ -291,67 +274,67 @@ not ok 28 - sync skip option is false fail
     *
   ...
 # Subtest: <anonymous>
-ok 29 - <anonymous>
+ok 28 - <anonymous>
   ---
   duration_ms: *
   ...
 # Subtest: functionOnly
-ok 30 - functionOnly
+ok 29 - functionOnly
   ---
   duration_ms: *
   ...
 # Subtest: <anonymous>
-ok 31 - <anonymous>
+ok 30 - <anonymous>
   ---
   duration_ms: *
   ...
 # Subtest: test with only a name provided
-ok 32 - test with only a name provided
+ok 31 - test with only a name provided
   ---
   duration_ms: *
   ...
 # Subtest: <anonymous>
-ok 33 - <anonymous>
+ok 32 - <anonymous>
   ---
   duration_ms: *
   ...
 # Subtest: <anonymous>
-ok 34 - <anonymous> # SKIP
+ok 33 - <anonymous> # SKIP
   ---
   duration_ms: *
   ...
 # Subtest: test with a name and options provided
-ok 35 - test with a name and options provided # SKIP
+ok 34 - test with a name and options provided # SKIP
   ---
   duration_ms: *
   ...
 # Subtest: functionAndOptions
-ok 36 - functionAndOptions # SKIP
+ok 35 - functionAndOptions # SKIP
   ---
   duration_ms: *
   ...
 # Subtest: escaped description \\ \# \\\#\\
-ok 37 - escaped description \\ \# \\\#\\
+ok 36 - escaped description \\ \# \\\#\\
   ---
   duration_ms: *
   ...
 # Subtest: escaped skip message
-ok 38 - escaped skip message # SKIP \#skip
+ok 37 - escaped skip message # SKIP \#skip
   ---
   duration_ms: *
   ...
 # Subtest: escaped todo message
-ok 39 - escaped todo message # TODO \#todo
+ok 38 - escaped todo message # TODO \#todo
   ---
   duration_ms: *
   ...
 # Subtest: callback pass
-ok 40 - callback pass
+ok 39 - callback pass
   ---
   duration_ms: *
   ...
 # Subtest: callback fail
-not ok 41 - callback fail
+not ok 40 - callback fail
   ---
   duration_ms: *
   failureType: 'testCodeFailure'
@@ -362,22 +345,22 @@ not ok 41 - callback fail
     *
   ...
 # Subtest: sync t is this in test
-ok 42 - sync t is this in test
+ok 41 - sync t is this in test
   ---
   duration_ms: *
   ...
 # Subtest: async t is this in test
-ok 43 - async t is this in test
+ok 42 - async t is this in test
   ---
   duration_ms: *
   ...
 # Subtest: callback t is this in test
-ok 44 - callback t is this in test
+ok 43 - callback t is this in test
   ---
   duration_ms: *
   ...
 # Subtest: callback also returns a Promise
-not ok 45 - callback also returns a Promise
+not ok 44 - callback also returns a Promise
   ---
   duration_ms: *
   failureType: 'callbackAndPromisePresent'
@@ -385,7 +368,7 @@ not ok 45 - callback also returns a Promise
   code: 'ERR_TEST_FAILURE'
   ...
 # Subtest: callback throw
-not ok 46 - callback throw
+not ok 45 - callback throw
   ---
   duration_ms: *
   failureType: 'testCodeFailure'
@@ -401,7 +384,7 @@ not ok 46 - callback throw
     *
   ...
 # Subtest: callback called twice
-not ok 47 - callback called twice
+not ok 46 - callback called twice
   ---
   duration_ms: *
   failureType: 'multipleCallbackInvocations'
@@ -412,12 +395,12 @@ not ok 47 - callback called twice
     *
   ...
 # Subtest: callback called twice in different ticks
-ok 48 - callback called twice in different ticks
+ok 47 - callback called twice in different ticks
   ---
   duration_ms: *
   ...
 # Subtest: callback called twice in future tick
-not ok 49 - callback called twice in future tick
+not ok 48 - callback called twice in future tick
   ---
   duration_ms: *
   failureType: 'uncaughtException'
@@ -427,7 +410,7 @@ not ok 49 - callback called twice in future tick
     *
   ...
 # Subtest: callback async throw
-not ok 50 - callback async throw
+not ok 49 - callback async throw
   ---
   duration_ms: *
   failureType: 'uncaughtException'
@@ -437,12 +420,12 @@ not ok 50 - callback async throw
     *
   ...
 # Subtest: callback async throw after done
-ok 51 - callback async throw after done
+ok 50 - callback async throw after done
   ---
   duration_ms: *
   ...
 # Subtest: custom inspect symbol fail
-not ok 52 - custom inspect symbol fail
+not ok 51 - custom inspect symbol fail
   ---
   duration_ms: *
   failureType: 'testCodeFailure'
@@ -450,7 +433,7 @@ not ok 52 - custom inspect symbol fail
   code: 'ERR_TEST_FAILURE'
   ...
 # Subtest: custom inspect symbol that throws fail
-not ok 53 - custom inspect symbol that throws fail
+not ok 52 - custom inspect symbol that throws fail
   ---
   duration_ms: *
   failureType: 'testCodeFailure'
@@ -501,7 +484,7 @@ not ok 53 - custom inspect symbol that throws fail
         *
       ...
     1..2
-not ok 54 - subtest sync throw fails
+not ok 53 - subtest sync throw fails
   ---
   duration_ms: *
   failureType: 'subtestsFailed'
@@ -518,7 +501,7 @@ not ok 54 - subtest sync throw fails
       code: 'ERR_TEST_FAILURE'
       ...
     1..1
-not ok 55 - describe sync throw fails
+not ok 54 - describe sync throw fails
   ---
   duration_ms: *
   failureType: 'testCodeFailure'
@@ -546,7 +529,7 @@ not ok 55 - describe sync throw fails
       code: 'ERR_TEST_FAILURE'
       ...
     1..1
-not ok 56 - describe async throw fails
+not ok 55 - describe async throw fails
   ---
   duration_ms: *
   failureType: 'testCodeFailure'
@@ -573,7 +556,7 @@ not ok 56 - describe async throw fails
       error: 'test timed out after 5ms'
       code: 'ERR_TEST_FAILURE'
       stack: |-
-        *
+        async Promise.all (index 0)
       ...
     # Subtest: timed out callback test
     not ok 2 - timed out callback test
@@ -594,15 +577,51 @@ not ok 56 - describe async throw fails
       duration_ms: *
       ...
     1..4
-not ok 57 - timeouts
+not ok 56 - timeouts
   ---
   duration_ms: *
   failureType: 'subtestsFailed'
   error: '2 subtests failed'
   code: 'ERR_TEST_FAILURE'
   ...
+# Subtest: successful thenable
+    # Subtest: successful thenable
+    ok 1 - successful thenable
+      ---
+      duration_ms: *
+      ...
+    # Subtest: rejected thenable
+    not ok 2 - rejected thenable
+      ---
+      duration_ms: *
+      failureType: 'testCodeFailure'
+      error: 'custom error'
+      code: 'ERR_TEST_FAILURE'
+      stack: |-
+        *
+        *
+      ...
+    1..2
+not ok 57 - successful thenable
+  ---
+  duration_ms: *
+  failureType: 'subtestsFailed'
+  error: '1 subtest failed'
+  code: 'ERR_TEST_FAILURE'
+  ...
+# Subtest: rejected thenable
+not ok 58 - rejected thenable
+  ---
+  duration_ms: *
+  failureType: 'testCodeFailure'
+  error: 'custom error'
+  code: 'ERR_TEST_FAILURE'
+  stack: |-
+    *
+    *
+  ...
 # Subtest: invalid subtest fail
-not ok 58 - invalid subtest fail
+not ok 59 - invalid subtest fail
   ---
   duration_ms: *
   failureType: 'parentAlreadyFinished'
@@ -611,16 +630,16 @@ not ok 58 - invalid subtest fail
   stack: |-
     *
   ...
-1..58
+1..59
 # Warning: Test "unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event.
 # Warning: Test "async unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from async unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event.
 # Warning: Test "immediate throw - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from immediate throw fail" and would have caused the test to fail, but instead triggered an uncaughtException event.
 # Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event.
 # Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event.
 # Warning: Test "callback async throw after done" 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 58
-# pass 23
-# fail 21
+# tests 59
+# pass 22
+# fail 23
 # cancelled 0
 # skipped 9
 # todo 5

test/message/test_runner_output.js

@@ -349,3 +349,25 @@ test('large timeout async test is ok', { timeout: 30_000_000 }, async (t) => {
 test('large timeout callback test is ok', { timeout: 30_000_000 }, (t, done) => {
   setTimeout(done, 10);
 });
+
+test('successful thenable', () => {
+  let thenCalled = false;
+  return {
+    get then() {
+      if (thenCalled) throw new Error();
+      thenCalled = true;
+      return (successHandler) => successHandler();
+    },
+  };
+});
+
+test('rejected thenable', () => {
+  let thenCalled = false;
+  return {
+    get then() {
+      if (thenCalled) throw new Error();
+      thenCalled = true;
+      return (_, errorHandler) => errorHandler('custom error');
+    },
+  };
+});

test/message/test_runner_output.out

@@ -588,8 +588,21 @@ ok 60 - large timeout callback test is ok
   ---
   duration_ms: *
   ...
+# Subtest: successful thenable
+ok 61 - successful thenable
+  ---
+  duration_ms: *
+  ...
+# Subtest: rejected thenable
+not ok 62 - rejected thenable
+  ---
+  duration_ms: *
+  failureType: 'testCodeFailure'
+  error: 'custom error'
+  code: 'ERR_TEST_FAILURE'
+  ...
 # Subtest: invalid subtest fail
-not ok 61 - invalid subtest fail
+not ok 63 - invalid subtest fail
   ---
   duration_ms: *
   failureType: 'parentAlreadyFinished'
@@ -598,16 +611,16 @@ not ok 61 - invalid subtest fail
   stack: |-
     *
   ...
-1..61
+1..63
 # Warning: Test "unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event.
 # Warning: Test "async unhandled rejection - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from async unhandled rejection fail" and would have caused the test to fail, but instead triggered an unhandledRejection event.
 # Warning: Test "immediate throw - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: thrown from immediate throw fail" and would have caused the test to fail, but instead triggered an uncaughtException event.
 # Warning: Test "immediate reject - passes but warns" generated asynchronous activity after the test ended. This activity created the error "Error: rejected from immediate reject fail" and would have caused the test to fail, but instead triggered an unhandledRejection event.
 # Warning: Test "callback called twice in different ticks" generated asynchronous activity after the test ended. This activity created the error "Error [ERR_TEST_FAILURE]: callback invoked multiple times" and would have caused the test to fail, but instead triggered an uncaughtException event.
 # Warning: Test "callback async throw after done" 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 61
-# pass 26
-# fail 18
+# tests 63
+# pass 27
+# fail 19
 # cancelled 2
 # skipped 10
 # todo 5

test/parallel/test-runner-concurrency.js

@@ -1,6 +1,6 @@
 'use strict';
-require('../common');
-const { describe, it } = require('node:test');
+const common = require('../common');
+const { describe, it, test } = require('node:test');
 const assert = require('assert');
 
 describe('Concurrency option (boolean) = true ', { concurrency: true }, () => {
@@ -27,3 +27,38 @@ describe(
     });
   }
 );
+
+{
+  // Make sure tests run in order when root concurrency is 1 (default)
+  const tree = [];
+  const expectedTestTree = common.mustCall(() => {
+    assert.deepStrictEqual(tree, [
+      'suite 1', 'nested', 'suite 2',
+      '1', '2', 'nested 1', 'nested 2',
+      'test', 'test 1', 'test 2',
+    ]);
+  });
+
+  describe('suite 1', () => {
+    tree.push('suite 1');
+    it('1', () => tree.push('1'));
+    it('2', () => tree.push('2'));
+
+    describe('nested', () => {
+      tree.push('nested');
+      it('nested 1', () => tree.push('nested 1'));
+      it('nested 2', () => tree.push('nested 2'));
+    });
+  });
+
+  test('test', async (t) => {
+    tree.push('test');
+    await t.test('test1', () => tree.push('test 1'));
+    await t.test('test 2', () => tree.push('test 2'));
+  });
+
+  describe('suite 2', () => {
+    tree.push('suite 2');
+    it('should run after other suites', expectedTestTree);
+  });
+}