test_runner: fixed to run after hook if before throws an error

PR-URL: nodejs#51062 Refs: nodejs#50842 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Raz Luvaton <[email protected]>
ofrobots · Jan 14, 2024 · a814b8a · a814b8a
1 parent 7c4b189
commit a814b8a
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 8 deletions.
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -924,6 +924,7 @@ class Suite extends Test {
     const hookArgs = this.getRunArgs();
 
     let stopPromise;
+    const after = runOnce(() => this.runHook('after', hookArgs));
     try {
       this.parent.activeSubtests++;
       await this.buildSuite;
@@ -946,10 +947,11 @@ class Suite extends Test {
       const promise = SafePromiseAll(subtests, (subtests) => subtests.start());
 
       await SafePromiseRace([promise, stopPromise]);
-      await this.runHook('after', hookArgs);
+      await after();
 
       this.pass();
     } catch (err) {
+      try { await after(); } catch { /* suite is already failing */ }
       if (isTestFailureError(err)) {
         this.fail(err);
       } else {

diff --git a/test/fixtures/test-runner/output/hooks.js b/test/fixtures/test-runner/output/hooks.js
@@ -166,5 +166,13 @@ test('t.after() is called if test body throws', (t) => {
   throw new Error('bye');
 });
 
+describe('run after when before throws', () => {
+  after(common.mustCall(() => {
+    console.log("- after() called")
+  }));
+  before(() => { throw new Error('before')});
+  it('1', () => {});
+});
+
 before((t) => t.diagnostic('before 2 called'));
 after((t) => t.diagnostic('after 2 called'));
diff --git a/test/fixtures/test-runner/output/hooks.snapshot b/test/fixtures/test-runner/output/hooks.snapshot
@@ -1,3 +1,4 @@
+- after() called
 TAP version 13
 # Subtest: describe hooks
     # Subtest: 1
@@ -568,16 +569,46 @@ not ok 14 - t.after() is called if test body throws
     *
   ...
 # - after() called
-1..14
+# Subtest: run after when before throws
+    # Subtest: 1
+    not ok 1 - 1
+      ---
+      duration_ms: *
+      location: '/test/fixtures/test-runner/output/hooks.js:(LINE):3'
+      failureType: 'cancelledByParent'
+      error: 'test did not finish before its parent and was cancelled'
+      code: 'ERR_TEST_FAILURE'
+      ...
+    1..1
+not ok 15 - run after when before throws
+  ---
+  duration_ms: *
+  type: 'suite'
+  location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
+  failureType: 'hookFailed'
+  error: 'before'
+  code: 'ERR_TEST_FAILURE'
+  stack: |-
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+  ...
+1..15
 # before 1 called
 # before 2 called
 # after 1 called
 # after 2 called
-# tests 38
-# suites 8
+# tests 39
+# suites 9
 # pass 14
 # fail 22
-# cancelled 2
+# cancelled 3
 # skipped 0
 # todo 0
 # duration_ms *
diff --git a/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot b/test/fixtures/test-runner/output/hooks_spec_reporter.snapshot
@@ -1,3 +1,4 @@
+- after() called
  describe hooks
    1 (*ms)
    2 (*ms)
@@ -293,15 +294,32 @@
       *
 
  - after() called
+ run after when before throws
+   1
+    'test did not finish before its parent and was cancelled'
+
+ run after when before throws (*ms)
+
+  Error: before
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+
  before 1 called
  before 2 called
  after 1 called
  after 2 called
- tests 38
- suites 8
+ tests 39
+ suites 9
  pass 14
  fail 22
- cancelled 2
+ cancelled 3
  skipped 0
  todo 0
  duration_ms *
@@ -567,3 +585,20 @@
       *
       *
       *
+
+*
+ 1
+  'test did not finish before its parent and was cancelled'
+
+*
+ run after when before throws (*ms)
+  Error: before
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *
+      *