From 272ff66ac0240d651ba801b983fc544a7cfcb37e Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Tue, 25 Jul 2023 12:40:00 +0300 Subject: [PATCH] test_runner: cleanup test timeout abort listener fix #48475 --- lib/internal/test_runner/test.js | 43 +++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index a53ea8b4fc1d44..346226dbffd6ea 100644 --- 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,6 +557,8 @@ class Test extends AsyncResource { } }); + let testTimeout; + try { if (this.parent?.hooks.before.length > 0) { await this.parent.runHook('before', this.parent.getRunArgs()); @@ -556,7 +566,7 @@ class Test extends AsyncResource { 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 {