Moshe Suggestion

nodejs · Jul 25, 2023 · fa48543 · fa48543
1 parent 60b916d
commit fa48543
Showing 1 changed file with 27 additions and 54 deletions.
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -22,8 +22,10 @@ const {
   SafePromiseRace,
   ObjectDefineProperty,
   Symbol,
+  SymbolDispose,
 } = primordials;
 const { AsyncResource } = require('async_hooks');
+const { addAbortListener } = require('events');
 const { AbortController } = require('internal/abort_controller');
 const {
   codes: {
@@ -77,56 +79,27 @@ let kResistStopPropagation;
 
 function stopTest(timeout, signal) {
   const deferred = createDeferredPromise();
+  const abortListener = addAbortListener(signal, deferred.resolve);
+  let timer;
+
 
   if (timeout === kDefaultTimeout) {
-    signal.addEventListener('abort', deferred.resolve, { __proto__: null, once: true });
-
-    ObjectDefineProperty(deferred, 'cleanup', {
-      __proto__: null,
-      configurable: true,
-      writable: true,
-      value: () => signal.removeEventListener('abort', deferred.resolve),
-    });
-
-    return deferred;
-  }
-
-  signal.addEventListener('abort', throwOnAbort, { __proto__: null, once: true });
-  const cleanAbort = () => signal.removeEventListener('abort', throwOnAbort);
-  const timer = setTimeout(() => {
-    deferred.resolve();
-    cleanAbort();
-  }, timeout);
-
-  timer.unref();
-  function throwOnAbort() {
-    deferred.reject(new AbortError(undefined, { cause: signal.reason }));
-    clearTimeout(timer);
-  }
-
-  ObjectDefineProperty(deferred, 'promise', {
-    __proto__: null,
-    configurable: true,
-    writable: true,
-    value: PromisePrototypeThen(deferred.promise, () => {
+    deferred.promise[SymbolDispose] = abortListener[SymbolDispose];
+  } if (timeout !== kDefaultTimeout) {
+    timer = setTimeout(() => deferred.resolve(), timeout);
+    timer.unref();
+    deferred.promise = PromisePrototypeThen(deferred.promise, () => {
       throw new ERR_TEST_FAILURE(
         `test timed out after ${timeout}ms`,
         kTestTimeoutFailure,
       );
-    }),
-  });
-
-  ObjectDefineProperty(deferred, 'cleanup', {
-    __proto__: null,
-    configurable: true,
-    writable: true,
-    value: () => {
-      clearTimeout(timer);
-      cleanAbort();
-    },
-  });
-
-  return deferred;
+    });
+    deferred.promise[SymbolDispose] = () => {
+      abortListener[SymbolDispose]();
+      timer[SymbolDispose]();
+    }
+  }
+  return deferred.promise;
 }
 
 class TestContext {
@@ -591,7 +564,7 @@ class Test extends AsyncResource {
       }
     });
 
-    let stopPromiseWrapper;
+    let stopPromise;
 
     try {
       if (this.parent?.hooks.before.length > 0) {
@@ -600,7 +573,7 @@ class Test extends AsyncResource {
       if (this.parent?.hooks.beforeEach.length > 0) {
         await this.parent.runHook('beforeEach', { args, ctx });
       }
-      stopPromiseWrapper = stopTest(this.timeout, this.signal);
+      stopPromise = stopTest(this.timeout, this.signal);
       const runArgs = ArrayPrototypeSlice(args);
       ArrayPrototypeUnshift(runArgs, this.fn, ctx);
 
@@ -616,14 +589,14 @@ class Test extends AsyncResource {
             'passed a callback but also returned a Promise',
             kCallbackAndPromisePresent,
           ));
-          await SafePromiseRace([ret, stopPromiseWrapper.promise]);
+          await SafePromiseRace([ret, stopPromise]);
         } else {
-          await SafePromiseRace([PromiseResolve(promise), stopPromiseWrapper.promise]);
+          await SafePromiseRace([PromiseResolve(promise), stopPromise]);
         }
       } else {
         // This test is synchronous or using Promises.
         const promise = ReflectApply(this.runInAsyncScope, this, runArgs);
-        await SafePromiseRace([PromiseResolve(promise), stopPromiseWrapper.promise]);
+        await SafePromiseRace([PromiseResolve(promise), stopPromise]);
       }
 
       if (this[kShouldAbort]()) {
@@ -647,7 +620,7 @@ class Test extends AsyncResource {
         this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
       }
     } finally {
-      stopPromiseWrapper?.cleanup();
+      stopPromise?.[SymbolDispose]();
 
       // 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.
@@ -863,7 +836,7 @@ class Suite extends Test {
   async run() {
     const hookArgs = this.getRunArgs();
 
-    let stopPromiseWrapper;
+    let stopPromise;
     try {
       this.parent.activeSubtests++;
       await this.buildSuite;
@@ -881,11 +854,11 @@ class Suite extends Test {
 
       await this.runHook('before', hookArgs);
 
-      stopPromiseWrapper = stopTest(this.timeout, this.signal);
+      stopPromise = stopTest(this.timeout, this.signal);
       const subtests = this.skipped || this.error ? [] : this.subtests;
       const promise = SafePromiseAll(subtests, (subtests) => subtests.start());
 
-      await SafePromiseRace([promise, stopPromiseWrapper.promise]);
+      await SafePromiseRace([promise, stopPromise]);
       await this.runHook('after', hookArgs);
 
       this.pass();
@@ -896,7 +869,7 @@ class Suite extends Test {
         this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
       }
     } finally {
-      stopPromiseWrapper?.cleanup();
+      stopPromise?.[SymbolDispose]();
     }
 
     this.postRun();