From 67f713731ca8c978b873161513e0a72bb59053b1 Mon Sep 17 00:00:00 2001
From: Colin Ihrig <cjihrig@gmail.com>
Date: Sun, 4 Aug 2024 11:56:04 -0400
Subject: [PATCH] test_runner: run after hooks even if test is aborted

If a test is run, but aborted, any after hooks should still be
run, as they may need to perform cleanup.

PR-URL: https://github.com/nodejs/node/pull/54151
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Raz Luvaton <rluvaton@gmail.com>
---
 lib/internal/test_runner/test.js              | 10 ++------
 .../output/abort-runs-after-hook.js           | 14 +++++++++++
 .../output/abort-runs-after-hook.snapshot     | 23 +++++++++++++++++++
 test/parallel/test-runner-output.mjs          |  1 +
 4 files changed, 40 insertions(+), 8 deletions(-)
 create mode 100644 test/fixtures/test-runner/output/abort-runs-after-hook.js
 create mode 100644 test/fixtures/test-runner/output/abort-runs-after-hook.snapshot

diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index 1c461a07f16bfa..bc29d9204552e1 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -763,10 +763,7 @@ class Test extends AsyncResource {
   }
 
   [kShouldAbort]() {
-    if (this.signal.aborted) {
-      return true;
-    }
-    if (this.outerSignal?.aborted) {
+    if (this.signal.aborted || this.outerSignal?.aborted) {
       this.#abortHandler();
       return true;
     }
@@ -866,10 +863,7 @@ class Test extends AsyncResource {
         await SafePromiseRace([PromiseResolve(promise), stopPromise]);
       }
 
-      if (this[kShouldAbort]()) {
-        this.postRun();
-        return;
-      }
+      this[kShouldAbort]();
       this.plan?.check();
       this.pass();
       await afterEach();
diff --git a/test/fixtures/test-runner/output/abort-runs-after-hook.js b/test/fixtures/test-runner/output/abort-runs-after-hook.js
new file mode 100644
index 00000000000000..693ea535ca60ba
--- /dev/null
+++ b/test/fixtures/test-runner/output/abort-runs-after-hook.js
@@ -0,0 +1,14 @@
+'use strict';
+const { test } = require('node:test');
+
+test('test that aborts', (t, done) => {
+  t.after(() => {
+    // This should still run.
+    console.log('AFTER');
+  });
+
+  setImmediate(() => {
+    // This creates an uncaughtException, which aborts the test.
+    throw new Error('boom');
+  });
+});
diff --git a/test/fixtures/test-runner/output/abort-runs-after-hook.snapshot b/test/fixtures/test-runner/output/abort-runs-after-hook.snapshot
new file mode 100644
index 00000000000000..c734a264b0f07f
--- /dev/null
+++ b/test/fixtures/test-runner/output/abort-runs-after-hook.snapshot
@@ -0,0 +1,23 @@
+TAP version 13
+AFTER
+# Subtest: test that aborts
+not ok 1 - test that aborts
+  ---
+  duration_ms: *
+  location: '/test/fixtures/test-runner/output/abort-runs-after-hook.js:(LINE):1'
+  failureType: 'uncaughtException'
+  error: 'boom'
+  code: 'ERR_TEST_FAILURE'
+  stack: |-
+    *
+    *
+  ...
+1..1
+# tests 1
+# suites 0
+# pass 0
+# fail 1
+# cancelled 0
+# skipped 0
+# todo 0
+# duration_ms *
diff --git a/test/parallel/test-runner-output.mjs b/test/parallel/test-runner-output.mjs
index 676168e9df4e50..da08cb1b146b3a 100644
--- a/test/parallel/test-runner-output.mjs
+++ b/test/parallel/test-runner-output.mjs
@@ -93,6 +93,7 @@ const lcovTransform = snapshot.transform(
 
 const tests = [
   { name: 'test-runner/output/abort.js' },
+  { name: 'test-runner/output/abort-runs-after-hook.js' },
   { name: 'test-runner/output/abort_suite.js' },
   { name: 'test-runner/output/abort_hooks.js' },
   { name: 'test-runner/output/describe_it.js' },