test_runner: fix timeout in *Each hook failing further tests

PR-URL: #48925 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
nodejs · Jul 31, 2023 · 88094bb · 88094bb
1 parent 217abd4
commit 88094bb
Show file tree

Hide file tree

Showing 7 changed files with 385 additions and 15 deletions.
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -193,8 +193,8 @@ class SuiteContext {
 }
 
 class Test extends AsyncResource {
-  #abortController;
-  #outerSignal;
+  abortController;
+  outerSignal;
   #reportedSubtest;
 
   constructor(options) {
@@ -292,16 +292,16 @@ class Test extends AsyncResource {
       fn = noop;
     }
 
-    this.#abortController = new AbortController();
-    this.#outerSignal = signal;
-    this.signal = this.#abortController.signal;
+    this.abortController = new AbortController();
+    this.outerSignal = signal;
+    this.signal = this.abortController.signal;
 
     validateAbortSignal(signal, 'options.signal');
     if (signal) {
       kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation;
     }
 
-    this.#outerSignal?.addEventListener(
+    this.outerSignal?.addEventListener(
       'abort',
       this.#abortHandler,
       { __proto__: null, [kResistStopPropagation]: true },
@@ -441,7 +441,7 @@ class Test extends AsyncResource {
   }
 
   #abortHandler = () => {
-    const error = this.#outerSignal?.reason || new AbortError('The test was aborted');
+    const error = this.outerSignal?.reason || new AbortError('The test was aborted');
     error.failureType = kAborted;
     this.#cancel(error);
   };
@@ -459,7 +459,7 @@ class Test extends AsyncResource {
     );
     this.startTime = this.startTime || this.endTime; // If a test was canceled before it was started, e.g inside a hook
     this.cancelled = true;
-    this.#abortController.abort();
+    this.abortController.abort();
   }
 
   createHook(name, fn, options) {
@@ -527,7 +527,7 @@ class Test extends AsyncResource {
     if (this.signal.aborted) {
       return true;
     }
-    if (this.#outerSignal?.aborted) {
+    if (this.outerSignal?.aborted) {
       this.#abortHandler();
       return true;
     }
@@ -639,7 +639,7 @@ class Test extends AsyncResource {
       // Do not abort hooks and the root test as hooks instance are shared between tests suite so aborting them will
       // cause them to not run for further tests.
       if (this.parent !== null) {
-        this.#abortController.abort();
+        this.abortController.abort();
       }
     }
 
@@ -679,7 +679,7 @@ class Test extends AsyncResource {
       this.fail(new ERR_TEST_FAILURE(msg, kSubtestsFailed));
     }
 
-    this.#outerSignal?.removeEventListener('abort', this.#abortHandler);
+    this.outerSignal?.removeEventListener('abort', this.#abortHandler);
     this.mock?.reset();
 
     if (this.parent !== null) {
@@ -795,6 +795,14 @@ class TestHook extends Test {
     super({ __proto__: null, fn, timeout, signal });
   }
   run(args) {
+    if (this.error && !this.outerSignal?.aborted) {
+      this.passed = false;
+      this.error = null;
+      this.abortController.abort();
+      this.abortController = new AbortController();
+      this.signal = this.abortController.signal;
+    }
+
     this.#args = args;
     return super.run();
   }

diff --git a/test/fixtures/test-runner/output/abort_hooks.js b/test/fixtures/test-runner/output/abort_hooks.js
@@ -0,0 +1,63 @@
+// Flags: --no-warnings
+'use strict';
+const { before, beforeEach, describe, it, after, afterEach } = require('node:test');
+
+describe('1 before describe', () => {
+  const ac = new AbortController();
+  before(() => {
+    console.log('before');
+    ac.abort()
+  }, {signal: ac.signal});
+
+  it('test 1', () => {
+    console.log('1.1');
+  });
+  it('test 2', () => {
+    console.log('1.2');
+  });
+});
+
+describe('2 after describe', () => {
+  const ac = new AbortController();
+  after(() => {
+    console.log('after');
+    ac.abort()
+  }, {signal: ac.signal});
+
+  it('test 1', () => {
+    console.log('2.1');
+  });
+  it('test 2', () => {
+    console.log('2.2');
+  });
+});
+
+describe('3 beforeEach describe', () => {
+  const ac = new AbortController();
+  beforeEach(() => {
+    console.log('beforeEach');
+    ac.abort()
+  }, {signal: ac.signal});
+
+  it('test 1', () => {
+    console.log('3.1');
+  });
+  it('test 2', () => {
+    console.log('3.2');
+  });
+});
+
+describe('4 afterEach describe', () => {
+  const ac = new AbortController();
+  afterEach(() => {
+    console.log('afterEach');
+    ac.abort()
+  }, {signal: ac.signal});
+
+  it('test 1', () => {
+    console.log('4.1');
+  });
+  it('test 2', () => {
+    console.log('4.2');
+  });
+});
diff --git a/test/fixtures/test-runner/output/abort_hooks.snapshot b/test/fixtures/test-runner/output/abort_hooks.snapshot
@@ -0,0 +1,188 @@
+before
+2.1
+2.2
+after
+beforeEach
+4.1
+afterEach
+4.2
+TAP version 13
+# Subtest: 1 before describe
+    # Subtest: test 1
+    not ok 1 - test 1
+      ---
+      duration_ms: ZERO
+      failureType: 'cancelledByParent'
+      error: 'test did not finish before its parent and was cancelled'
+      code: 'ERR_TEST_FAILURE'
+      ...
+    # Subtest: test 2
+    not ok 2 - test 2
+      ---
+      duration_ms: ZERO
+      failureType: 'cancelledByParent'
+      error: 'test did not finish before its parent and was cancelled'
+      code: 'ERR_TEST_FAILURE'
+      ...
+    1..2
+not ok 1 - 1 before describe
+  ---
+  duration_ms: *
+  type: 'suite'
+  failureType: 'hookFailed'
+  error: 'This operation was aborted'
+  code: 20
+  name: 'AbortError'
+  stack: |-
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+  ...
+# Subtest: 2 after describe
+    # Subtest: test 1
+    ok 1 - test 1
+      ---
+      duration_ms: *
+      ...
+    # Subtest: test 2
+    ok 2 - test 2
+      ---
+      duration_ms: *
+      ...
+    1..2
+not ok 2 - 2 after describe
+  ---
+  duration_ms: *
+  type: 'suite'
+  failureType: 'hookFailed'
+  error: 'This operation was aborted'
+  code: 20
+  name: 'AbortError'
+  stack: |-
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+    *
+  ...
+# Subtest: 3 beforeEach describe
+    # Subtest: test 1
+    not ok 1 - test 1
+      ---
+      duration_ms: *
+      failureType: 'hookFailed'
+      error: 'This operation was aborted'
+      code: 20
+      name: 'AbortError'
+      stack: |-
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        async Promise.all (index 0)
+      ...
+    # Subtest: test 2
+    not ok 2 - test 2
+      ---
+      duration_ms: *
+      failureType: 'hookFailed'
+      error: 'This operation was aborted'
+      code: 20
+      name: 'AbortError'
+      stack: |-
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        async Promise.all (index 0)
+      ...
+    1..2
+not ok 3 - 3 beforeEach describe
+  ---
+  duration_ms: *
+  type: 'suite'
+  failureType: 'subtestsFailed'
+  error: '2 subtests failed'
+  code: 'ERR_TEST_FAILURE'
+  ...
+# Subtest: 4 afterEach describe
+    # Subtest: test 1
+    not ok 1 - test 1
+      ---
+      duration_ms: *
+      failureType: 'hookFailed'
+      error: 'This operation was aborted'
+      code: 20
+      name: 'AbortError'
+      stack: |-
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+      ...
+    # Subtest: test 2
+    not ok 2 - test 2
+      ---
+      duration_ms: *
+      failureType: 'hookFailed'
+      error: 'This operation was aborted'
+      code: 20
+      name: 'AbortError'
+      stack: |-
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+      ...
+    1..2
+not ok 4 - 4 afterEach describe
+  ---
+  duration_ms: *
+  type: 'suite'
+  failureType: 'subtestsFailed'
+  error: '2 subtests failed'
+  code: 'ERR_TEST_FAILURE'
+  ...
+1..4
+# tests 8
+# suites 4
+# pass 2
+# fail 4
+# cancelled 2
+# skipped 0
+# todo 0
+# duration_ms *
diff --git a/test/fixtures/test-runner/output/hooks.snapshot b/test/fixtures/test-runner/output/hooks.snapshot
@@ -134,8 +134,6 @@ not ok 3 - after throws
         *
         *
         *
-        async Promise.all (index 0)
-        *
         *
       ...
     1..2
@@ -183,7 +181,6 @@ not ok 4 - beforeEach throws
         *
         *
         *
-        async Promise.all (index 0)
         *
       ...
     1..2
@@ -265,7 +262,6 @@ not ok 6 - afterEach when test fails
         *
         *
         *
-        async Promise.all (index 0)
         *
       ...
     1..2