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: add timeout for tests #43490

Closed
MoLow opened this issue Jun 19, 2022 · 8 comments
Closed

test_runner: add timeout for tests #43490

MoLow opened this issue Jun 19, 2022 · 8 comments
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.

Comments

@MoLow
Copy link
Member

MoLow commented Jun 19, 2022

What is the problem this feature will solve?

currently - tests can easily be stuck or never end, which makes it very hard to understand and locate where your problem might originate

for example, this program will be very hard to debug:

test('top level', { concurrency: 2 }, async (t) => {
  await t.test('endless', () => new Promise(() => {
    setTimeout(() => {}, /* Large number */);
  }));


  // ... many other tests
});

What is the feature you are proposing to solve the problem?

add a timeout to tests running via node:test, I propose adding a default timeout that will be configurable
one of the most important parts of testing is failing fast and knowing what the failure is, and timing out will help determine which test is misfunctioning

What alternatives have you considered?

No response

@MoLow MoLow added the feature request Issues that request new features to be added to Node.js. label Jun 19, 2022
@MoLow
Copy link
Member Author

MoLow commented Jun 19, 2022

CC @benjamingr Please ping relevant people and add a label?

@VoltrexKeyva VoltrexKeyva added the test_runner Issues and PRs related to the test runner subsystem. label Jun 19, 2022
@VoltrexKeyva
Copy link
Member

@nodejs/test_runner

@benjamingr
Copy link
Member

Yes, this is a good idea and I am in favor. Is this something you'd like to work on?

@MoLow
Copy link
Member Author

MoLow commented Jun 19, 2022

Yes, this is a good idea and I am in favor. Is this something you'd like to work on?

Yes, assuming no objections

@cjihrig
Copy link
Contributor

cjihrig commented Jun 19, 2022

No objections - this is something we should definitely support.

nodejs-github-bot pushed a commit that referenced this issue Jul 14, 2022
PR-URL: #43505
Refs: #43490
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
aduh95 pushed a commit to aduh95/node-core-test that referenced this issue Jul 15, 2022
PR-URL: nodejs/node#43505
Refs: nodejs/node#43490
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
(cherry picked from commit c1d659591d1c5a6002e762549ee46b69d495b70b)
aduh95 pushed a commit to aduh95/node-core-test that referenced this issue Jul 16, 2022
PR-URL: nodejs/node#43505
Refs: nodejs/node#43490
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
(cherry picked from commit c1d659591d1c5a6002e762549ee46b69d495b70b)
@MoLow MoLow closed this as completed Jul 17, 2022
aduh95 pushed a commit to aduh95/node-core-test that referenced this issue Jul 20, 2022
PR-URL: nodejs/node#43505
Refs: nodejs/node#43490
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
(cherry picked from commit c1d659591d1c5a6002e762549ee46b69d495b70b)
danielleadams pushed a commit that referenced this issue Jul 26, 2022
PR-URL: #43505
Refs: #43490
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
targos pushed a commit that referenced this issue Jul 31, 2022
PR-URL: #43505
Refs: #43490
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
PR-URL: nodejs/node#43505
Refs: nodejs/node#43490
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@bitflower
Copy link

Has this been implemented?

@benjamingr
Copy link
Member

Yeah there is a timeout parameter now check the test runner docs. @bitflower

@bitflower
Copy link

Thanks for the feedback @benjamingr ! Sorry to not report back. I had found it in the meantime (it's different to e.g. mocha).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

No branches or pull requests

5 participants