test_runner: fix global after hook

PR-URL: #48231 Fixes: #48230 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
nodejs · Jun 4, 2023 · f3268d6 · f3268d6
1 parent 5ddca72
commit f3268d6
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 21 deletions.
diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
@@ -140,8 +140,8 @@ function setup(root) {
   const rejectionHandler =
     createProcessEventHandler('unhandledRejection', root);
   const coverage = configureCoverage(root, globalOptions);
-  const exitHandler = () => {
-    root.postRun(new ERR_TEST_FAILURE(
+  const exitHandler = async () => {
+    await root.run(new ERR_TEST_FAILURE(
       'Promise resolution is still pending but the event loop has already resolved',
       kCancelledByParent));
 
@@ -150,8 +150,8 @@ function setup(root) {
     process.removeListener('uncaughtException', exceptionHandler);
   };
 
-  const terminationHandler = () => {
-    exitHandler();
+  const terminationHandler = async () => {
+    await exitHandler();
     process.exit();
   };
 

diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -427,7 +427,7 @@ class Test extends AsyncResource {
     validateOneOf(name, 'hook name', kHookNames);
     // eslint-disable-next-line no-use-before-define
     const hook = new TestHook(fn, options);
-    if (name === 'before') {
+    if (name === 'before' || name === 'after') {
       hook.run = runOnce(hook.run);
     }
     ArrayPrototypePush(this.hooks[name], hook);
@@ -514,7 +514,7 @@ class Test extends AsyncResource {
     }
   }
 
-  async run() {
+  async run(pendingSubtestsError) {
     if (this.parent !== null) {
       this.parent.activeSubtests++;
     }
@@ -526,11 +526,11 @@ class Test extends AsyncResource {
     }
 
     const { args, ctx } = this.getRunArgs();
-    const after = runOnce(async () => {
+    const after = 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 });
@@ -579,8 +579,8 @@ class Test extends AsyncResource {
       await after();
       this.pass();
     } catch (err) {
-      try { await after(); } catch { /* Ignore error. */ }
       try { await afterEach(); } catch { /* test is already failing, let's ignore the error */ }
+      try { await after(); } catch { /* Ignore error. */ }
       if (isTestFailureError(err)) {
         if (err.failureType === kTestTimeoutFailure) {
           this.#cancel(err);
@@ -594,7 +594,7 @@ class Test extends AsyncResource {
 
     // Clean up the test. Then, try to report the results and execute any
     // tests that were pending due to available concurrency.
-    this.postRun();
+    this.postRun(pendingSubtestsError);
   }
 
   postRun(pendingSubtestsError) {

diff --git a/test/fixtures/test-runner/output/hooks.js b/test/fixtures/test-runner/output/hooks.js
@@ -5,6 +5,7 @@ const assert = require('assert');
 const { test, describe, it, before, after, beforeEach, afterEach } = require('node:test');
 
 before((t) => t.diagnostic('before 1 called'));
+after((t) => t.diagnostic('after 1 called'));
 
 describe('describe hooks', () => {
   const testArr = [];
@@ -107,17 +108,20 @@ test('test hooks', async (t) => {
     await t.test('nested 2', () => testArr.push('nested 2'));
   });
 
-  assert.deepStrictEqual(testArr, [
-    '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',
-  ]);
+  t.after(common.mustCall(() => {
+    assert.deepStrictEqual(testArr, [
+      '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',
+      'after test hooks',
+    ]);
+  }));
 });
 
 test('t.before throws', async (t) => {
@@ -164,3 +168,4 @@ test('t.after() is called if test body throws', (t) => {
 });
 
 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
@@ -97,6 +97,7 @@ not ok 3 - after throws
     *
     *
     *
+    *
   ...
 # Subtest: beforeEach throws
     # Subtest: 1
@@ -544,6 +545,8 @@ not ok 14 - t.after() is called if test body throws
 1..14
 # before 1 called
 # before 2 called
+# after 1 called
+# after 2 called
 # tests 38
 # suites 8
 # pass 14