fix: run t.after() if test body throws

This commit fixes a bug where t.after() was not called if the test body threw an exception. PR-URL: nodejs/node#45870 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> (cherry picked from commit 385d595a4f1d887f6d4221e6071571132498d57c)
MoLow · Feb 2, 2023 · c80e426 · c80e426
1 parent d1343a7
commit c80e426
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 6 deletions.
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -1,4 +1,4 @@
-// https://github.com/nodejs/node/blob/232efb06fe8787e9573e298ce7ac293ad23b7684/lib/internal/test_runner/test.js
+// https://github.com/nodejs/node/blob/385d595a4f1d887f6d4221e6071571132498d57c/lib/internal/test_runner/test.js
 
 'use strict'
 
@@ -496,6 +496,11 @@ class Test extends AsyncResource {
     }
 
     const { args, ctx } = this.getRunArgs()
+    const after = runOnce(async () => {
+      if (this.hooks.after.length > 0) {
+        await this.runHook('after', { args, ctx })
+      }
+    })
     const afterEach = runOnce(async () => {
       if (this.parent?.hooks.afterEach.length > 0) {
         await this.parent.runHook('afterEach', { args, ctx })
@@ -537,10 +542,11 @@ class Test extends AsyncResource {
         return
       }
 
-      await this.runHook('after', { args, ctx })
+      await after()
       await afterEach()
       this.pass()
     } catch (err) {
+      try { await after() } catch { /* Ignore error. */ }
       try { await afterEach() } catch { /* test is already failing, let's the error */ }
       if (isTestFailureError(err)) {
         if (err.failureType === kTestTimeoutFailure) {

diff --git a/test/message/test_runner_hooks.js b/test/message/test_runner_hooks.js
@@ -1,4 +1,4 @@
-// https://github.com/nodejs/node/blob/215c5317d4837287fddb2e3b97872babd53183ac/test/message/test_runner_hooks.js
+// https://github.com/nodejs/node/blob/385d595a4f1d887f6d4221e6071571132498d57c/test/message/test_runner_hooks.js
 // Flags: --no-warnings
 'use strict'
 const common = require('../common')
@@ -142,3 +142,10 @@ test('afterEach throws and test fails', async (t) => {
   await t.test('1', () => { throw new Error('test') })
   await t.test('2', () => {})
 })
+
+test('t.after() is called if test body throws', (t) => {
+  t.after(() => {
+    t.diagnostic('- after() called')
+  })
+  throw new Error('bye')
+})
diff --git a/test/message/test_runner_hooks.out b/test/message/test_runner_hooks.out
@@ -475,10 +475,28 @@ not ok 12 - afterEach throws and test fails
   error: '2 subtests failed'
   code: 'ERR_TEST_FAILURE'
   ...
-1..12
-# tests 12
+# Subtest: t.after() is called if test body throws
+not ok 13 - t.after() is called if test body throws
+  ---
+  duration_ms: *
+  failureType: 'testCodeFailure'
+  error: 'bye'
+  code: 'ERR_TEST_FAILURE'
+  stack: |-
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+  ...
+# - after() called
+1..13
+# tests 13
 # pass 2
-# fail 10
+# fail 11
 # cancelled 0
 # skipped 0
 # todo 0