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

[Bug]: mockReset restoring mocks instead of resetting them #13916

Closed
trivikr opened this issue Feb 15, 2023 · 15 comments
Closed

[Bug]: mockReset restoring mocks instead of resetting them #13916

trivikr opened this issue Feb 15, 2023 · 15 comments

Comments

@trivikr
Copy link
Contributor

trivikr commented Feb 15, 2023

Version

29.4.3

Steps to reproduce

Simple test to repro:

describe("mockReset", () => {
  const originalReturnValue = "original";
  const mockReturnValue = "mocked";

  test("spyOn", () => {
    const module = { api: () => originalReturnValue };
    jest.spyOn(module, "api").mockImplementation(() => mockReturnValue);

    expect(module.api()).toStrictEqual(mockReturnValue);
    expect(module.api).toHaveBeenCalledTimes(1);

    module.api.mockReset();
    expect(module.api).toHaveBeenCalledTimes(0);

    expect(module.api()).toStrictEqual(undefined);
  });

  test("overwrite", () => {
    const module = { api: () => originalReturnValue };
    module.api = jest.fn().mockImplementation(() => mockReturnValue);

    expect(module.api()).toStrictEqual(mockReturnValue);
    expect(module.api).toHaveBeenCalledTimes(1);

    module.api.mockReset();
    expect(module.api).toHaveBeenCalledTimes(0);

    expect(module.api()).toStrictEqual(undefined);
  });
});

Expected behavior

The test should pass where module.api() is reset to undefined in both the spyOn and overwrite cases.

Actual behavior

The test fails in spyOn case.

  ● mockReset › spyOn

    expect(received).toStrictEqual(expected) // deep equality

    Expected: undefined
    Received: "original"

      13 |     expect(module.api).toHaveBeenCalledTimes(0);
      14 |
    > 15 |     expect(module.api()).toStrictEqual(undefined);
         |                          ^
      16 |   });

Additional context

The fix for resetAllMocks from #13808 should be applied to mockReset too?

Environment

System:
    OS: macOS 13.1
    CPU: (8) arm64 Apple M1 Pro
  Binaries:
    Node: 18.13.0 - ~/Library/Caches/fnm_multishells/59531_1676389811348/bin/node
    Yarn: 3.4.1 - ~/Library/Caches/fnm_multishells/59531_1676389811348/bin/yarn
    npm: 8.19.3 - ~/Library/Caches/fnm_multishells/59531_1676389811348/bin/npm
  npmPackages:
    jest: ^29.4.3 => 29.4.3
@trivikr
Copy link
Contributor Author

trivikr commented Feb 15, 2023

The fix appears to be writing an equivalent of #13866 for mockReset function.

@mrazauskas
Copy link
Contributor

The behaviour is correct. Try using jest.resetAllMocks() and mockFn.mockReset(), you should get the same result. Before #13866 this was not the case.

Documentation of mockFn.mockreset(): "removes any mocked return values or implementations. This is useful when you want to completely reset a mock back to its initial state."

After being reset the module.api mock created using jest.spyOn should return originalReturnValue, because that was its initial state.

With module.api = jest.fn() you are replacing the original method manually. Hence the initial state is defined by jest.fn(), not the original method.

@trivikr
Copy link
Contributor Author

trivikr commented Feb 15, 2023

Verified that mockFn.mockReset() should reset mock to it's initial state.

This is useful when you want to completely reset a mock back to its initial state.

@trivikr trivikr closed this as completed Feb 15, 2023
@trivikr
Copy link
Contributor Author

trivikr commented Feb 15, 2023

Looks like this issue was reported as a feature request in #13229
And the fix was released in v29.4.0

@trivikr
Copy link
Contributor Author

trivikr commented Feb 15, 2023

Reopening as @shlomo-artlist had commented on how this is a breaking change, and how it makes mockRestore redundant.

#13229 (comment)

@trivikr
Copy link
Contributor Author

trivikr commented Feb 15, 2023

When I searched for differences between mockReset and mockRestore, multiple posts online share examples where mockReset returns undefined.

Example: https://marek-rozmus.medium.com/f52395581950

@dhampik
Copy link

dhampik commented Feb 16, 2023

I'd like to second that 29.4.3 has some breaking changes. It's probably either of these, but we have not identified which particularly:

We've been using restoreMocks: true jest config option since jest version 24 and it worked for us, but with the 29.4.2 -> 29.4.3 update quarter of our tests started to fail. If we flip the option to false, there are much less tests starting to fail (and they fail for a different reason).

Unfortunately, I can't provide a minimal reproducible example yet. If we will be able to identify the exact problem, I'll post an update here. I'm making this comment just to see if others have similar issues.

Also, this comment might be related, since replit example from it also doesn't work with version 29.4.3.

PS. Both of the above changes are announced as "fixes", so maybe we were relying on a bug in our setup for 5 major versions of jest 😅. Still it does not sound like it should be a patch-version update.

@mrazauskas
Copy link
Contributor

The #5143 (comment) which you mentioned is now somewhat outdated. Since Jest v29.4.0 first answer should read:

"Does mockReset() reset implementation for spies?"
Yes. The implementation of a mock will be reset to the initial state.

Reference #13692


If your CIs are public, I can take a look. Minimal reproduction is always welcome too.

@dhampik
Copy link

dhampik commented Feb 16, 2023

Thank you so much for your quick reply @mrazauskas! This information is helpful.

Unfortunately, the CI is not public. I'll post an update as soon as we figure out something.

@mscrivo
Copy link

mscrivo commented Feb 16, 2023

Echoing @dhampik, 300+ of our tests started failing with this update, all with the same pattern of error:

TypeError: Cannot read properties of undefined (reading 'data')

I'm still trying to trace down why, so I don't have a minimal repro yet, nor are our tests public, but just wanted to add another data point here that something is fishy with the changes in this release.

@mrazauskas
Copy link
Contributor

The restoreMocks: true option is most likely the trouble. See #13925 (comment)

@github-actions
Copy link

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.

@github-actions github-actions bot added the Stale label Mar 18, 2023
@github-actions
Copy link

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 17, 2023
@github-actions
Copy link

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

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

No branches or pull requests

4 participants