Add async stack traces for circus #6281
Conversation
| at __tests__/during_tests.test.js:36:3 | ||
|
|
||
| ● returned promise rejection | ||
|
|
||
| Failed: Object { | ||
| thrown: Object { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
stderr.replace(/thrown|Failed/, 'REPLACED');
😛
| } | ||
| event.test.errors = event.test.errors.map(errors => { | ||
| let error; | ||
| if (Array.isArray(errors)) { |
There was a problem hiding this comment.
this part is really hairy (even though I removed the react native hack).
Ideas welcome!
| at __tests__/during_tests.test.js:36:3 | ||
|
|
||
| ● returned promise rejection | ||
|
|
||
| Failed: Object { | ||
| thrown: Object { |
There was a problem hiding this comment.
stderr.replace(/thrown|Failed/, 'REPLACED');
😛
| @@ -43,33 +43,42 @@ const handler: EventHandler = (event, state): void => { | |||
| } | |||
| case 'add_hook': { | |||
| const {currentDescribeBlock} = state; | |||
| const {fn, hookType: type, timeout} = event; | |||
| const {asyncError, fn, hookType: type, timeout} = event; | |||
There was a problem hiding this comment.
i was thinking that maybe we can name it somehow differently?
when i see asyncError i assume that it already happened, but in this case we're just capturing it to use in the future?
not in this PR, but maybe something to change in the future
There was a problem hiding this comment.
Yeah, it's just so we can keep the stack across async boundaries
There was a problem hiding this comment.
Open to naming suggestions
packages/jest-circus/src/index.js
Outdated
|
|
||
| const asyncError = new Error(); | ||
| if (Error.captureStackTrace) { | ||
| Error.captureStackTrace(asyncError, test); |
There was a problem hiding this comment.
why is stack captured here but not in the other ones?
There was a problem hiding this comment.
Mostly because I'm lazy.
Error.captureStackTrace removes the frame (function) from the second argument, so this in practice removes this function from the stack, meaning users don't get circus in their output.
| return error.stack; | ||
| } else if (error.message) { | ||
| return error.message; | ||
| const _formatError = (errors: ?Exception | [?Exception, Exception]): string => { |
There was a problem hiding this comment.
?Exception | {error: Exception, asyncError?: Exception}?
There was a problem hiding this comment.
I tried this, but I failed horribly... Feel free to fix it if you want, my in progress stuff is here: 8a8d362
| @@ -7,12 +7,13 @@ | |||
| * @flow strict-local | |||
There was a problem hiding this comment.
this should probably import the one in jest-jasmine rather than copy it...
|
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. |
Summary
A first pass at making jest-circus support the same async stack traces that jest-jasmine does. I'm not totally happy with the implementation, so happy to take feedback.
Test plan
See #4362 for instructions on how to run with circus.
Note that jest-circus will still fail the test because of #6277, but all other tests are green
/cc @captbaritone I'm not allowed to assign you