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

[Feature]: Explicitly mention that test is failing because done() is not being called #13844

Closed
daanklijn opened this issue Jan 30, 2023 · 8 comments · Fixed by #13847
Closed

Comments

@daanklijn
Copy link

🚀 Feature Proposal

Currently, when using the done callback the test will fail with a timeout if done is never called. I believe that this test failure is not explicit enough. We should instead also mention that this timeout occurred because the done callback was never called.

Motivation

I believe it should be obvious why a test is failing. I therefore believe there is added value in letting the user know that a test is failing because the done callback was never called.

Example

Current behaviour:

test("1+1 equals 2", (done) => {
  expect(1 + 1).toBe(2);
});

>>>

Exceeded timeout of 5000 ms for a test. 
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test

Proposed behaviour:

test("1+1 equals 2", (done) => {
  expect(1 + 1).toBe(2);
});

>>>

Exceeded timeout of 5000 ms while waiting for done to be called. 
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test

Pitch

See motivation.

@daanklijn
Copy link
Author

This is great! Thanks @lpizzinidev!

@SimenB
Copy link
Member

SimenB commented Feb 7, 2023

@DercilioFontes
Copy link
Contributor

Hi guys,

I am getting an error that is very common to happen in the CI pipeline with the message:

thrown: "Exceeded timeout of 5000 ms for a testfalse.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

It took a while to understand what "testfalse" meant. The test was negative (passed), but it timed out!? Googling gives nothing. 😅

I searched in the jest code and found that:

// node_modules/jest-circus/build/utils.js
// line 208
const _makeTimeoutMessage = (timeout, isHook, takesDoneCallback) =>
  `Exceeded timeout of ${(0, _jestUtil.formatTime)(timeout)} for a ${
    isHook ? 'hook' : 'test'
  }${
    takesDoneCallback && ' while waiting for `done()` to be called'
  }.\nUse jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test.`;

So, the change from this issue creates the word "testfalse" when the takesDoneCallback is false.

It was very confusing for me. Maybe it could be good to change it to avoid creating this testfalse.

@SimenB
Copy link
Member

SimenB commented Feb 24, 2023

yep! wanna send a PR? Just changing takesDoneCallback && ' while waiting for `done()` to be called' to takesDoneCallback ? ' while waiting for `done()` to be called' : '' should do it

@DercilioFontes
Copy link
Contributor

@SimenB, sure! I can do it. I'll send it. Thanks for your feedback.

@DercilioFontes
Copy link
Contributor

Hi @SimenB,

I sent the PR (#13954)

@SimenB
Copy link
Member

SimenB commented Mar 6, 2023

@github-actions
Copy link

github-actions bot commented Apr 6, 2023

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants