[Bug]: "mockImplementation" is behaving different than the syntacical-sugar version"mockResolvedValue"

[mhombach]

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 #6645 and #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

[mhombach]

I can now confirm it has to do with #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.

[mhombach]

Any chance this issue will be verified and tackled? It's an easy reproducible bug which should be also somewhat-eaasy to fix.

[yannbriancon]

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.

[SimenB]

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.

[SimenB]

@mhombach please give https://github.com/facebook/jest/releases/tag/v29.2.2 a try 🙂

[mhombach]

@SimenB big thanks, will test tomorrow and report back :)

[mhombach]

Update:

Following a longer discussion on this issue: #6645

The #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 #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 expected 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 #6645 implemented was precisely about resolvedPromise instanceOf Promise beeing TRUE, I would consider that PR-fix failed/flawed since it

1. doesn't change the resolvedPromise instanceOf Promise expression to be TRUE and
2. 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');
    });

[mhombach]

@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.

[mhombach]

Issue is still not fixed