jest-circus Timeouts #3760
jest-circus Timeouts #3760
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3760 +/- ##
==========================================
- Coverage 59.92% 59.91% -0.01%
==========================================
Files 191 191
Lines 7001 7002 +1
Branches 6 6
==========================================
Hits 4195 4195
- Misses 2803 2804 +1
Partials 3 3
Continue to review full report at Codecov.
|
packages/jest-circus/src/utils.js
Outdated
| @@ -110,46 +110,59 @@ const getEachHooksForTest = ( | |||
| return result; | |||
| }; | |||
|
|
|||
| const _makeTimeoutMessage = (timeout, isHook) => { | |||
| const message = `Exceeded timeout of ${timeout}ms for a ${isHook ? 'hook' : 'test'}.`; | |||
There was a problem hiding this comment.
What do you think about also receiving testEntry.name as a parameter? So that it is a bit more explicit about in which test this happened
There was a problem hiding this comment.
i actually did it first and then realized that the name of the test is already printed right before the error (cause it's going to be under the failing test) 🙂
There was a problem hiding this comment.
What does this look like? Currently, timeouts are the hardest thing to deal with and the signal they give is so bad.
Also, you can write this function without curly braces, return statements or new variables. I believe in you.
There was a problem hiding this comment.
timeouts are hard to deal with, but mostly because of the quality of the test.
There's not much we can do here. We can try to print the exact hook that timeouted (if it happened in a hook) or we can try to print test coverage of the test function (to see which parts weren't called), but i'm not sure how helpful it is
There was a problem hiding this comment.
Can we add "Use jest.setTimeout(…) to update the timeout" maybe?
There was a problem hiding this comment.
that's actually a good idea
packages/jest-circus/src/utils.js
Outdated
packages/jest-circus/src/utils.js
Outdated
| } | ||
| // If it's a Promise, return it. | ||
| if (returnedValue instanceof Promise) { | ||
| return returnedValue.then(resolve).catch(reject); |
|
i also renamed |
|
I recommend making the jasmine.DEFAULT_TEST_TIMEOUT a getter/setter and write to jest.setTimeout for the compat layer. |
aaronabramov
force-pushed
the
circus-timeouts-1
branch
from
46a35bd
to
b3bdb5e
Compare
|
|
|
Mind inverting this? "Use … if this is a long running test." |
aaronabramov
force-pushed
the
circus-timeouts-1
branch
from
b3bdb5e
to
d458736
Compare
|
@cpojer done! |
|
shipit |
* jest-circus Timeouts * setTestTimeout/setTimeout
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
jest.setTestTimeoutwill actually set a timeout per async fn.So if there's a timeout of
20msand there's abeforeEachhook defined, the total timeout for a test will be40ms(20ms forbeforeEachand 20ms for the test itself).i think this is reasonable, but maybe we'll need to rename
setTestTimeouttosetTimeoutas @cpojer suggested earlier