test_runner: preserve hook promise when executed twice

Author: MoLow

lib/internal/test_runner/test.js

@@ -567,7 +567,7 @@ class Test extends AsyncResource {
     // eslint-disable-next-line no-use-before-define
     const hook = new TestHook(fn, options);
     if (name === 'before' || name === 'after') {
-      hook.run = runOnce(hook.run);
+      hook.run = runOnce(hook.run, true);
     }
     if (name === 'before' && this.startTime !== null) {
       // Test has already started, run the hook immediately
@@ -710,7 +710,7 @@ class Test extends AsyncResource {
       if (this.parent?.hooks.afterEach.length > 0 && !this.skipped) {
         await this.parent.runHook('afterEach', hookArgs);
       }
-    });
+    }, true);
 
     let stopPromise;
 
@@ -1081,7 +1081,7 @@ class Suite extends Test {
     const hookArgs = this.getRunArgs();
 
     let stopPromise;
-    const after = runOnce(() => this.runHook('after', hookArgs));
+    const after = runOnce(() => this.runHook('after', hookArgs), true);
     try {
       this.parent.activeSubtests++;
       await this.buildSuite;

lib/internal/util.js

@@ -514,12 +514,15 @@ function isInsideNodeModules() {
   return false;
 }
 
-function once(callback) {
+function once(callback, preserveReturnValue = false) {
   let called = false;
+  let returnValue;
   return function(...args) {
-    if (called) return;
+    if (called) return returnValue;
     called = true;
-    return ReflectApply(callback, this, args);
+    const result = ReflectApply(callback, this, args);
+    returnValue = preserveReturnValue ? result : undefined;
+    return result;
   };
 }
 

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

@@ -2,6 +2,7 @@
 const common = require('../../../common');
 const assert = require('assert');
 const { test, describe, it, before, after, beforeEach, afterEach } = require('node:test');
+const { setTimeout } = require('node:timers/promises');
 
 before((t) => t.diagnostic('before 1 called'));
 after((t) => t.diagnostic('after 1 called'));
@@ -149,6 +150,55 @@ test('test hooks', async (t) => {
 });
 
 
+test('test hooks - async', async (t) => {
+  const testArr = [];
+
+  t.before(async (t) => {
+    testArr.push('before starting ' + t.name);
+    await setTimeout(10);
+    testArr.push('before ending ' + t.name);
+  });
+  t.after(async (t) => {
+    testArr.push('after starting ' + t.name);
+    await setTimeout(10);
+    testArr.push('after ending ' + t.name);
+  });
+  t.beforeEach(async (t) => {
+    testArr.push('beforeEach starting ' + t.name);
+    await setTimeout(10);
+    testArr.push('beforeEach ending ' + t.name);
+  });
+  t.afterEach(async (t) => {
+    testArr.push('afterEach starting ' + t.name);
+    await setTimeout(10);
+    testArr.push('afterEach ending ' + t.name);
+  });
+  await t.test('1', async () => {
+    testArr.push('1 starting');
+    await setTimeout(10);
+    testArr.push('1 ending');
+  });
+  await t.test('2', async () => {
+    testArr.push('2 starting');
+    await setTimeout(10);
+    testArr.push('2 ending');
+  });
+
+  t.after(common.mustCall(() => {
+    assert.deepStrictEqual(testArr, [
+      'before starting test hooks - async', 'before ending test hooks - async',
+      'beforeEach starting 1', 'beforeEach ending 1',
+      '1 starting', '1 ending',
+      'afterEach starting 1', 'afterEach ending 1',
+      'beforeEach starting 2', 'beforeEach ending 2',
+      '2 starting', '2 ending',
+      'afterEach starting 2', 'afterEach ending 2',
+      'after starting test hooks - async', 'after ending test hooks - async',
+    ]);
+  }));
+});
+
+
 test('test hooks - no subtests', async (t) => {
   const testArr = [];
 

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

@@ -1,4 +1,3 @@
-- after() called
 TAP version 13
 # Subtest: describe hooks
     # Subtest: 1
@@ -367,8 +366,25 @@ ok 11 - test hooks
   ---
   duration_ms: *
   ...
+# Subtest: test hooks - async
+    # Subtest: 1
+    ok 1 - 1
+      ---
+      duration_ms: *
+      ...
+    # Subtest: 2
+    ok 2 - 2
+      ---
+      duration_ms: *
+      ...
+- after() called
+    1..2
+ok 12 - test hooks - async
+  ---
+  duration_ms: *
+  ...
 # Subtest: test hooks - no subtests
-ok 12 - test hooks - no subtests
+ok 13 - test hooks - no subtests
   ---
   duration_ms: *
   ...
@@ -414,7 +430,7 @@ ok 12 - test hooks - no subtests
         *
       ...
     1..2
-not ok 13 - t.before throws
+not ok 14 - t.before throws
   ---
   duration_ms: *
   location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -434,7 +450,7 @@ not ok 13 - t.before throws
     *
   ...
 # Subtest: t.before throws - no subtests
-not ok 14 - t.before throws - no subtests
+not ok 15 - t.before throws - no subtests
   ---
   duration_ms: *
   location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -465,7 +481,7 @@ not ok 14 - t.before throws - no subtests
       duration_ms: *
       ...
     1..2
-not ok 15 - t.after throws
+not ok 16 - t.after throws
   ---
   duration_ms: *
   location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -485,7 +501,7 @@ not ok 15 - t.after throws
     *
   ...
 # Subtest: t.after throws - no subtests
-not ok 16 - t.after throws - no subtests
+not ok 17 - t.after throws - no subtests
   ---
   duration_ms: *
   location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -546,7 +562,7 @@ not ok 16 - t.after throws - no subtests
         *
       ...
     1..2
-not ok 17 - t.beforeEach throws
+not ok 18 - t.beforeEach throws
   ---
   duration_ms: *
   location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -596,7 +612,7 @@ not ok 17 - t.beforeEach throws
         *
       ...
     1..2
-not ok 18 - t.afterEach throws
+not ok 19 - t.afterEach throws
   ---
   duration_ms: *
   location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -630,7 +646,7 @@ not ok 18 - t.afterEach throws
       duration_ms: *
       ...
     1..2
-not ok 19 - afterEach when test fails
+not ok 20 - afterEach when test fails
   ---
   duration_ms: *
   location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -645,7 +661,7 @@ not ok 19 - afterEach when test fails
       duration_ms: *
       ...
     1..1
-ok 20 - afterEach context when test passes
+ok 21 - afterEach context when test passes
   ---
   duration_ms: *
   ...
@@ -665,7 +681,7 @@ ok 20 - afterEach context when test passes
         *
       ...
     1..1
-not ok 21 - afterEach context when test fails
+not ok 22 - afterEach context when test fails
   ---
   duration_ms: *
   location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -714,7 +730,7 @@ not ok 21 - afterEach context when test fails
         *
       ...
     1..2
-not ok 22 - afterEach throws and test fails
+not ok 23 - afterEach throws and test fails
   ---
   duration_ms: *
   location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -723,7 +739,7 @@ not ok 22 - afterEach throws and test fails
   code: 'ERR_TEST_FAILURE'
   ...
 # Subtest: t.after() is called if test body throws
-not ok 23 - t.after() is called if test body throws
+not ok 24 - t.after() is called if test body throws
   ---
   duration_ms: *
   location: '/test/fixtures/test-runner/output/hooks.js:(LINE):1'
@@ -748,7 +764,7 @@ not ok 23 - t.after() is called if test body throws
       code: 'ERR_TEST_FAILURE'
       ...
     1..1
-not ok 24 - run after when before throws
+not ok 25 - run after when before throws
   ---
   duration_ms: *
   type: 'suite'
@@ -767,14 +783,14 @@ not ok 24 - run after when before throws
     *
     *
   ...
-1..24
+1..25
 # before 1 called
 # before 2 called
 # after 1 called
 # after 2 called
-# tests 49
+# tests 52
 # suites 12
-# pass 19
+# pass 22
 # fail 27
 # cancelled 3
 # skipped 0

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

@@ -1,4 +1,3 @@
-- after() called
  describe hooks
    1 (*ms)
    2 (*ms)
@@ -172,6 +171,11 @@
      nested 2 (*ms)
    nested (*ms)
  test hooks (*ms)
+ test hooks - async
+   1 (*ms)
+   2 (*ms)
+- after() called
+ test hooks - async (*ms)
  test hooks - no subtests (*ms)
  t.before throws
    1 (*ms)
@@ -396,9 +400,9 @@
  before 2 called
  after 1 called
  after 2 called
- tests 49
+ tests 52
  suites 12
- pass 19
+ pass 22
  fail 27
  cancelled 3
  skipped 0