fix

nodejs-github-bot · Nov 7, 2022 · Oct 27, 2022 · Nov 5, 2022 · Nov 5, 2022 · Nov 6, 2022
commit 30268cde9df3939ffec70cfb71334f85ed8bfe33
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -41,6 +41,7 @@ const {
 const {
   createDeferredPromise,
   kEmptyObject,
+  once: runOnce,
 } = require('internal/util');
 const { isPromise } = require('internal/util/types');
 const {
@@ -483,6 +484,11 @@ class Test extends AsyncResource {
     }
 
     const { args, ctx } = this.getRunArgs();
+    const afterEach = runOnce(async () => {
+      if (this.parent?.hooks.afterEach.length > 0) {
+        await this.parent[kRunHook]('afterEach', { args, ctx });
+      }
+    });
 
     try {
       if (this.parent?.hooks.beforeEach.length > 0) {
@@ -519,8 +525,10 @@ class Test extends AsyncResource {
         return;
       }
 
+      await afterEach();
       this.pass();
     } catch (err) {
+      await afterEach();
       if (isTestFailureError(err)) {
         if (err.failureType === kTestTimeoutFailure) {
           this.cancel(err);
@@ -530,12 +538,6 @@ class Test extends AsyncResource {
       } else {
         this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
       }
-    } finally {
-      const afterEach = this.parent?.hooks?.afterEach || [];
-      const shouldRunAfterEach = afterEach.length > 0;
-      if (shouldRunAfterEach) {
-        await this.parent[kRunHook]('afterEach', { args, ctx });
-      }
     }
 
     // Clean up the test. Then, try to report the results and execute any
@@ -731,6 +733,12 @@ class Suite extends Test {
   }
 
   async run() {
+    const hookArgs = this.getRunArgs();
+    const afterEach = runOnce(async () => {
+      if (this.parent?.hooks.afterEach.length > 0) {
+        await this.parent[kRunHook]('afterEach', hookArgs);
+      }
+    });
     try {
       this.parent.activeSubtests++;
       await this.buildSuite;
@@ -742,7 +750,6 @@ class Suite extends Test {
         return;
       }
 
-      const hookArgs = this.getRunArgs();
 
       if (this.parent?.hooks.beforeEach.length > 0) {
         await this.parent[kRunHook]('beforeEach', hookArgs);
@@ -756,13 +763,11 @@ class Suite extends Test {
 
       await SafePromiseRace([promise, stopPromise]);
       await this[kRunHook]('after', hookArgs);
-
-      if (this.parent?.hooks.afterEach.length > 0) {
-        await this.parent[kRunHook]('afterEach', hookArgs);
-      }
+      await afterEach();
 
       this.pass();
     } catch (err) {
+      await afterEach();
       if (isTestFailureError(err)) {
         this.fail(err);
       } else {

diff --git a/lib/internal/util.js b/lib/internal/util.js
@@ -453,7 +453,7 @@ function once(callback) {
   return function(...args) {
     if (called) return;
     called = true;
-    ReflectApply(callback, this, args);
+    return ReflectApply(callback, this, args);
   };
 }
 

diff --git a/test/message/test_runner_describe_it.out b/test/message/test_runner_describe_it.out
@@ -42,8 +42,6 @@ not ok 4 - sync fail todo with message # TODO this is a failing todo
     *
     *
     *
-    *
-    *
   ...
 # Subtest: sync skip pass
 ok 5 - sync skip pass # SKIP

diff --git a/test/message/test_runner_hooks.js b/test/message/test_runner_hooks.js
@@ -1,6 +1,6 @@
 // Flags: --no-warnings
 'use strict';
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const { test, describe, it, before, after, beforeEach, afterEach } = require('node:test');
 
@@ -76,6 +76,12 @@ describe('afterEach throws', () => {
   it('2', () => {});
 });
 
+describe('afterEach when test fails', () => {
+  afterEach(common.mustCall(2));
+  it('1', () => { throw new Error('test'); });
+  it('2', () => {});
+});
+
 test('test hooks', async (t) => {
   const testArr = [];
   t.beforeEach((t) => testArr.push('beforeEach ' + t.name));
@@ -111,3 +117,10 @@ test('t.afterEach throws', async (t) => {
   await t.test('1', () => {});
   await t.test('2', () => {});
 });
+
+
+test('afterEach when test fails', async (t) => {
+  t.afterEach(common.mustCall(2));
+  await t.test('1', () => { throw new Error('test'); });
+  await t.test('2', () => {});
+});
diff --git a/test/message/test_runner_hooks.out b/test/message/test_runner_hooks.out
@@ -189,6 +189,39 @@ not ok 5 - afterEach throws
   error: '2 subtests failed'
   code: 'ERR_TEST_FAILURE'
   ...
+# Subtest: afterEach when test fails
+    # Subtest: 1
+    not ok 1 - 1
+      ---
+      duration_ms: *
+      failureType: 'testCodeFailure'
+      error: 'test'
+      code: 'ERR_TEST_FAILURE'
+      stack: |-
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+      ...
+    # Subtest: 2
+    ok 2 - 2
+      ---
+      duration_ms: *
+      ...
+    1..2
+not ok 6 - afterEach when test fails
+  ---
+  duration_ms: *
+  failureType: 'subtestsFailed'
+  error: '1 subtest failed'
+  code: 'ERR_TEST_FAILURE'
+  ...
 # Subtest: test hooks
     # Subtest: 1
     ok 1 - 1
@@ -217,7 +250,7 @@ not ok 5 - afterEach throws
       duration_ms: *
       ...
     1..3
-ok 6 - test hooks
+ok 7 - test hooks
   ---
   duration_ms: *
   ...
@@ -261,7 +294,7 @@ ok 6 - test hooks
         *
       ...
     1..2
-not ok 7 - t.beforeEach throws
+not ok 8 - t.beforeEach throws
   ---
   duration_ms: *
   failureType: 'subtestsFailed'
@@ -308,17 +341,50 @@ not ok 7 - t.beforeEach throws
         *
       ...
     1..2
-not ok 8 - t.afterEach throws
+not ok 9 - t.afterEach throws
   ---
   duration_ms: *
   failureType: 'subtestsFailed'
   error: '2 subtests failed'
   code: 'ERR_TEST_FAILURE'
   ...
-1..8
-# tests 8
+# Subtest: afterEach when test fails
+    # Subtest: 1
+    not ok 1 - 1
+      ---
+      duration_ms: *
+      failureType: 'testCodeFailure'
+      error: 'test'
+      code: 'ERR_TEST_FAILURE'
+      stack: |-
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+      ...
+    # Subtest: 2
+    ok 2 - 2
+      ---
+      duration_ms: *
+      ...
+    1..2
+not ok 10 - afterEach when test fails
+  ---
+  duration_ms: *
+  failureType: 'subtestsFailed'
+  error: '1 subtest failed'
+  code: 'ERR_TEST_FAILURE'
+  ...
+1..10
+# tests 10
 # pass 2
-# fail 6
+# fail 8
 # cancelled 0
 # skipped 0
 # todo 0

diff --git a/test/message/test_runner_output.out b/test/message/test_runner_output.out
@@ -42,8 +42,6 @@ not ok 4 - sync fail todo with message # TODO this is a failing todo
     *
     *
     *
-    *
-    *
   ...
 # Subtest: sync skip pass
 ok 5 - sync skip pass # SKIP
-Original file line number
+Diff line change
@@ Expand Up @@
         *
         *
         *
-        *
-        *
       ...
     # Subtest: sync skip pass
     ok 5 - sync skip pass # SKIP
@@ Expand Down @@