[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

[github-actions]

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.

[mhombach]

Issue is not fixed yet

[github-actions]

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.

[mhombach]

Issue is not fixed yet

[github-actions]

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.

[mhombach]

Issue is not fixed yet

[mhombach]

@SimenB any chance you can have a look at this again? See my last comment here where I provided more insights. thx

[github-actions]

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.

[mhombach]

Issue still needs a fix.

[github-actions]

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.

[mhombach]

Issue is not fixed yet. Seriously, this issue is now open for almost 1 full year and it's ignored?

[github-actions]

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.

[mhombach]

@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? :/

[mrazauskas]

is not even looked at by you or others?

Can you proof that?

[mhombach]

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

[github-actions]

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.

[mhombach]

Issue is not fixed yet

[github-actions]

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.

[mhombach]

@SimenB could you please have a look at this issue again? It's open for over 1 year now. I provided sffucient evidence that the last "patch" is not working and also a reproduceable example for testing. If this does not fit your needs, then what could a fellow dev possibly do to get a fix for this bug?

[github-actions]

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.

[mhombach]

Bug still open, just nobody is accepting this fact. Please archive the repo if you don't intend to fix reported bugs.

[github-actions]

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.

[mhombach]

Ok I give up. I tried now for almost 2 years to have a serious bug fixed in your worldwide shipped unit-testing code. I provided a vast amount of information, also when you shipped a patch that didn't fix the issue (proven by example, as I explained in full detail). I bumped this issue for eternity, pinging one of the maintainers directly but never got any response.

I will stop bumping this thread. For whoever encounters the same initial problem and scrolled all the way to the bottom: Maybe consider using a different, new and modern framework that looks promising. It's not only about this bug, but the way how bugs are explicitly not considered or responded to on this repo. I assume there are all kind of other bugs (as in every code/repo) and if the procedure here shows how bugs are tackled, then jest is just not as reliable as it should.

My bug still exists, you will get PASSES for unit tests that should FAIL because of some error, which obviously leads directly to buggy code beeing deployed.

[github-actions]

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.