Repository: jestjs/jest (TypeScript)

Issue:
[Bug]: resetAllMocks restoring mocks instead of resetting them
### 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

```shell
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
```



Hints:
Are you working on this? 
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)
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](https://github.com/facebook/jest/issues/13229)) 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
@feliperli Could you take a look, please? Seems like your PR (#13692) introduced this issue. Or is that expected behaviour?
> 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

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](https://github.com/facebook/jest/blob/eca83e7221137785d585215f9ae9251adcdbacc3/packages/jest-mock/src/index.ts#L1410):

```
  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.
> 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 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.
I also see this!!!!
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:
``` js
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.
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.

Patch:
diff --git a/packages/jest-mock/src/index.ts b/packages/jest-mock/src/index.ts
--- a/packages/jest-mock/src/index.ts
+++ b/packages/jest-mock/src/index.ts
@@ -258,6 +258,8 @@ type MockFunctionConfig = {
   specificMockImpls: Array<Function>;
 };
 
+type SpyState = {reset?: () => void; restore: () => void};
+
 const MOCK_CONSTRUCTOR_NAME = 'mockConstructor';
 
 const FUNCTION_NAME_RESERVED_PATTERN = /[\s!-/:-@[-`{-~]/;
@@ -506,9 +508,8 @@ export class ModuleMocker {
   private readonly _environmentGlobal: typeof globalThis;
   private _mockState: WeakMap<Mock, MockFunctionState>;
   private _mockConfigRegistry: WeakMap<Function, MockFunctionConfig>;
-  private _spyState: Set<() => void>;
+  private _spyState: Set<SpyState>;
   private _invocationCallCounter: number;
-  private _originalFn: WeakMap<Mock, Function>;
 
   /**
    * @see README.md
@@ -521,7 +522,6 @@ export class ModuleMocker {
     this._mockConfigRegistry = new WeakMap();
     this._spyState = new Set();
     this._invocationCallCounter = 1;
-    this._originalFn = new WeakMap();
   }
 
   private _getSlots(object?: Record<string, any>): Array<string> {
@@ -613,27 +613,27 @@ export class ModuleMocker {
 
   private _makeComponent<T extends Record<string, any>>(
     metadata: MockMetadata<T, 'object'>,
-    restore?: () => void,
+    spyState?: SpyState,
   ): T;
   private _makeComponent<T extends Array<unknown>>(
     metadata: MockMetadata<T, 'array'>,
-    restore?: () => void,
+    spyState?: SpyState,
   ): T;
   private _makeComponent<T extends RegExp>(
     metadata: MockMetadata<T, 'regexp'>,
-    restore?: () => void,
+    spyState?: SpyState,
   ): T;
   private _makeComponent<T>(
     metadata: MockMetadata<T, 'constant' | 'collection' | 'null' | 'undefined'>,
-    restore?: () => void,
+    spyState?: SpyState,
   ): T;
   private _makeComponent<T extends UnknownFunction>(
     metadata: MockMetadata<T, 'function'>,
-    restore?: () => void,
+    spyState?: SpyState,
   ): Mock<T>;
   private _makeComponent<T extends UnknownFunction>(
     metadata: MockMetadata<T>,
-    restore?: () => void,
+    spyState?: SpyState,
   ): Record<string, any> | Array<unknown> | RegExp | T | Mock | undefined {
     if (metadata.type === 'object') {
       return new this._environmentGlobal.Object();
@@ -754,8 +754,8 @@ export class ModuleMocker {
       f._isMockFunction = true;
       f.getMockImplementation = () => this._ensureMockConfig(f).mockImpl as T;
 
-      if (typeof restore === 'function') {
-        this._spyState.add(restore);
+      if (spyState != null) {
+        this._spyState.add(spyState);
       }
 
       this._mockState.set(f, this._defaultMockState());
@@ -775,18 +775,23 @@ export class ModuleMocker {
 
       f.mockReset = () => {
         f.mockClear();
-        const originalFn = this._originalFn.get(f);
-        const originalMockImpl = {
-          ...this._defaultMockConfig(),
-          mockImpl: originalFn,
-        };
-        this._mockConfigRegistry.set(f, originalMockImpl);
+        this._mockConfigRegistry.delete(f);
+
+        if (spyState != null) {
+          spyState.reset?.();
+        }
+
         return f;
       };
 
       f.mockRestore = () => {
-        f.mockReset();
-        return restore ? restore() : undefined;
+        f.mockClear();
+        this._mockConfigRegistry.delete(f);
+
+        if (spyState != null) {
+          spyState.restore();
+          this._spyState.delete(spyState);
+        }
       };
 
       f.mockReturnValueOnce = (value: ReturnType<T>) =>
@@ -994,14 +999,14 @@ export class ModuleMocker {
     T extends object,
     K extends PropertyLikeKeys<T>,
   >(object: T, propertyKey: K): ReplacedPropertyRestorer<T, K> | undefined {
-    for (const spyState of this._spyState) {
+    for (const {restore} of this._spyState) {
       if (
-        'object' in spyState &&
-        'property' in spyState &&
-        spyState.object === object &&
-        spyState.property === propertyKey
+        'object' in restore &&
+        'property' in restore &&
+        restore.object === object &&
+        restore.property === propertyKey
       ) {
-        return spyState as ReplacedPropertyRestorer<T, K>;
+        return restore as ReplacedPropertyRestorer<T, K>;
       }
     }
 
@@ -1117,6 +1122,15 @@ export class ModuleMocker {
     return fn;
   }
 
+  private _attachMockImplementation<T extends Function>(
+    mock: Mock<UnknownFunction>,
+    original: T,
+  ) {
+    mock.mockImplementation(function (this: unknown) {
+      return original.apply(this, arguments);
+    });
+  }
+
   spyOn<
     T extends object,
     K extends PropertyLikeKeys<T>,
@@ -1202,29 +1216,43 @@ export class ModuleMocker {
 
       if (descriptor && descriptor.get) {
         const originalGet = descriptor.get;
-        mock = this._makeComponent({type: 'function'}, () => {
-          descriptor!.get = originalGet;
-          Object.defineProperty(object, methodKey, descriptor!);
-        });
+        mock = this._makeComponent(
+          {type: 'function'},
+          {
+            reset: () => {
+              this._attachMockImplementation(mock, original);
+            },
+            restore: () => {
+              descriptor!.get = originalGet;
+              Object.defineProperty(object, methodKey, descriptor!);
+            },
+          },
+        );
         descriptor.get = () => mock;
         Object.defineProperty(object, methodKey, descriptor);
       } else {
-        mock = this._makeComponent({type: 'function'}, () => {
-          if (isMethodOwner) {
-            object[methodKey] = original;
-          } else {
-            delete object[methodKey];
-          }
-        });
-        // @ts-expect-error overriding original method with a Mock
+        mock = this._makeComponent(
+          {type: 'function'},
+          {
+            reset: () => {
+              this._attachMockImplementation(mock, original);
+            },
+            restore: () => {
+              if (isMethodOwner) {
+                object[methodKey] = original;
+              } else {
+                delete object[methodKey];
+              }
+            },
+          },
+        );
+        // @ts-expect-error: overriding original method with a mock
         object[methodKey] = mock;
       }
 
-      mock.mockImplementation(function (this: unknown) {
-        return original.apply(this, arguments);
-      });
+      this._attachMockImplementation(mock, original);
     }
-    this._originalFn.set(object[methodKey] as Mock, original);
+
     return object[methodKey] as Mock;
   }
 
@@ -1276,18 +1304,24 @@ export class ModuleMocker {
         );
       }
 
-      descriptor[accessType] = this._makeComponent({type: 'function'}, () => {
-        // @ts-expect-error: mock is assignable
-        descriptor![accessType] = original;
-        Object.defineProperty(object, propertyKey, descriptor!);
-      });
+      descriptor[accessType] = this._makeComponent(
+        {type: 'function'},
+        {
+          reset: () => {
+            this._attachMockImplementation(
+              descriptor![accessType] as Mock,
+              original,
+            );
+          },
+          restore: () => {
+            // @ts-expect-error: overriding original method with a mock
+            descriptor![accessType] = original;
+            Object.defineProperty(object, propertyKey, descriptor!);
+          },
+        },
+      );
 
-      (descriptor[accessType] as Mock).mockImplementation(function (
-        this: unknown,
-      ) {
-        // @ts-expect-error - wrong context
-        return original.apply(this, arguments);
-      });
+      this._attachMockImplementation(descriptor[accessType] as Mock, original);
     }
 
     Object.defineProperty(object, propertyKey, descriptor);
@@ -1392,7 +1426,7 @@ export class ModuleMocker {
       restore: () => {
         restore();
 
-        this._spyState.delete(restore);
+        this._spyState.delete({restore});
       },
     };
 
@@ -1400,7 +1434,7 @@ export class ModuleMocker {
     restore.property = propertyKey;
     restore.replaced = replaced;
 
-    this._spyState.add(restore);
+    this._spyState.add({restore});
 
     return replaced.replaceValue(value);
   }
@@ -1410,14 +1444,15 @@ export class ModuleMocker {
   }
 
   resetAllMocks(): void {
-    this._spyState.forEach(reset => reset());
+    this.clearAllMocks();
     this._mockConfigRegistry = new WeakMap();
-    this._mockState = new WeakMap();
+    this._spyState.forEach(spyState => spyState.reset?.());
   }
 
   restoreAllMocks(): void {
-    this._mockState = new WeakMap();
-    this._spyState.forEach(restore => restore());
+    this.clearAllMocks();
+    this._mockConfigRegistry = new WeakMap();
+    this._spyState.forEach(spyState => spyState.restore());
     this._spyState = new Set();
   }
 


Test patch:
diff --git a/packages/jest-mock/src/__tests__/index.test.ts b/packages/jest-mock/src/__tests__/index.test.ts
--- a/packages/jest-mock/src/__tests__/index.test.ts
+++ b/packages/jest-mock/src/__tests__/index.test.ts
@@ -1247,30 +1247,6 @@ describe('moduleMocker', () => {
     expect(fn.getMockName()).toBe('jest.fn()');
   });
 
-  test('after mock reset, the object should return to its original value', () => {
-    const myObject = {bar: () => 'bar'};
-
-    const barStub = moduleMocker.spyOn(myObject, 'bar');
-
-    barStub.mockReturnValue('POTATO!');
-    expect(myObject.bar()).toBe('POTATO!');
-    barStub.mockReset();
-
-    expect(myObject.bar()).toBe('bar');
-  });
-
-  test('after resetAllMocks, the object should return to its original value', () => {
-    const myObject = {bar: () => 'bar'};
-
-    const barStub = moduleMocker.spyOn(myObject, 'bar');
-
-    barStub.mockReturnValue('POTATO!');
-    expect(myObject.bar()).toBe('POTATO!');
-    moduleMocker.resetAllMocks();
-
-    expect(myObject.bar()).toBe('bar');
-  });
-
   test('mockName gets reset by mockRestore', () => {
     const fn = jest.fn();
     expect(fn.getMockName()).toBe('jest.fn()');
@@ -1344,6 +1320,134 @@ describe('moduleMocker', () => {
       );
     });
 
+    it('supports clearing a spy', () => {
+      let methodOneCalls = 0;
+      const obj = {
+        methodOne() {
+          methodOneCalls++;
+        },
+      };
+
+      const spy1 = moduleMocker.spyOn(obj, 'methodOne');
+
+      obj.methodOne();
+
+      // The spy and the original function are called.
+      expect(methodOneCalls).toBe(1);
+      expect(spy1.mock.calls).toHaveLength(1);
+
+      expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(true);
+
+      spy1.mockClear();
+
+      // After clearing the spy, the method is still mock function.
+      expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(true);
+
+      // After clearing the spy, call count is reset.
+      expect(spy1.mock.calls).toHaveLength(0);
+    });
+
+    it('supports clearing all spies', () => {
+      let methodOneCalls = 0;
+      let methodTwoCalls = 0;
+      const obj = {
+        methodOne() {
+          methodOneCalls++;
+        },
+        methodTwo() {
+          methodTwoCalls++;
+        },
+      };
+
+      const spy1 = moduleMocker.spyOn(obj, 'methodOne');
+      const spy2 = moduleMocker.spyOn(obj, 'methodTwo');
+
+      obj.methodOne();
+      obj.methodTwo();
+
+      // Both spies and both original functions are called.
+      expect(methodOneCalls).toBe(1);
+      expect(methodTwoCalls).toBe(1);
+      expect(spy1.mock.calls).toHaveLength(1);
+      expect(spy2.mock.calls).toHaveLength(1);
+
+      expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(true);
+      expect(moduleMocker.isMockFunction(obj.methodTwo)).toBe(true);
+
+      moduleMocker.clearAllMocks();
+
+      // After clearing all mocks, the methods are still mock functions.
+      expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(true);
+      expect(moduleMocker.isMockFunction(obj.methodTwo)).toBe(true);
+
+      // After clearing all mocks, call counts are reset.
+      expect(spy1.mock.calls).toHaveLength(0);
+      expect(spy2.mock.calls).toHaveLength(0);
+    });
+
+    it('supports resetting a spy', () => {
+      const methodOneReturn = 0;
+      const obj = {
+        methodOne() {
+          return methodOneReturn;
+        },
+      };
+
+      const spy1 = moduleMocker.spyOn(obj, 'methodOne').mockReturnValue(10);
+
+      // Return value is mocked.
+      expect(methodOneReturn).toBe(0);
+      expect(obj.methodOne()).toBe(10);
+
+      expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(true);
+
+      spy1.mockReset();
+
+      // After resetting the spy, the method is still mock functions.
+      expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(true);
+
+      // After resetting the spy, the method returns the original return value.
+      expect(methodOneReturn).toBe(0);
+      expect(obj.methodOne()).toBe(0);
+    });
+
+    it('supports resetting all spies', () => {
+      const methodOneReturn = 10;
+      const methodTwoReturn = 20;
+      const obj = {
+        methodOne() {
+          return methodOneReturn;
+        },
+        methodTwo() {
+          return methodTwoReturn;
+        },
+      };
+
+      moduleMocker.spyOn(obj, 'methodOne').mockReturnValue(100);
+      moduleMocker.spyOn(obj, 'methodTwo').mockReturnValue(200);
+
+      // Return values are mocked.
+      expect(methodOneReturn).toBe(10);
+      expect(methodTwoReturn).toBe(20);
+      expect(obj.methodOne()).toBe(100);
+      expect(obj.methodTwo()).toBe(200);
+
+      expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(true);
+      expect(moduleMocker.isMockFunction(obj.methodTwo)).toBe(true);
+
+      moduleMocker.resetAllMocks();
+
+      // After resetting all mocks, the methods are still mock functions.
+      expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(true);
+      expect(moduleMocker.isMockFunction(obj.methodTwo)).toBe(true);
+
+      // After resetting all mocks, the methods return the original return value.
+      expect(methodOneReturn).toBe(10);
+      expect(methodTwoReturn).toBe(20);
+      expect(obj.methodOne()).toBe(10);
+      expect(obj.methodTwo()).toBe(20);
+    });
+
     it('supports restoring a spy', () => {
       let methodOneCalls = 0;
       const obj = {
@@ -1551,6 +1655,59 @@ describe('moduleMocker', () => {
       );
     });
 
+    it('supports resetting a spy', () => {
+      const methodOneReturn = 0;
+      const obj = {
+        get methodOne() {
+          return methodOneReturn;
+        },
+      };
+
+      const spy1 = moduleMocker
+        .spyOn(obj, 'methodOne', 'get')
+        .mockReturnValue(10);
+
+      // Return value is mocked.
+      expect(methodOneReturn).toBe(0);
+      expect(obj.methodOne).toBe(10);
+
+      spy1.mockReset();
+
+      // After resetting the spy, the method returns the original return value.
+      expect(methodOneReturn).toBe(0);
+      expect(obj.methodOne).toBe(0);
+    });
+
+    it('supports resetting all spies', () => {
+      const methodOneReturn = 10;
+      const methodTwoReturn = 20;
+      const obj = {
+        get methodOne() {
+          return methodOneReturn;
+        },
+        get methodTwo() {
+          return methodTwoReturn;
+        },
+      };
+
+      moduleMocker.spyOn(obj, 'methodOne', 'get').mockReturnValue(100);
+      moduleMocker.spyOn(obj, 'methodTwo', 'get').mockReturnValue(200);
+
+      // Return values are mocked.
+      expect(methodOneReturn).toBe(10);
+      expect(methodTwoReturn).toBe(20);
+      expect(obj.methodOne).toBe(100);
+      expect(obj.methodTwo).toBe(200);
+
+      moduleMocker.resetAllMocks();
+
+      // After resetting all mocks, the methods return the original return value.
+      expect(methodOneReturn).toBe(10);
+      expect(methodTwoReturn).toBe(20);
+      expect(obj.methodOne).toBe(10);
+      expect(obj.methodTwo).toBe(20);
+    });
+
     it('supports restoring a spy', () => {
       let methodOneCalls = 0;
       const obj = {
@@ -1683,6 +1840,61 @@ describe('moduleMocker', () => {
       expect(obj.property).toBe(true);
     });
 
+    it('supports resetting a spy on the prototype chain', () => {
+      const methodOneReturn = 0;
+      const prototype = {
+        get methodOne() {
+          return methodOneReturn;
+        },
+      };
+      const obj = Object.create(prototype, {});
+
+      const spy1 = moduleMocker
+        .spyOn(obj, 'methodOne', 'get')
+        .mockReturnValue(10);
+
+      // Return value is mocked.
+      expect(methodOneReturn).toBe(0);
+      expect(obj.methodOne).toBe(10);
+
+      spy1.mockReset();
+
+      // After resetting the spy, the method returns the original return value.
+      expect(methodOneReturn).toBe(0);
+      expect(obj.methodOne).toBe(0);
+    });
+
+    it('supports resetting all spies on the prototype chain', () => {
+      const methodOneReturn = 10;
+      const methodTwoReturn = 20;
+      const prototype = {
+        get methodOne() {
+          return methodOneReturn;
+        },
+        get methodTwo() {
+          return methodTwoReturn;
+        },
+      };
+      const obj = Object.create(prototype, {});
+
+      moduleMocker.spyOn(obj, 'methodOne', 'get').mockReturnValue(100);
+      moduleMocker.spyOn(obj, 'methodTwo', 'get').mockReturnValue(200);
+
+      // Return values are mocked.
+      expect(methodOneReturn).toBe(10);
+      expect(methodTwoReturn).toBe(20);
+      expect(obj.methodOne).toBe(100);
+      expect(obj.methodTwo).toBe(200);
+
+      moduleMocker.resetAllMocks();
+
+      // After resetting all mocks, the methods return the original return value.
+      expect(methodOneReturn).toBe(10);
+      expect(methodTwoReturn).toBe(20);
+      expect(obj.methodOne).toBe(10);
+      expect(obj.methodTwo).toBe(20);
+    });
+
     it('supports restoring a spy on the prototype chain', () => {
       let methodOneCalls = 0;
       const prototype = {