[Bug]: resetAllMocks restoring mocks instead of resetting them

[vetruvet]

trafficstars

Version

29.4.0

Steps to reproduce

Simple test to reproduce:

test('it should reset properly', () => {
  const obj = {
    foo: () => 42,
  };

  const fooSpy = jest.spyOn(obj, 'foo');

  console.log(obj.foo());
  expect(obj.foo).toHaveBeenCalled();

  jest.resetAllMocks();

  console.log(obj.foo());
  expect(obj.foo).toHaveBeenCalled();
});

Expected behavior

The test should pass

Actual behavior

The test fails with Matcher error: received value must be a mock or spy function.

Additional context

It seems that resetAllMocks is acting like mockRestore instead of mockReset.

If I replace jest.resetAllMocks() with fooSpy.mockReset(), the test passes as expected, whereas if I replace it with fooSpy.mockRestore() it fails with that error again.

Environment

System:
    OS: Linux 5.15 KDE neon 5.26 5.26
    CPU: (16) x64 AMD Ryzen 7 PRO 6850U with Radeon Graphics
  Binaries:
    Node: 18.12.0 - ~/.nvm/versions/node/v18.12.0/bin/node
    npm: 8.19.2 - ~/.nvm/versions/node/v18.12.0/bin/npm
  npmPackages:
    jest: ^29.4.0 => 29.4.0

[pratik9333]

Are you working on this?

[vetruvet]

No, I don't know enough about Jest internals to suggest a fix.

~Besides, I'd expect that a certain multi-billion-dollar corporation would be maintaining this. Don't get me wrong, I'm all for contributing to open source but if the project is backed by a huge corporation, I think they can spare the time to fix it.~ (see https://jestjs.io/blog/2022/05/11/jest-joins-openjs, thanks @mrazauskas for pointing it out)

[me4502]

This line in this PR introduced the regression; https://github.com/facebook/jest/pull/13692/files#diff-7ae0bd704c3c2789b19abe2bbf94aca3505e2a6b3823d85c7f6b316b216d37c9R1411

Looking here (https://github.com/facebook/jest/pull/13692/files#diff-7ae0bd704c3c2789b19abe2bbf94aca3505e2a6b3823d85c7f6b316b216d37c9L1409), the same function is called to restore the mocks. While it's called reset in the lambda function of the new addition, it's still the same function being called.

Unsure if just removing this call is the solution, or if something else needs to be done to still retain functionality of that PR

I'm actually a bit confused, as it seems the point of that PR (and the issue it fixes) is to make the behaviour of mockReset similar to mockRestore? AFAICT it's still a regression on resetAllMocks though, as that's actually removing the mock status of the function. But the original purpose of that PR is still a breaking change either way

[mrazauskas]

@feliperli Could you take a look, please? Seems like your PR (#13692) introduced this issue. Or is that expected behaviour?

[mrazauskas]

Besides, I'd expect that a certain multi-billion-dollar corporation would be maintaining this.

By the way, Jest is just another open source project maintained by few volunteers. See https://jestjs.io/blog/2022/05/11/jest-joins-openjs

[vetruvet]

TIL, thanks for pointing that out.

I did end up doing some digging, and while I'm not any closer to finding the root cause, I did notice this:

  resetAllMocks(): void {
    this._spyState.forEach(reset => reset());
    this._mockConfigRegistry = new WeakMap();
    this._mockState = new WeakMap();
  }

  restoreAllMocks(): void {
    this._spyState.forEach(restore => restore());
    this._spyState = new Set();
  }

It seems that both of those methods end up calling all the _spyState functions and on line 1401 just above, you see this._spyState.add(restore); so it seems that resetAllMocks is calling the restore function (but confusingly naming it reset in the forEach).

I'm not sure if this is intentional, or if it's not, what a fix would look like, but hopefully this info at least helps.

[bmuenzenmeyer]

I'm not sure if this is intentional, or if it's not, what a fix would look like, but hopefully this info at least helps.

also seeing this impact our unit tests, with only a regenerated lockfile causing the breaking change

[feliperli]

@feliperli Could you take a look, please? Seems like your PR (#13692) introduced this issue. Or is that expected behaviour?

Oh no :( I'm looking into it.

[ghost]

I also see this!!!!

[EladHeller]

I have a different problem, but I think the cause is the same change in mockReset. Someone called spyOn on a mock function and it caused a maximum call stack error This is an example that reproduces the error:

describe('test console.log', () => {
  console.log = jest.fn();
  const log = jest.spyOn(console, 'log');

  afterEach(() => {
    log.mockReset();
  });

  test('console.log a', () => {
    console.log('test a');

    expect(log).toBeCalledWith('test a');
  });

  test('console.log b', () => {
    /**
     * An error is thrown on the next line:
     * RangeError: Maximum call stack size exceeded
     * at WeakMap.get (<anonymous>)
     * at ModuleMocker._ensureMockState (node_modules/jest-mock/build/index.js:275:33)
     */
    console.log('test b');

    expect(log).toBeCalledWith('test b');
  });
});

When I remove the line this._mockConfigRegistry.set(f, originalMockImpl); it works fine.

[rdsedmundo]

Seems related: I'm also getting random RangeError: Maximum Call Stack Size Exceeded that are pointing to the jest-mock package, reverting pack to 29.3.1 fixes the issue.

[mrazauskas]

Should be fixed in #13866

[SimenB]

https://github.com/facebook/jest/releases/tag/v29.4.3

[trivikr]

Looks like the bug wasn't fixed in mockReset

I've created a new issue at https://github.com/facebook/jest/issues/13916

[vetruvet]

29.4.3 fixes the original issue for me, along with the "Maximum Call Stack Size Exceeded" error. Thanks all!

[github-actions[bot]]

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.