From c281c155467bb44d3fbb9b63d2c1cece9f705f91 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Sun, 23 Jul 2023 13:22:35 +0300
Subject: [PATCH] test_runner: fix async callback in describe not awaited

PR-URL: https://github.com/nodejs/node/pull/48856
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
---
 lib/internal/test_runner/test.js              | 21 ++++++----
 .../test-runner/output/describe_it.js         | 19 +++++++++
 .../test-runner/output/describe_it.snapshot   | 39 ++++++++++++++++---
 3 files changed, 67 insertions(+), 12 deletions(-)

diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index 9a333e1457fbc0..becb4c07d7cf83 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -13,6 +13,7 @@ const {
   ObjectSeal,
   PromisePrototypeThen,
   PromiseResolve,
+  SafePromisePrototypeFinally,
   ReflectApply,
   RegExpPrototypeExec,
   SafeMap,
@@ -789,17 +790,23 @@ class Suite extends Test {
       const { ctx, args } = this.getRunArgs();
       const runArgs = [this.fn, ctx];
       ArrayPrototypePushApply(runArgs, args);
-      this.buildSuite = PromisePrototypeThen(
-        PromiseResolve(ReflectApply(this.runInAsyncScope, this, runArgs)),
-        undefined,
-        (err) => {
-          this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
-        });
+      this.buildSuite = SafePromisePrototypeFinally(
+        PromisePrototypeThen(
+          PromiseResolve(ReflectApply(this.runInAsyncScope, this, runArgs)),
+          undefined,
+          (err) => {
+            this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
+          }),
+        () => {
+          this.buildPhaseFinished = true;
+        },
+      );
     } catch (err) {
       this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
+
+      this.buildPhaseFinished = true;
     }
     this.fn = () => {};
-    this.buildPhaseFinished = true;
   }
 
   getRunArgs() {
diff --git a/test/fixtures/test-runner/output/describe_it.js b/test/fixtures/test-runner/output/describe_it.js
index c6d3f9c1b72fb3..0b89e1a11112b1 100644
--- a/test/fixtures/test-runner/output/describe_it.js
+++ b/test/fixtures/test-runner/output/describe_it.js
@@ -375,3 +375,22 @@ describe('rejected thenable', () => {
     },
   };
 });
+
+describe("async describe function", async () => {
+  await null;
+
+  await it("it inside describe 1", async () => {
+    await null
+  });
+  await it("it inside describe 2", async () => {
+    await null;
+  });
+
+  describe("inner describe", async () => {
+    await null;
+
+    it("it inside inner describe", async () => {
+      await null;
+    });
+  });
+});
diff --git a/test/fixtures/test-runner/output/describe_it.snapshot b/test/fixtures/test-runner/output/describe_it.snapshot
index e085ff9535ec70..c5b4610d522e43 100644
--- a/test/fixtures/test-runner/output/describe_it.snapshot
+++ b/test/fixtures/test-runner/output/describe_it.snapshot
@@ -635,8 +635,37 @@ not ok 59 - rejected thenable
   stack: |-
     *
   ...
+# Subtest: async describe function
+    # Subtest: it inside describe 1
+    ok 1 - it inside describe 1
+      ---
+      duration_ms: *
+      ...
+    # Subtest: it inside describe 2
+    ok 2 - it inside describe 2
+      ---
+      duration_ms: *
+      ...
+    # Subtest: inner describe
+        # Subtest: it inside inner describe
+        ok 1 - it inside inner describe
+          ---
+          duration_ms: *
+          ...
+        1..1
+    ok 3 - inner describe
+      ---
+      duration_ms: *
+      type: 'suite'
+      ...
+    1..3
+ok 60 - async describe function
+  ---
+  duration_ms: *
+  type: 'suite'
+  ...
 # Subtest: invalid subtest fail
-not ok 60 - invalid subtest fail
+not ok 61 - invalid subtest fail
   ---
   duration_ms: *
   failureType: 'parentAlreadyFinished'
@@ -645,16 +674,16 @@ not ok 60 - invalid subtest fail
   stack: |-
     *
   ...
-1..60
+1..61
 # 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 67
-# suites 9
-# pass 29
+# tests 70
+# suites 11
+# pass 32
 # fail 19
 # cancelled 4
 # skipped 10