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

Jest marks a test as "Passed" when an assertion fails on a .resolves #4997

Closed
Vinnl opened this issue Dec 1, 2017 · 7 comments
Closed

Jest marks a test as "Passed" when an assertion fails on a .resolves #4997

Vinnl opened this issue Dec 1, 2017 · 7 comments

Comments

@Vinnl
Copy link
Contributor

Vinnl commented Dec 1, 2017

Do you want to request a feature or report a bug?

A bug.

What is the current behavior?

The following assertion fails:

expect(Promise.resolve('Expected value')).resolves.toBe('Not the expected value');

It throws an error, but nevertheless, Jest marks the test as Passed. See this repl.it:

https://repl.it/repls/ObedientNegativeGrison

What is the expected behavior?

Jest should mention that the test failed.

Please provide your exact Jest configuration and mention your Jest, node,
yarn/npm version and operating system.

$ yarn run jest --version
v21.2.1

$ node --version
v6.11.3

$ yarn --version
1.3.2

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 16.04.3 LTS
Release:        16.04
Codename:       xenial
@SimenB
Copy link
Member

SimenB commented Dec 1, 2017

You have to return (or await) the expect.

http://facebook.github.io/jest/docs/en/expect.html#resolves

@SimenB SimenB closed this as completed Dec 1, 2017
@Vinnl
Copy link
Contributor Author

Vinnl commented Dec 1, 2017

Oh really? Ugh, sorry for wasting your time - I thought that requirement had been removed with the addition of .resolves.

@SimenB
Copy link
Member

SimenB commented Dec 1, 2017

You still have to somehow tell jest to wait for it - it's still a promise 🙂

Difference now is that you await the expect itself, and not the value within it

@Vinnl
Copy link
Contributor Author

Vinnl commented Dec 1, 2017

Hehe yeah it makes sense. Gives me some bad faith about all those tests I've already written without this measure :P I could submit a PR to emphasise this in the docs if you think that's a good idea?

@SimenB
Copy link
Member

SimenB commented Dec 1, 2017

Please do! All doc improvements are very welcome

Vinnl added a commit to Vinnl/jest that referenced this issue Dec 1, 2017
Emphasise that `.rejects` and `.resolves` still require the assertion to be returned, in response to jestjs#4997.
@Vinnl
Copy link
Contributor Author

Vinnl commented Dec 1, 2017

@SimenB #4999 - almost at number 5000!

cpojer pushed a commit that referenced this issue Dec 1, 2017
Emphasise that `.rejects` and `.resolves` still require the assertion to be returned, in response to #4997.
@github-actions
Copy link

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 May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants