test_runner: cleanup test timeout abort listener

fix nodejs#48475
rluvaton · Jul 29, 2023 · 272ff66 · 272ff66
1 parent ee391f3
commit 272ff66
Showing 1 changed file with 29 additions and 14 deletions.
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -23,7 +23,6 @@ const {
   Symbol,
 } = primordials;
 const { AsyncResource } = require('async_hooks');
-const { once } = require('events');
 const { AbortController } = require('internal/abort_controller');
 const {
   codes: {
@@ -77,14 +76,23 @@ let kResistStopPropagation;
 
 function stopTest(timeout, signal) {
   if (timeout === kDefaultTimeout) {
-    return once(signal, 'abort');
+    const deferred = createDeferredPromise();
+
+    signal.addEventListener('abort', deferred.resolve, { __proto__: null, once: true });
+    deferred.cleanup = () => signal.removeEventListener('abort', deferred.resolve);
+    return deferred;
   }
-  return PromisePrototypeThen(setTimeout(timeout, null, { __proto__: null, ref: false, signal }), () => {
-    throw new ERR_TEST_FAILURE(
-      `test timed out after ${timeout}ms`,
-      kTestTimeoutFailure,
-    );
-  });
+
+  return {
+    __proto__: null,
+    promise: PromisePrototypeThen(setTimeout(timeout, null, { ref: false, signal }), () => {
+      throw new ERR_TEST_FAILURE(
+        `test timed out after ${timeout}ms`,
+        kTestTimeoutFailure,
+      );
+    }),
+    cleanup: noop,
+  };
 }
 
 class TestContext {
@@ -549,14 +557,16 @@ class Test extends AsyncResource {
       }
     });
 
+    let testTimeout;
+
     try {
       if (this.parent?.hooks.before.length > 0) {
         await this.parent.runHook('before', this.parent.getRunArgs());
       }
       if (this.parent?.hooks.beforeEach.length > 0) {
         await this.parent.runHook('beforeEach', { __proto__: null, args, ctx });
       }
-      const stopPromise = stopTest(this.timeout, this.signal);
+      testTimeout = stopTest(this.timeout, this.signal);
       const runArgs = ArrayPrototypeSlice(args);
       ArrayPrototypeUnshift(runArgs, this.fn, ctx);
 
@@ -572,16 +582,18 @@ class Test extends AsyncResource {
             'passed a callback but also returned a Promise',
             kCallbackAndPromisePresent,
           ));
-          await SafePromiseRace([ret, stopPromise]);
+          await SafePromiseRace([ret, testTimeout.promise]);
         } else {
-          await SafePromiseRace([PromiseResolve(promise), stopPromise]);
+          await SafePromiseRace([PromiseResolve(promise), testTimeout.promise]);
         }
       } else {
         // This test is synchronous or using Promises.
         const promise = ReflectApply(this.runInAsyncScope, this, runArgs);
-        await SafePromiseRace([PromiseResolve(promise), stopPromise]);
+        await SafePromiseRace([PromiseResolve(promise), testTimeout.promise]);
       }
 
+      testTimeout.cleanup();
+
       if (this[kShouldAbort]()) {
         this.postRun();
         return;
@@ -591,6 +603,7 @@ class Test extends AsyncResource {
       await after();
       this.pass();
     } catch (err) {
+      testTimeout?.cleanup();
       try { await afterEach(); } catch { /* test is already failing, let's ignore the error */ }
       try { await after(); } catch { /* Ignore error. */ }
       if (isTestFailureError(err)) {
@@ -817,6 +830,7 @@ class Suite extends Test {
   async run() {
     const hookArgs = this.getRunArgs();
 
+    let testTimeout;
     try {
       this.parent.activeSubtests++;
       await this.buildSuite;
@@ -834,15 +848,16 @@ class Suite extends Test {
 
       await this.runHook('before', hookArgs);
 
-      const stopPromise = stopTest(this.timeout, this.signal);
+      testTimeout = stopTest(this.timeout, this.signal);
       const subtests = this.skipped || this.error ? [] : this.subtests;
       const promise = SafePromiseAll(subtests, (subtests) => subtests.start());
 
-      await SafePromiseRace([promise, stopPromise]);
+      await SafePromiseRace([promise, testTimeout.promise]);
       await this.runHook('after', hookArgs);
 
       this.pass();
     } catch (err) {
+      testTimeout?.cleanup();
       if (isTestFailureError(err)) {
         this.fail(err);
       } else {