From 4737314865c979d8c585a5aff8415f40c60c1802 Mon Sep 17 00:00:00 2001
From: Phil Nash <phillip.nash@sonarsource.com>
Date: Thu, 11 May 2023 20:10:56 +1000
Subject: [PATCH] test_runner: fix ordering of test hooks

For tests with subtests the before hook was being run after the
beforeEach hook, which is the opposite to test suites and expectations.

Also, a function was being used to close over the after hooks, but at
the point it was being run the after hooks were not yet set up.

Fixes #47915

PR-URL: https://github.com/nodejs/node/pull/47931
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 lib/internal/test_runner/test.js          | 8 ++++----
 test/fixtures/test-runner/output/hooks.js | 7 ++++++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index 9f82bf6b361544..2e241d9f0eef7b 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -537,12 +537,12 @@ class Test extends AsyncResource {
     });
 
     try {
-      if (this.parent?.hooks.beforeEach.length > 0) {
-        await this.parent.runHook('beforeEach', { args, ctx });
-      }
       if (this.parent?.hooks.before.length > 0) {
         await this.parent.runHook('before', this.parent.getRunArgs());
       }
+      if (this.parent?.hooks.beforeEach.length > 0) {
+        await this.parent.runHook('beforeEach', { args, ctx });
+      }
       const stopPromise = stopTest(this.timeout, this.signal);
       const runArgs = ArrayPrototypeSlice(args);
       ArrayPrototypeUnshift(runArgs, this.fn, ctx);
@@ -574,8 +574,8 @@ class Test extends AsyncResource {
         return;
       }
 
-      await after();
       await afterEach();
+      await after();
       this.pass();
     } catch (err) {
       try { await after(); } catch { /* Ignore error. */ }
diff --git a/test/fixtures/test-runner/output/hooks.js b/test/fixtures/test-runner/output/hooks.js
index 30532a29ad69f4..a69506bbda5ef7 100644
--- a/test/fixtures/test-runner/output/hooks.js
+++ b/test/fixtures/test-runner/output/hooks.js
@@ -99,6 +99,8 @@ test('test hooks', async (t) => {
   await t.test('2', () => testArr.push('2'));
 
   await t.test('nested', async (t) => {
+    t.before((t) => testArr.push('nested before ' + t.name));
+    t.after((t) => testArr.push('nested after ' + t.name));
     t.beforeEach((t) => testArr.push('nested beforeEach ' + t.name));
     t.afterEach((t) => testArr.push('nested afterEach ' + t.name));
     await t.test('nested 1', () => testArr.push('nested1'));
@@ -106,12 +108,15 @@ test('test hooks', async (t) => {
   });
 
   assert.deepStrictEqual(testArr, [
-    'beforeEach 1', 'before test hooks', '1', 'afterEach 1',
+    'before test hooks',
+    'beforeEach 1', '1', 'afterEach 1',
     'beforeEach 2', '2', 'afterEach 2',
     'beforeEach nested',
+    'nested before nested',
     'beforeEach nested 1', 'nested beforeEach nested 1', 'nested1', 'afterEach nested 1', 'nested afterEach nested 1',
     'beforeEach nested 2', 'nested beforeEach nested 2', 'nested 2', 'afterEach nested 2', 'nested afterEach nested 2',
     'afterEach nested',
+    'nested after nested',
   ]);
 });