GeezerVison is showing github.com — test_runner: improve --test-timeout to be per test · nodejs/node@87ac6cf
File treeExpand file treeCollapse file tree Open diff view settings fixtures/test-runner/output
Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/test_runner/runner.js +1Lines changed: 1 addition & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -191,6 +191,7 @@ class FileTest extends Test { | 191 | 191 | column: 1, | 192 | 192 | file: resolve(this.name), | 193 | 193 | }; | | 194 | + this.timeout = null; | 194 | 195 | } | 195 | 196 |
| 196 | 197 | #skipReporting() { | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/test_runner/test.js +7Lines changed: 7 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -608,6 +608,12 @@ class Test extends AsyncResource { | 608 | 608 | if (timeout != null && timeout !== Infinity) { | 609 | 609 | validateNumber(timeout, 'options.timeout', 0, TIMEOUT_MAX); | 610 | 610 | this.timeout = timeout; | | 611 | + } else if (timeout == null) { | | 612 | + const cliTimeout = this.config.timeout; | | 613 | + if (cliTimeout != null && cliTimeout !== Infinity) { | | 614 | + validateNumber(cliTimeout, 'this.config.timeout', 0, TIMEOUT_MAX); | | 615 | + this.timeout = cliTimeout; | | 616 | + } | 611 | 617 | } | 612 | 618 |
| 613 | 619 | if (skip) { | @@ -1394,6 +1400,7 @@ class Suite extends Test { | 1394 | 1400 | reportedType = 'suite'; | 1395 | 1401 | constructor(options) { | 1396 | 1402 | super(options); | | 1403 | + this.timeout = null; | 1397 | 1404 |
| 1398 | 1405 | if (this.config.testNamePatterns !== null && | 1399 | 1406 | this.config.testSkipPatterns !== null && | |
Collapse file Copy file name to clipboardExpand all lines: lib/internal/test_runner/utils.js +1-3Lines changed: 1 addition & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -196,6 +196,7 @@ function parseCommandLine() { | 196 | 196 | const sourceMaps = getOptionValue('--enable-source-maps'); | 197 | 197 | const updateSnapshots = getOptionValue('--test-update-snapshots'); | 198 | 198 | const watch = getOptionValue('--watch'); | | 199 | + const timeout = getOptionValue('--test-timeout') || Infinity; | 199 | 200 | const isChildProcess = process.env.NODE_TEST_CONTEXT === 'child'; | 200 | 201 | const isChildProcessV8 = process.env.NODE_TEST_CONTEXT === 'child-v8'; | 201 | 202 | let concurrency; | @@ -211,7 +212,6 @@ function parseCommandLine() { | 211 | 212 | let shard; | 212 | 213 | let testNamePatterns = mapPatternFlagToRegExArray('--test-name-pattern'); | 213 | 214 | let testSkipPatterns = mapPatternFlagToRegExArray('--test-skip-pattern'); | 214 | | - let timeout; | 215 | 215 |
| 216 | 216 | if (isChildProcessV8) { | 217 | 217 | kBuiltinReporters.set('v8-serializer', 'internal/test_runner/reporter/v8-serializer'); | @@ -242,7 +242,6 @@ function parseCommandLine() { | 242 | 242 |
| 243 | 243 | if (isTestRunner) { | 244 | 244 | isolation = getOptionValue('--test-isolation'); | 245 | | - timeout = getOptionValue('--test-timeout') || Infinity; | 246 | 245 |
| 247 | 246 | if (isolation === 'none') { | 248 | 247 | concurrency = 1; | @@ -271,7 +270,6 @@ function parseCommandLine() { | 271 | 270 | }; | 272 | 271 | } | 273 | 272 | } else { | 274 | | - timeout = Infinity; | 275 | 273 | concurrency = 1; | 276 | 274 | const testNamePatternFlag = getOptionValue('--test-name-pattern'); | 277 | 275 | only = getOptionValue('--test-only'); | |
Collapse file Copy file name to clipboard +19Lines changed: 19 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +// Flags: --test-timeout=20 | | 2 | +'use strict'; | | 3 | +const { describe, test } = require('node:test'); | | 4 | +const { setTimeout } = require('node:timers/promises'); | | 5 | + | | 6 | +describe('--test-timeout is set to 20ms', () => { | | 7 | + test('should timeout after 20ms', async () => { | | 8 | + await setTimeout(200000, undefined, { ref: false }); | | 9 | + }); | | 10 | + test('should timeout after 5ms', { timeout: 5 }, async () => { | | 11 | + await setTimeout(200000, undefined, { ref: false }); | | 12 | + }); | | 13 | + | | 14 | + test('should not timeout', { timeout: 50000 }, async () => { | | 15 | + await setTimeout(1); | | 16 | + }); | | 17 | + | | 18 | + test('should pass', async () => {}); | | 19 | +}); |
Collapse file Copy file name to clipboard +55Lines changed: 55 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +TAP version 13 | | 2 | +# Subtest: --test-timeout is set to 20ms | | 3 | + # Subtest: should timeout after 20ms | | 4 | + not ok 1 - should timeout after 20ms | | 5 | + --- | | 6 | + duration_ms: * | | 7 | + type: 'test' | | 8 | + location: '/test/fixtures/test-runner/output/test-timeout-flag.js:(LINE):3' | | 9 | + failureType: 'testTimeoutFailure' | | 10 | + error: 'test timed out after 20ms' | | 11 | + code: 'ERR_TEST_FAILURE' | | 12 | + stack: |- | | 13 | + async Promise.all (index 0) | | 14 | + ... | | 15 | + # Subtest: should timeout after 5ms | | 16 | + not ok 2 - should timeout after 5ms | | 17 | + --- | | 18 | + duration_ms: * | | 19 | + type: 'test' | | 20 | + location: '/test/fixtures/test-runner/output/test-timeout-flag.js:(LINE):3' | | 21 | + failureType: 'testTimeoutFailure' | | 22 | + error: 'test timed out after 5ms' | | 23 | + code: 'ERR_TEST_FAILURE' | | 24 | + ... | | 25 | + # Subtest: should not timeout | | 26 | + ok 3 - should not timeout | | 27 | + --- | | 28 | + duration_ms: * | | 29 | + type: 'test' | | 30 | + ... | | 31 | + # Subtest: should pass | | 32 | + ok 4 - should pass | | 33 | + --- | | 34 | + duration_ms: * | | 35 | + type: 'test' | | 36 | + ... | | 37 | + 1..4 | | 38 | +not ok 1 - --test-timeout is set to 20ms | | 39 | + --- | | 40 | + duration_ms: * | | 41 | + type: 'suite' | | 42 | + location: '/test/fixtures/test-runner/output/test-timeout-flag.js:(LINE):1' | | 43 | + failureType: 'subtestsFailed' | | 44 | + error: '2 subtests failed' | | 45 | + code: 'ERR_TEST_FAILURE' | | 46 | + ... | | 47 | +1..1 | | 48 | +# tests 4 | | 49 | +# suites 1 | | 50 | +# pass 2 | | 51 | +# fail 0 | | 52 | +# cancelled 2 | | 53 | +# skipped 0 | | 54 | +# todo 0 | | 55 | +# duration_ms * |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-runner-filetest-location.js +2-2Lines changed: 2 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -4,7 +4,7 @@ const fixtures = require('../common/fixtures'); | 4 | 4 | const { strictEqual } = require('node:assert'); | 5 | 5 | const { relative } = require('node:path'); | 6 | 6 | const { run } = require('node:test'); | 7 | | -const fixture = fixtures.path('test-runner', 'never_ending_sync.js'); | | 7 | +const fixture = fixtures.path('test-runner', 'index.js'); | 8 | 8 | const relativePath = relative(process.cwd(), fixture); | 9 | 9 | const stream = run({ | 10 | 10 | files: [relativePath], | @@ -13,7 +13,7 @@ const stream = run({ | 13 | 13 |
| 14 | 14 | stream.on('test:fail', common.mustCall((result) => { | 15 | 15 | strictEqual(result.name, relativePath); | 16 | | - strictEqual(result.details.error.failureType, 'testTimeoutFailure'); | | 16 | + strictEqual(result.details.error.failureType, 'testCodeFailure'); | 17 | 17 | strictEqual(result.line, 1); | 18 | 18 | strictEqual(result.column, 1); | 19 | 19 | strictEqual(result.file, fixture); | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-runner-output.mjs +9Lines changed: 9 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -132,6 +132,15 @@ const tests = [ | 132 | 132 | name: 'test-runner/output/timeout_in_before_each_should_not_affect_further_tests.js', | 133 | 133 | flags: ['--test-reporter=tap'], | 134 | 134 | }, | | 135 | + { | | 136 | + name: 'test-runner/output/test-timeout-flag.js', | | 137 | + flags: ['--test-reporter=tap'], | | 138 | + }, | | 139 | + // --test-timeout should work with or without --test flag | | 140 | + { | | 141 | + name: 'test-runner/output/test-timeout-flag.js', | | 142 | + flags: ['--test-reporter=tap', '--test'], | | 143 | + }, | 135 | 144 | { | 136 | 145 | name: 'test-runner/output/hooks-with-no-global-test.js', | 137 | 146 | flags: ['--test-reporter=tap'], | |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-runner-run.mjs +2-3Lines changed: 2 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -70,10 +70,9 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { | 70 | 70 |
| 71 | 71 | it('should support timeout', async () => { | 72 | 72 | const stream = run({ timeout: 50, files: [ | 73 | | - fixtures.path('test-runner', 'never_ending_sync.js'), | 74 | | - fixtures.path('test-runner', 'never_ending_async.js'), | | 73 | + fixtures.path('test-runner', 'timeout-basic.mjs'), | 75 | 74 | ] }); | 76 | | - stream.on('test:fail', common.mustCall(2)); | | 75 | + stream.on('test:fail', common.mustCall(1)); | 77 | 76 | stream.on('test:pass', common.mustNotCall()); | 78 | 77 | // eslint-disable-next-line no-unused-vars | 79 | 78 | for await (const _ of stream); | |
|