jest
jest copied to clipboard
[Bug]: "mockImplementation" is behaving different than the syntacical-sugar version"mockResolvedValue"
Version
27.5.1
Steps to reproduce
Repro:
const testMock = {
works: jest.fn().mockImplementation(() => Promise.resolve(undefined)),
fails: jest.fn().mockResolvedValue(undefined),
};
it('test-mockResolvedValue', fakeAsync(async() => {
await testMock.fails();
flush();
expect(true).toBeTrue();
}));
it('test-mockImplementation', fakeAsync(async() => {
await testMock.works();
flush();
expect(true).toBeTrue();
}));
Expected behavior
Both tests are passing without any error, since both should be exactly the same as the Jest-docs are stating that mockFn.mockResolvedValue(value)
is "Syntactic sugar function for" jest.fn().mockImplementation(() => Promise.resolve(value))
.
Actual behavior
Test test-mockResolvedValue
fails on the flush()
due to The code should be running in the fakeAsync zone to call this function
:
Additional context
I feel this problem is linked to https://github.com/facebook/jest/issues/6645 and https://github.com/facebook/jest/issues/11146 .
Environment
System:
OS: Windows 10 10.0.19043
CPU: (12) x64 Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
Binaries:
Node: 16.16.0 - C:\Program Files\nodejs\node.EXE
npm: 8.11.0 - C:\Program Files\nodejs\npm.CMD
npmPackages:
jest: ^27.2.3 => 27.5.1
I can now confirm it has to do with https://github.com/facebook/jest/issues/6645 :
const testMock = {
works: jest.fn().mockImplementation(() => Promise.resolve(undefined)),
fails: jest.fn().mockResolvedValue(undefined),
};
it('test-mockImplementation', fakeAsync(async() => {
console.log(new Promise(() => null) instanceof Promise); // true
console.log(testMock.works() instanceof Promise); // true
console.log(testMock.fails() instanceof Promise); // FALSE
expect(true).toBeTrue();
}));
The mockResolvedValue()
does NOT return a normal/real Promise
, which buggs out ngZone.
Any chance this issue will be verified and tackled? It's an easy reproducible bug which should be also somewhat-eaasy to fix.
I confirm this should be fixed. This is very hard to debug as nothing indicates mockResolved or mockReject could have a different behavior.
For example, waiting for a next tick would finish the promises with mockImplementation but not with mockResolved or mockReject.
Reproductions here give me ReferenceError: fakeAsync is not defined
, fwiw.
A full standalone reproduction would allow me to verify if #13503 fixed the reported issue. You can also probably just make the change I made there in your own node_modules
to verify.
@mhombach please give https://github.com/facebook/jest/releases/tag/v29.2.2 a try 🙂
@SimenB big thanks, will test tomorrow and report back :)
Update:
Following a longer discussion on this issue: https://github.com/facebook/jest/issues/6645
The https://github.com/facebook/jest/issues/6645 tried to fix the problem, but (to my impression) failed to do so.
It added this unit-test (https://github.com/SimenB/jest/blob/464b45a015f98485d4037f1b4d4c5da750e39890/e2e/mock-functions/tests/index.js) to make sure that jest.fn().mockResolvedValue('hello')()
is an instance of Promise
.
test('instanceof promise', async () => {
const resolvedPromise = jest.fn().mockResolvedValue('hello')();
const rejectedPromise = jest.fn().mockRejectedValue('hello')();
expect(resolvedPromise).toBeInstanceOf(Promise);
expect(rejectedPromise).toBeInstanceOf(Promise);
await expect(resolvedPromise).resolves.toBe('hello');
await expect(rejectedPromise).rejects.toBe('hello');
});
This is the code written in the unit test in https://github.com/facebook/jest/issues/6645 .
A working unit test like expect(resolvedPromise).toBeInstanceOf(Promise);
should fail if I negate the unit test to expect(resolvedPromise).not.toBeInstanceOf(Promise);
. But this is not the case. The unit test always silently fails, regardless of what is expect
ed and so it "passes" with errors.
Also, if we do a console.log
on resolvedPromise instanceOf Promise
we get FALSE
.
Since the whole purpose of the fix that https://github.com/facebook/jest/issues/6645 implemented was precisely about resolvedPromise instanceOf Promise
beeing TRUE
, I would consider that PR-fix failed/flawed since it
- doesn't change the
resolvedPromise instanceOf Promise
expression to beTRUE
and - it implements unit tests that can never make the test suit fail and are, by that, useless.
@SimenB maybe we can continue our discussion here? I think I have provided sufficient evidence and explained my case in detail. A minimum repro would be:
test('instanceof promise', async () => {
const resolvedPromise = jest.fn().mockResolvedValue('hello')();
const rejectedPromise = jest.fn().mockRejectedValue('hello')();
expect(resolvedPromise).not.toBeInstanceOf(Promise);
expect(rejectedPromise).not.toBeInstanceOf(Promise);
console.log(resolvedPromise instanceof Promise);
console.log(rejectedPromise instanceof Promise);
await expect(resolvedPromise).resolves.toBe('hello');
await expect(rejectedPromise).rejects.toBe('hello');
});
@SimenB any feedback on this? Again, my assumption is that the fix you implemented does not work and the unit tests added are not passing and just silently erroring out.
Issue is still not fixed
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.
Issue is not fixed yet
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.
Issue is not fixed yet
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.
Issue is not fixed yet
@SimenB any chance you can have a look at this again? See my last comment here where I provided more insights. thx
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.
Issue is not fixed yet. Seriously, this issue is now open for over half a year and it's ignored?
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.
Issue needs a fix.
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.
Issue still needs a fix.
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.
Issue is not fixed yet. Seriously, this issue is now open for almost 1 full year and it's ignored?
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.
@SimenB Almost 1 year has passed and there is no response or feedback regarding this issue. Could you at least explain why this bug, which isn't affecting only me and is hard to debug/find (as others here can confirm), is not even looked at by you or others? :/
is not even looked at by you or others?
Can you proof that?
@mrazauskas No communication since Nov 2, 2022
for a bug that is silently giving a PASS
to any affected unit-test while the test should be failing (in a repo that is used in a gazillion of apps) is sadly something that I consider is not even looked
, yes.
I provided multiple ready-to-use examples for the bug and also provided my own insights of research (even if it wasn't a deepdive). The last statement was that there was a fix which, as I explained in detail, is having a bug on its own. Also, no reaction to this flawed unit-test aka "fix".
I'm merely trying to get a dangerous bug fixed ...
This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.
Issue is not fixed yet