-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
fix .toThrow for promises #4884
Conversation
fixes #3601 Signed-off-by: Łukasz Sentkiewicz <[email protected]>
Signed-off-by: Łukasz Sentkiewicz <[email protected]>
Signed-off-by: Łukasz Sentkiewicz <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #4884 +/- ##
=======================================
Coverage 59.75% 59.75%
=======================================
Files 195 195
Lines 6533 6533
Branches 4 3 -1
=======================================
Hits 3904 3904
Misses 2629 2629 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome! Thanks for tackling it.
Could you update the changelog as well?
@@ -416,15 +416,15 @@ For example, this code tests that the promise rejects with reason `'octopus'`: | |||
```js | |||
test('rejects to octopus', () => { | |||
// make sure to add a return statement | |||
return expect(Promise.reject('octopus')).rejects.toBe('octopus'); | |||
return expect(Promise.reject(new Error('octopus'))).rejects.toThrow('octopus'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This old example is still valid, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(although it's not recommended to reject with a non-error)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it still works
Signed-off-by: Łukasz Sentkiewicz <[email protected]>
@SimenB |
}); | ||
``` | ||
|
||
Alternatively, you can use `async/await` in combination with `.rejects`. | ||
|
||
```js | ||
test('rejects to octopus', async () => { | ||
await expect(Promise.reject('octopus')).rejects.toBe('octopus'); | ||
await expect(Promise.reject(new Error('octopus'))).rejects.toThrow('octopus'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we should say that toThrow
only works in jest 21.3.0+. People copy-paste examples, and having them not working is not a good UX
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently, it mentions
### `.rejects`
##### available in Jest **20.0.0+**
Change it to 21.3.0+
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or a separate line describing rejecting promises and toThrow
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lsentkiewicz this came up in #4945, mind sending a PR with clarification? 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really painful way to discover why rejects.toThrow
was not working on my [email protected]
setup
@SimenB you were right, it is probably a good idea to have a mention to it in the docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you send a PR fixing the docs?
Nice! |
is there a new release planned including this fix? it seems impossible currently to correctly test for Promise rejections 🤔 |
Shouldn't it be |
both methods work |
@Turbo87 it's out as |
* Beta version because it has a bug fix for testing rejected promises: * jestjs/jest#4884 * Needed for upcoming unit tests for events/schedules API
* Beta version because it has a bug fix for testing rejected promises: * jestjs/jest#4884 * Needed for upcoming unit tests for events/schedules API
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. |
fixes #3601
From docs
In real app, an
Error
object must be rejected.Fixed to