Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_runner: cleanup test timeout abort listener #48915

56 changes: 46 additions & 10 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ const {
SafeSet,
SafePromiseAll,
SafePromiseRace,
SymbolDispose,
ObjectDefineProperty,
Symbol,
} = primordials;
const { addAbortListener } = require('events');
const { AsyncResource } = require('async_hooks');
const { once } = require('events');
const { AbortController } = require('internal/abort_controller');
const {
codes: {
Expand Down Expand Up @@ -52,7 +54,7 @@ const {
validateOneOf,
validateUint32,
} = require('internal/validators');
const { setTimeout } = require('timers/promises');
const { setTimeout } = require('timers');
const { TIMEOUT_MAX } = require('internal/timers');
const { availableParallelism } = require('os');
const { bigint: hrtime } = process.hrtime;
Expand All @@ -76,15 +78,42 @@ const { testNamePatterns, testOnlyFlag } = parseCommandLine();
let kResistStopPropagation;

function stopTest(timeout, signal) {
const deferred = createDeferredPromise();
const abortListener = addAbortListener(signal, deferred.resolve);
let timer;
let disposeFunction;

if (timeout === kDefaultTimeout) {
return once(signal, 'abort');
disposeFunction = abortListener[SymbolDispose];
} if (timeout !== kDefaultTimeout) {
timer = setTimeout(() => deferred.resolve(), timeout);
timer.unref();

ObjectDefineProperty(deferred, 'promise', {
__proto__: null,
configurable: true,
writable: true,
value: PromisePrototypeThen(deferred.promise, () => {
throw new ERR_TEST_FAILURE(
`test timed out after ${timeout}ms`,
kTestTimeoutFailure,
);
}),
rluvaton marked this conversation as resolved.
Show resolved Hide resolved
});

disposeFunction = () => {
abortListener[SymbolDispose]();
timer[SymbolDispose]();
};
}
return PromisePrototypeThen(setTimeout(timeout, null, { __proto__: null, ref: false, signal }), () => {
throw new ERR_TEST_FAILURE(
`test timed out after ${timeout}ms`,
kTestTimeoutFailure,
);

ObjectDefineProperty(deferred.promise, SymbolDispose, {
__proto__: null,
configurable: true,
writable: true,
value: disposeFunction,
});
return deferred.promise;
}

class TestContext {
Expand Down Expand Up @@ -549,14 +578,16 @@ class Test extends AsyncResource {
}
});

let stopPromise;

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);
stopPromise = stopTest(this.timeout, this.signal);
const runArgs = ArrayPrototypeSlice(args);
ArrayPrototypeUnshift(runArgs, this.fn, ctx);

Expand Down Expand Up @@ -603,6 +634,8 @@ class Test extends AsyncResource {
this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
}
} finally {
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.
if (this.parent !== null) {
Expand Down Expand Up @@ -817,6 +850,7 @@ class Suite extends Test {
async run() {
const hookArgs = this.getRunArgs();

let stopPromise;
rluvaton marked this conversation as resolved.
Show resolved Hide resolved
try {
this.parent.activeSubtests++;
await this.buildSuite;
Expand All @@ -834,7 +868,7 @@ class Suite extends Test {

await this.runHook('before', hookArgs);

const stopPromise = 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());

Expand All @@ -848,6 +882,8 @@ class Suite extends Test {
} else {
this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
}
} finally {
stopPromise?.[SymbolDispose]();
}

this.postRun();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';
const { beforeEach, afterEach, test} = require("node:test");
beforeEach(() => {});
afterEach(() => {});

for (let i = 1; i <= 11; ++i) {
test(`${i}`, () => {});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
TAP version 13
# Subtest: 1
ok 1 - 1
---
duration_ms: *
...
# Subtest: 2
ok 2 - 2
---
duration_ms: *
...
# Subtest: 3
ok 3 - 3
---
duration_ms: *
...
# Subtest: 4
ok 4 - 4
---
duration_ms: *
...
# Subtest: 5
ok 5 - 5
---
duration_ms: *
...
# Subtest: 6
ok 6 - 6
---
duration_ms: *
...
# Subtest: 7
ok 7 - 7
---
duration_ms: *
...
# Subtest: 8
ok 8 - 8
---
duration_ms: *
...
# Subtest: 9
ok 9 - 9
---
duration_ms: *
...
# Subtest: 10
ok 10 - 10
---
duration_ms: *
...
# Subtest: 11
ok 11 - 11
---
duration_ms: *
...
1..11
# tests 11
# suites 0
# pass 11
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms *
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
'use strict';
const { beforeEach, afterEach, test} = require("node:test");
beforeEach(() => {}, {timeout: 10000});
afterEach(() => {}, {timeout: 10000});

for (let i = 1; i <= 11; ++i) {
test(`${i}`, () => {});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
TAP version 13
# Subtest: 1
ok 1 - 1
---
duration_ms: *
...
# Subtest: 2
ok 2 - 2
---
duration_ms: *
...
# Subtest: 3
ok 3 - 3
---
duration_ms: *
...
# Subtest: 4
ok 4 - 4
---
duration_ms: *
...
# Subtest: 5
ok 5 - 5
---
duration_ms: *
...
# Subtest: 6
ok 6 - 6
---
duration_ms: *
...
# Subtest: 7
ok 7 - 7
---
duration_ms: *
...
# Subtest: 8
ok 8 - 8
---
duration_ms: *
...
# Subtest: 9
ok 9 - 9
---
duration_ms: *
...
# Subtest: 10
ok 10 - 10
---
duration_ms: *
...
# Subtest: 11
ok 11 - 11
---
duration_ms: *
...
1..11
# tests 11
# suites 0
# pass 11
# fail 0
# cancelled 0
# skipped 0
# todo 0
# duration_ms *
2 changes: 2 additions & 0 deletions test/parallel/test-runner-output.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ const tests = [
{ name: 'test-runner/output/describe_nested.js' },
{ name: 'test-runner/output/hooks.js' },
{ name: 'test-runner/output/hooks-with-no-global-test.js' },
{ name: 'test-runner/output/before-and-after-each-too-many-listeners.js' },
{ name: 'test-runner/output/before-and-after-each-with-timeout-too-many-listeners.js' },
{ name: 'test-runner/output/no_refs.js' },
{ name: 'test-runner/output/no_tests.js' },
{ name: 'test-runner/output/only_tests.js' },
Expand Down