test_runner: fix global after hook

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));
 

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,16 +526,7 @@ 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 });
-      }
-    });
+    const afterEach = runOnce(() => this.parent.runHook('afterEach', { args, ctx }));
 
     try {
       if (this.parent?.hooks.before.length > 0) {
@@ -548,7 +539,10 @@ class Test extends AsyncResource {
       const runArgs = ArrayPrototypeSlice(args);
       ArrayPrototypeUnshift(runArgs, this.fn, ctx);
 
-      if (this.fn.length === runArgs.length - 1) {
+      if (this.fn === noop && this.parent === null) {
+        // Avoid dealy of microtasks for root test
+        // to allow calling root.run() after event loop has ended
+      } else if (this.fn.length === runArgs.length - 1) {
         // This test is using legacy Node.js error first callbacks.
         const { promise, cb } = createDeferredCallback();
 
@@ -575,12 +569,20 @@ class Test extends AsyncResource {
         return;
       }
 
-      await afterEach();
-      await after();
+      if (this.parent?.hooks.afterEach.length > 0) {
+        await afterEach();
+      }
+      if (this.hooks.after.length > 0) {
+        await this.runHook('after', { args, ctx });
+      }
       this.pass();
     } catch (err) {
-      try { await after(); } catch { /* Ignore error. */ }
-      try { await afterEach(); } catch { /* test is already failing, let's ignore the error */ }
+      if (this.parent?.hooks.afterEach.length > 0) {
+        try { await afterEach(); } catch { /* test is already failing, let's ignore the error */ }
+      }
+      if (this.hooks.after.length > 0) {
+        try { await this.runHook('after', { args, ctx }); } catch { /* Ignore error. */ }
+      }
       if (isTestFailureError(err)) {
         if (err.failureType === kTestTimeoutFailure) {
           this.#cancel(err);
@@ -594,7 +596,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) {

test/fixtures/test-runner/output/describe_it.snapshot

@@ -40,6 +40,9 @@ not ok 4 - sync fail todo with message # TODO this is a failing todo
     *
     *
     *
+    *
+    *
+    *
   ...
 # Subtest: sync skip pass
 ok 5 - sync skip pass # SKIP
@@ -488,10 +491,10 @@ not ok 53 - custom inspect symbol that throws fail
         *
         *
         *
+        new Promise (<anonymous>)
         *
         *
-        *
-        async Promise.all (index 0)
+        Array.map (<anonymous>)
       ...
     1..2
 not ok 54 - subtest sync throw fails

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'));

test/fixtures/test-runner/output/hooks.snapshot

@@ -97,6 +97,7 @@ not ok 3 - after throws
     *
     *
     *
+    *
   ...
 # Subtest: beforeEach throws
     # Subtest: 1
@@ -162,9 +163,9 @@ not ok 4 - beforeEach throws
         *
         *
         *
-        *
         async Promise.all (index 0)
         *
+        *
       ...
     # Subtest: 2
     not ok 2 - 2
@@ -181,9 +182,9 @@ not ok 4 - beforeEach throws
         *
         *
         *
-        *
         async Promise.all (index 0)
         *
+        *
       ...
     1..2
 not ok 5 - afterEach throws
@@ -263,9 +264,9 @@ not ok 6 - afterEach when test fails
         *
         *
         *
-        *
         async Promise.all (index 0)
         *
+        *
       ...
     1..2
 not ok 7 - afterEach throws and test fails
@@ -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

test/fixtures/test-runner/output/output.snapshot

@@ -40,6 +40,9 @@ not ok 4 - sync fail todo with message # TODO this is a failing todo
     *
     *
     *
+    *
+    *
+    *
   ...
 # Subtest: sync skip pass
 ok 5 - sync skip pass # SKIP

test/fixtures/test-runner/output/output_cli.snapshot

@@ -40,6 +40,9 @@ not ok 4 - sync fail todo with message # TODO this is a failing todo
     *
     *
     *
+    *
+    *
+    *
   ...
 # Subtest: sync skip pass
 ok 5 - sync skip pass # SKIP

test/fixtures/test-runner/output/spec_reporter.snapshot

@@ -19,6 +19,9 @@
       *
       *
       *
+      *
+      *
+      *
 
  sync skip pass (*ms) # SKIP
  sync skip pass with message (*ms) # SKIP
@@ -326,6 +329,9 @@
       *
       *
       *
+      *
+      *
+      *
 
  sync throw fail (*ms)
   Error: thrown from sync throw fail

test/fixtures/test-runner/output/spec_reporter_cli.snapshot

@@ -19,6 +19,9 @@
       *
       *
       *
+      *
+      *
+      *
 
  sync skip pass (*ms) # SKIP
  sync skip pass with message (*ms) # SKIP
@@ -326,6 +329,9 @@
       *
       *
       *
+      *
+      *
+      *
 
  sync throw fail (*ms)
   Error: thrown from sync throw fail