Skip to content

test_runner: improve --test-timeout to be per test #57672

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

Merged
merged 10 commits into from
Apr 9, 2025
1 change: 1 addition & 0 deletions lib/internal/test_runner/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ class FileTest extends Test {
column: 1,
file: resolve(this.name),
};
this.timeout = null;
}

#skipReporting() {
Expand Down
7 changes: 7 additions & 0 deletions lib/internal/test_runner/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,12 @@ class Test extends AsyncResource {
if (timeout != null && timeout !== Infinity) {
validateNumber(timeout, 'options.timeout', 0, TIMEOUT_MAX);
this.timeout = timeout;
} else if (timeout == null) {
const cliTimeout = this.config.timeout;
if (cliTimeout != null && cliTimeout !== Infinity) {
validateNumber(cliTimeout, 'this.config.timeout', 0, TIMEOUT_MAX);
this.timeout = cliTimeout;
}
}

if (skip) {
Expand Down Expand Up @@ -1394,6 +1400,7 @@ class Suite extends Test {
reportedType = 'suite';
constructor(options) {
super(options);
this.timeout = null;

if (this.config.testNamePatterns !== null &&
this.config.testSkipPatterns !== null &&
Expand Down
4 changes: 1 addition & 3 deletions lib/internal/test_runner/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ function parseCommandLine() {
const sourceMaps = getOptionValue('--enable-source-maps');
const updateSnapshots = getOptionValue('--test-update-snapshots');
const watch = getOptionValue('--watch');
const timeout = getOptionValue('--test-timeout') || Infinity;
const isChildProcess = process.env.NODE_TEST_CONTEXT === 'child';
const isChildProcessV8 = process.env.NODE_TEST_CONTEXT === 'child-v8';
let concurrency;
Expand All @@ -211,7 +212,6 @@ function parseCommandLine() {
let shard;
let testNamePatterns = mapPatternFlagToRegExArray('--test-name-pattern');
let testSkipPatterns = mapPatternFlagToRegExArray('--test-skip-pattern');
let timeout;

if (isChildProcessV8) {
kBuiltinReporters.set('v8-serializer', 'internal/test_runner/reporter/v8-serializer');
Expand Down Expand Up @@ -242,7 +242,6 @@ function parseCommandLine() {

if (isTestRunner) {
isolation = getOptionValue('--test-isolation');
timeout = getOptionValue('--test-timeout') || Infinity;

if (isolation === 'none') {
concurrency = 1;
Expand Down Expand Up @@ -271,7 +270,6 @@ function parseCommandLine() {
};
}
} else {
timeout = Infinity;
concurrency = 1;
const testNamePatternFlag = getOptionValue('--test-name-pattern');
only = getOptionValue('--test-only');
Expand Down
19 changes: 19 additions & 0 deletions test/fixtures/test-runner/output/test-timeout-flag.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Flags: --test-timeout=20
'use strict';
const { describe, test } = require('node:test');
const { setTimeout } = require('node:timers/promises');

describe('--test-timeout is set to 20ms', () => {
test('should timeout after 20ms', async () => {
await setTimeout(200000, undefined, { ref: false });
});
test('should timeout after 5ms', { timeout: 5 }, async () => {
await setTimeout(200000, undefined, { ref: false });
});

test('should not timeout', { timeout: 50000 }, async () => {
await setTimeout(1);
});

test('should pass', async () => {});
});
55 changes: 55 additions & 0 deletions test/fixtures/test-runner/output/test-timeout-flag.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
TAP version 13
# Subtest: --test-timeout is set to 20ms
# Subtest: should timeout after 20ms
not ok 1 - should timeout after 20ms
---
duration_ms: *
type: 'test'
location: '/test/fixtures/test-runner/output/test-timeout-flag.js:(LINE):3'
failureType: 'testTimeoutFailure'
error: 'test timed out after 20ms'
code: 'ERR_TEST_FAILURE'
stack: |-
async Promise.all (index 0)
...
# Subtest: should timeout after 5ms
not ok 2 - should timeout after 5ms
---
duration_ms: *
type: 'test'
location: '/test/fixtures/test-runner/output/test-timeout-flag.js:(LINE):3'
failureType: 'testTimeoutFailure'
error: 'test timed out after 5ms'
code: 'ERR_TEST_FAILURE'
...
# Subtest: should not timeout
ok 3 - should not timeout
---
duration_ms: *
type: 'test'
...
# Subtest: should pass
ok 4 - should pass
---
duration_ms: *
type: 'test'
...
1..4
not ok 1 - --test-timeout is set to 20ms
---
duration_ms: *
type: 'suite'
location: '/test/fixtures/test-runner/output/test-timeout-flag.js:(LINE):1'
failureType: 'subtestsFailed'
error: '2 subtests failed'
code: 'ERR_TEST_FAILURE'
...
1..1
# tests 4
# suites 1
# pass 2
# fail 0
# cancelled 2
# skipped 0
# todo 0
# duration_ms *
4 changes: 2 additions & 2 deletions test/parallel/test-runner-filetest-location.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const fixtures = require('../common/fixtures');
const { strictEqual } = require('node:assert');
const { relative } = require('node:path');
const { run } = require('node:test');
const fixture = fixtures.path('test-runner', 'never_ending_sync.js');
const fixture = fixtures.path('test-runner', 'index.js');
const relativePath = relative(process.cwd(), fixture);
const stream = run({
files: [relativePath],
Expand All @@ -13,7 +13,7 @@ const stream = run({

stream.on('test:fail', common.mustCall((result) => {
strictEqual(result.name, relativePath);
strictEqual(result.details.error.failureType, 'testTimeoutFailure');
strictEqual(result.details.error.failureType, 'testCodeFailure');
strictEqual(result.line, 1);
strictEqual(result.column, 1);
strictEqual(result.file, fixture);
Expand Down
9 changes: 9 additions & 0 deletions test/parallel/test-runner-output.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,15 @@ const tests = [
name: 'test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js',
flags: ['--test-reporter=tap'],
},
{
name: 'test-runner/output/test-timeout-flag.js',
flags: ['--test-reporter=tap'],
},
// --test-timeout should work with or without --test flag
{
name: 'test-runner/output/test-timeout-flag.js',
flags: ['--test-reporter=tap', '--test'],
},
{
name: 'test-runner/output/hooks-with-no-global-test.js',
flags: ['--test-reporter=tap'],
Expand Down
5 changes: 2 additions & 3 deletions test/parallel/test-runner-run.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,9 @@ describe('require(\'node:test\').run', { concurrency: true }, () => {

it('should support timeout', async () => {
const stream = run({ timeout: 50, files: [
fixtures.path('test-runner', 'never_ending_sync.js'),
fixtures.path('test-runner', 'never_ending_async.js'),
fixtures.path('test-runner', 'timeout-basic.mjs'),
] });
stream.on('test:fail', common.mustCall(2));
stream.on('test:fail', common.mustCall(1));
stream.on('test:pass', common.mustNotCall());
// eslint-disable-next-line no-unused-vars
for await (const _ of stream);
Expand Down
Loading