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

mockResolvedValue / mockRejectedValue should returns a real Promise #6645

Closed
jbdemonte opened this issue Jul 6, 2018 · 17 comments · Fixed by #13503
Closed

mockResolvedValue / mockRejectedValue should returns a real Promise #6645

jbdemonte opened this issue Jul 6, 2018 · 17 comments · Fixed by #13503

Comments

@jbdemonte
Copy link
Contributor

🐛 Bug Report

mockResolvedValue and mockRejectedValue should return a real Promise testable with toBeInstanceOf but they don't.

To Reproduce

    it('should returns a Promise', async () => {
      const fn = jest.fn().mockResolvedValue(123);
      const promise = fn();
      expect(promise).toBeInstanceOf(Promise);
      const result = await promise;
      expect(result).toBe(123);
    });

Expected behavior

It should pass
The behaviour should be the same as using

const fn = jest.fn().mockReturnValue(Promise.resolve(123));

Run npx envinfo --preset jest

Paste the results here:

npx: installed 1 in 1.878s

  System:
    OS: macOS High Sierra 10.13.5
    CPU: x64 Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz
  Binaries:
    Node: 8.9.3 - ~/.nvm/versions/node/v8.9.3/bin/node
    Yarn: 1.7.0 - /usr/local/bin/yarn
    npm: 5.7.1 - ~/.nvm/versions/node/v8.9.3/bin/npm
  npmPackages:
    @types/jest: ^22.2.0 => 22.2.0 
    jest: ^22.4.3 => 22.4.4 
@SimenB
Copy link
Member

SimenB commented Jul 6, 2018

I wonder if this is due to #2549?

@AoDev
Copy link

AoDev commented Dec 2, 2018

Hello, maybe it's also related to this issue.

The error I get when using mockResolvedValue is:

TypeError: Cannot read property 'catch' of undefined

I have an async utility to be able to handle individual errors in a Promise.all.
It looks like this:

/**
 * Return both failed and successful promises
 * @param {Array.<Promise>} promises
 */
function promiseAllResults (promises) {
  return Promise.all(promises.map((promise) => promise.catch((err) => err)))
}

If I use a "real promise" as a mock, it works. (but then I can not spy)

@idler8
Copy link

idler8 commented Aug 19, 2021

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@mhombach
Copy link

mhombach commented Sep 2, 2022

This issue isn't fixed for 4 years (?!) and it's still causing problems like this (#13206)? Please fix asap or remove that sugar-function.

@yannbriancon
Copy link

yannbriancon commented Oct 24, 2022

I agree with @mhombach I struggled with tests for some time to end up discovering this issue.

Would be good to fix it or remove it completely.

@SimenB
Copy link
Member

SimenB commented Oct 24, 2022

@mhombach
Copy link

@SimenB hey :) I tested the v29.2.2 but for me it still shows a difference in instanceof:

image

This is tested with a brandnew angular project using Jest 29.2.2.

I also tested with without the fakeAsync, same result:

image

Any idea?

@SimenB
Copy link
Member

SimenB commented Oct 28, 2022

@mhombach that's probably a separate issue - the reproduction from the OP passes now. Could you open up a new issue with a minimal reproduction?

@mhombach
Copy link

@SimenB I created a repro-repo (https://github.com/mhombach/jest_repro_6645) can you have a look? The thing is that if I test

const resolvedPromise = jest.fn().mockResolvedValue('hello')();
const rejectedPromise = jest.fn().mockRejectedValue('hello')();

expect(resolvedPromise).toBeInstanceOf(Promise);
expect(rejectedPromise).toBeInstanceOf(Promise);

the unit test seems to not fail hard but just be "not passing" in some way:
image

Please use npm ci when installing the packages to make sure you have the exact same pacakges as I do.
If you verify to me after testing that this is some other issue, I will ofc open a new issue. But just now my tests are somewhat "silently failing" when I use exactly the test lines you added in the jest fix.

PS: Use the file app.component.spec.ts for testing, it contains the tests, partially commented out.

@SimenB
Copy link
Member

SimenB commented Oct 28, 2022

You need to await at least the rejected promise, or it's an unhandled rejection.


Can you open up a new issue? This one is resolved and easy to lose track of 🙂

@mhombach
Copy link

@SimenB I feel there is a misunderstanding here.
I am using the exactly same code you are using in the official Jest unit test that is added in the new release to make sure the change works properly. This code "silently fails" the unit test, as it's preventing the test from exiting with some error code. This means, that the unit test, which was added in the "fix", is not properly testing the new implementation... Or am I missing something?
Again: If I would now open a new issue, the issue would contain the same you added to the jest lib and then I would claim that this code does not work properly, which can be reproduced by my linked repo... That sounds conflicting to you saying that it's not related to this issue here and that the fix you implemented works properly.

Just to make it even more precise:
image
This is the test that you added (if I got it right) to make sure the change in your fix works as expected.

const resolvedPromise = jest.fn().mockResolvedValue('hello')();
const rejectedPromise = jest.fn().mockRejectedValue('hello')();

expect(resolvedPromise).toBeInstanceOf(Promise);
expect(rejectedPromise).toBeInstanceOf(Promise);

The same code is what I am testing locally. And on my machine it's silently failing. Which makes me believe, that it's also failing silently on Github-CI. Which means, the test is flawed and you cannot ensure that the functionality of your fix is working as expected.

Again, if this is some huge misunderstanding, correct me and I will gladly open a new issue :)

@SimenB
Copy link
Member

SimenB commented Oct 28, 2022

You're missing the await expect(rejectedPromise).rejects.toBe('hello') part at the bottom (and the resolved promise, but that doesn't matter in this context as it resolves) which means the rejected promise is unhandled, and fails the test run

@mhombach
Copy link

mhombach commented Nov 2, 2022

@SimenB
image

This is 100% the code you added in the fix (except the one commented line which I added.
This test should be failing (obviously) because of the expect(true).toBeFalsy();, but it isn't.
The code silently fails, which means it would never catch one of the other failing expects, right?

image

@SimenB
Copy link
Member

SimenB commented Nov 2, 2022

Same thing - expect throws which means you never attach a rejection handler to the rejected promise, leading to node throwing an unhandled rejection error.

Put expect(true).toBeFalsy() below await expect(rejectedPromise).rejects.toBe('hello'); and it'll fail like you expect 🙂

@mhombach
Copy link

mhombach commented Nov 2, 2022

@SimenB This means if I negate the 2 tests you have there by adding a not...

image

your 2 expects are also not failing, which makes them irrelevant/wrong (?):

image

Adding 2 console-logs there you can see that the testing of the "instance of" is not working at all:

image

image

... Which was the hole purpose of this mockRejectedValue should returns a real Promise PR, right? :/

@SimenB
Copy link
Member

SimenB commented Nov 2, 2022

Please open up a new issue with a minimal reproduction

@jestjs jestjs locked as resolved and limited conversation to collaborators Nov 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants