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

add --test-timeout flag #50431

Closed
mcollina opened this issue Oct 27, 2023 · 8 comments · Fixed by #50443
Closed

add --test-timeout flag #50431

mcollina opened this issue Oct 27, 2023 · 8 comments · Fixed by #50443
Labels
test_runner Issues and PRs related to the test runner subsystem.

Comments

@mcollina
Copy link
Member

I think adding a --test-timeout 60000 flag is necessary for the test runner to fulfill basic uses cases without requiring a custom runner. It's often the case of having a test that times out, and currently no output is produced in such case.

@koshic
Copy link

koshic commented Oct 27, 2023

Agree, it will be very useful for CI (may be with a special exit code).

PS please don't forget to add it to run options

@mcollina
Copy link
Member Author

There is already in run

@mcollina mcollina added the test_runner Issues and PRs related to the test runner subsystem. label Oct 28, 2023
@shubham9411
Copy link
Contributor

Can I take this one? @mcollina

@mcollina
Copy link
Member Author

Go for it.

nodejs-github-bot pushed a commit that referenced this issue Nov 8, 2023
PR-URL: #50443
Fixes: #50431
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
targos pushed a commit that referenced this issue Nov 11, 2023
PR-URL: #50443
Fixes: #50431
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
targos pushed a commit that referenced this issue Nov 14, 2023
PR-URL: #50443
Fixes: #50431
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this issue Dec 11, 2023
PR-URL: #50443
Fixes: #50431
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
Reviewed-By: Raz Luvaton <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@rozzilla
Copy link

@mcollina / @shubham9411 do you know if/when this option will be available for Node v.20.x? 😊

@mcollina
Copy link
Member Author

mcollina commented Dec 21, 2023

Will be backported in the next few months.

@mcollina
Copy link
Member Author

Actually it will ship today (ideally) #51124

@cjihrig
Copy link
Contributor

cjihrig commented Mar 30, 2024

I think the implementation of --test-timeout that shipped is not what users would expect. If I were using --test-timeout my expectation (especially based on the documentation) would be for the timeout to be applied to each individual test within a file. What was shipped is a flag that only applies to entire test files.

Consider the following code run with --test-timeout=2000. It correctly times out when used with --test, but passes without --test.

const { test } = require('node:test');

test((t, done) => {
  setTimeout(done, 3000);
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants