From 6c0b1552b4d45f46cd0cf90b896c4ac5b63f284c Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Tue, 15 Aug 2023 11:10:50 +0100 Subject: [PATCH 1/7] Revert "fix(jest-mock): do not restore mocks when `jest.resetAllMocks()` is called (#13866)" This reverts commit 9432fc38105dec12ffc188b7ef25d734854aec52. --- .../jest-mock/src/__tests__/index.test.ts | 260 ++---------------- packages/jest-mock/src/index.ts | 151 ++++------ 2 files changed, 82 insertions(+), 329 deletions(-) diff --git a/packages/jest-mock/src/__tests__/index.test.ts b/packages/jest-mock/src/__tests__/index.test.ts index 7d08d17203df..da8fefb5b45d 100644 --- a/packages/jest-mock/src/__tests__/index.test.ts +++ b/packages/jest-mock/src/__tests__/index.test.ts @@ -1247,6 +1247,30 @@ 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()'); @@ -1481,134 +1505,6 @@ describe('moduleMocker', () => { expect(spy).toHaveBeenCalled(); }); - 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 = { @@ -1814,59 +1710,6 @@ 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 = { @@ -1999,61 +1842,6 @@ 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 = { diff --git a/packages/jest-mock/src/index.ts b/packages/jest-mock/src/index.ts index 8989c5320bf3..ea021839ebf8 100644 --- a/packages/jest-mock/src/index.ts +++ b/packages/jest-mock/src/index.ts @@ -254,8 +254,6 @@ type MockFunctionConfig = { specificMockImpls: Array; }; -type SpyState = {reset?: () => void; restore: () => void}; - const MOCK_CONSTRUCTOR_NAME = 'mockConstructor'; const FUNCTION_NAME_RESERVED_PATTERN = /[\s!-/:-@[-`{-~]/; @@ -504,8 +502,9 @@ export class ModuleMocker { private readonly _environmentGlobal: typeof globalThis; private _mockState: WeakMap; private _mockConfigRegistry: WeakMap; - private _spyState: Set; + private _spyState: Set<() => void>; private _invocationCallCounter: number; + private _originalFn: WeakMap; /** * @see README.md @@ -518,6 +517,7 @@ export class ModuleMocker { this._mockConfigRegistry = new WeakMap(); this._spyState = new Set(); this._invocationCallCounter = 1; + this._originalFn = new WeakMap(); } private _getSlots(object?: Record): Array { @@ -609,27 +609,27 @@ export class ModuleMocker { private _makeComponent>( metadata: MockMetadata, - spyState?: SpyState, + restore?: () => void, ): T; private _makeComponent>( metadata: MockMetadata, - spyState?: SpyState, + restore?: () => void, ): T; private _makeComponent( metadata: MockMetadata, - spyState?: SpyState, + restore?: () => void, ): T; private _makeComponent( metadata: MockMetadata, - spyState?: SpyState, + restore?: () => void, ): T; private _makeComponent( metadata: MockMetadata, - spyState?: SpyState, + restore?: () => void, ): Mock; private _makeComponent( metadata: MockMetadata, - spyState?: SpyState, + restore?: () => void, ): Record | Array | RegExp | T | Mock | undefined { if (metadata.type === 'object') { return new this._environmentGlobal.Object(); @@ -750,8 +750,8 @@ export class ModuleMocker { f._isMockFunction = true; f.getMockImplementation = () => this._ensureMockConfig(f).mockImpl as T; - if (spyState != null) { - this._spyState.add(spyState); + if (typeof restore === 'function') { + this._spyState.add(restore); } this._mockState.set(f, this._defaultMockState()); @@ -771,23 +771,18 @@ export class ModuleMocker { f.mockReset = () => { f.mockClear(); - this._mockConfigRegistry.delete(f); - - if (spyState != null) { - spyState.reset?.(); - } - + const originalFn = this._originalFn.get(f); + const originalMockImpl = { + ...this._defaultMockConfig(), + mockImpl: originalFn, + }; + this._mockConfigRegistry.set(f, originalMockImpl); return f; }; f.mockRestore = () => { - f.mockClear(); - this._mockConfigRegistry.delete(f); - - if (spyState != null) { - spyState.restore(); - this._spyState.delete(spyState); - } + f.mockReset(); + return restore ? restore() : undefined; }; f.mockReturnValueOnce = (value: ReturnType) => @@ -995,14 +990,14 @@ export class ModuleMocker { object: T, propertyKey: K, ): ReplacedPropertyRestorer | undefined { - for (const {restore} of this._spyState) { + for (const spyState of this._spyState) { if ( - 'object' in restore && - 'property' in restore && - restore.object === object && - restore.property === propertyKey + 'object' in spyState && + 'property' in spyState && + spyState.object === object && + spyState.property === propertyKey ) { - return restore as ReplacedPropertyRestorer; + return spyState as ReplacedPropertyRestorer; } } @@ -1118,15 +1113,6 @@ export class ModuleMocker { return fn; } - private _attachMockImplementation( - mock: Mock, - original: T, - ) { - mock.mockImplementation(function (this: unknown) { - return original.apply(this, arguments); - }); - } - spyOn< T extends object, K extends PropertyLikeKeys, @@ -1217,43 +1203,29 @@ export class ModuleMocker { if (descriptor && descriptor.get) { const originalGet = descriptor.get; - mock = this._makeComponent( - {type: 'function'}, - { - reset: () => { - this._attachMockImplementation(mock, original); - }, - restore: () => { - descriptor!.get = originalGet; - Object.defineProperty(object, methodKey, descriptor!); - }, - }, - ); + mock = this._makeComponent({type: 'function'}, () => { + descriptor!.get = originalGet; + Object.defineProperty(object, methodKey, descriptor!); + }); descriptor.get = () => mock; Object.defineProperty(object, methodKey, descriptor); } else { - 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 + mock = this._makeComponent({type: 'function'}, () => { + if (isMethodOwner) { + object[methodKey] = original; + } else { + delete object[methodKey]; + } + }); + // @ts-expect-error overriding original method with a Mock object[methodKey] = mock; } - this._attachMockImplementation(mock, original); + mock.mockImplementation(function (this: unknown) { + return original.apply(this, arguments); + }); } - + this._originalFn.set(object[methodKey] as Mock, original); return object[methodKey] as Mock; } @@ -1311,24 +1283,18 @@ export class ModuleMocker { ); } - 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] = this._makeComponent({type: 'function'}, () => { + // @ts-expect-error: mock is assignable + descriptor![accessType] = original; + Object.defineProperty(object, propertyKey, descriptor!); + }); - this._attachMockImplementation(descriptor[accessType] as Mock, original); + (descriptor[accessType] as Mock).mockImplementation(function ( + this: unknown, + ) { + // @ts-expect-error - wrong context + return original.apply(this, arguments); + }); } Object.defineProperty(object, propertyKey, descriptor); @@ -1434,7 +1400,7 @@ export class ModuleMocker { restore: () => { restore(); - this._spyState.delete({restore}); + this._spyState.delete(restore); }, }; @@ -1442,7 +1408,7 @@ export class ModuleMocker { restore.property = propertyKey; restore.replaced = replaced; - this._spyState.add({restore}); + this._spyState.add(restore); return replaced.replaceValue(value); } @@ -1452,15 +1418,14 @@ export class ModuleMocker { } resetAllMocks(): void { - this.clearAllMocks(); + this._spyState.forEach(reset => reset()); this._mockConfigRegistry = new WeakMap(); - this._spyState.forEach(spyState => spyState.reset?.()); + this._mockState = new WeakMap(); } restoreAllMocks(): void { - this.clearAllMocks(); - this._mockConfigRegistry = new WeakMap(); - this._spyState.forEach(spyState => spyState.restore()); + this._mockState = new WeakMap(); + this._spyState.forEach(restore => restore()); this._spyState = new Set(); } From a5e2404905e748059ef1eb1ab698ecc0576366c1 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Tue, 15 Aug 2023 14:02:04 +0100 Subject: [PATCH 2/7] Revert "fix(jest-mock): clear mock when `jest.restoreAllMocks()` is called (#13867)" This reverts commit 66fb417178edca466b0afc634b0c88fe32f9884f. --- .../jest-mock/src/__tests__/index.test.ts | 130 +++--------------- packages/jest-mock/src/index.ts | 1 - 2 files changed, 19 insertions(+), 112 deletions(-) diff --git a/packages/jest-mock/src/__tests__/index.test.ts b/packages/jest-mock/src/__tests__/index.test.ts index da8fefb5b45d..6ba5685e89f8 100644 --- a/packages/jest-mock/src/__tests__/index.test.ts +++ b/packages/jest-mock/src/__tests__/index.test.ts @@ -1505,36 +1505,6 @@ describe('moduleMocker', () => { expect(spy).toHaveBeenCalled(); }); - it('supports restoring a spy', () => { - let methodOneCalls = 0; - const obj = { - methodOne() { - methodOneCalls++; - }, - }; - - const spy1 = moduleMocker.spyOn(obj, 'methodOne'); - - obj.methodOne(); - - // The spy and the original function got called. - expect(methodOneCalls).toBe(1); - expect(spy1.mock.calls).toHaveLength(1); - - expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(true); - - spy1.mockRestore(); - - // After restoring the spy, the method is not mock function. - expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(false); - - obj.methodOne(); - - // After restoring the spy only the real method bumps its call count, not the spy. - expect(methodOneCalls).toBe(2); - expect(spy1.mock.calls).toHaveLength(0); - }); - it('supports restoring all spies', () => { let methodOneCalls = 0; let methodTwoCalls = 0; @@ -1550,32 +1520,25 @@ describe('moduleMocker', () => { const spy1 = moduleMocker.spyOn(obj, 'methodOne'); const spy2 = moduleMocker.spyOn(obj, 'methodTwo'); + // First, we call with the spies: both spies and both original functions + // should be called. obj.methodOne(); obj.methodTwo(); - - // Both spies and both original functions got 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.restoreAllMocks(); - // After restoring all mocks, the methods are not mock functions. - expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(false); - expect(moduleMocker.isMockFunction(obj.methodTwo)).toBe(false); - + // Then, after resetting all mocks, we call methods again. Only the real + // methods should bump their count, not the spies. obj.methodOne(); obj.methodTwo(); - - // After restoring all mocks only the real methods bump their count, not the spies. expect(methodOneCalls).toBe(2); expect(methodTwoCalls).toBe(2); - expect(spy1.mock.calls).toHaveLength(0); - expect(spy2.mock.calls).toHaveLength(0); + expect(spy1.mock.calls).toHaveLength(1); + expect(spy2.mock.calls).toHaveLength(1); }); it('should work with getters', () => { @@ -1710,33 +1673,6 @@ describe('moduleMocker', () => { ); }); - it('supports restoring a spy', () => { - let methodOneCalls = 0; - const obj = { - get methodOne() { - return function () { - methodOneCalls++; - }; - }, - }; - - const spy1 = moduleMocker.spyOn(obj, 'methodOne', 'get'); - - obj.methodOne(); - - // The spy and the original function are called. - expect(methodOneCalls).toBe(1); - expect(spy1.mock.calls).toHaveLength(1); - - spy1.mockRestore(); - - obj.methodOne(); - - // After restoring the spy only the real method bumps its call count, not the spy. - expect(methodOneCalls).toBe(2); - expect(spy1.mock.calls).toHaveLength(0); - }); - it('supports restoring all spies', () => { let methodOneCalls = 0; let methodTwoCalls = 0; @@ -1756,10 +1692,10 @@ describe('moduleMocker', () => { const spy1 = moduleMocker.spyOn(obj, 'methodOne', 'get'); const spy2 = moduleMocker.spyOn(obj, 'methodTwo', 'get'); + // First, we call with the spies: both spies and both original functions + // should be called. obj.methodOne(); obj.methodTwo(); - - // Both spies and both original functions got called. expect(methodOneCalls).toBe(1); expect(methodTwoCalls).toBe(1); expect(spy1.mock.calls).toHaveLength(1); @@ -1767,14 +1703,14 @@ describe('moduleMocker', () => { moduleMocker.restoreAllMocks(); + // Then, after resetting all mocks, we call methods again. Only the real + // methods should bump their count, not the spies. obj.methodOne(); obj.methodTwo(); - - // After restoring all mocks only the real methods bump their count, not the spies. expect(methodOneCalls).toBe(2); expect(methodTwoCalls).toBe(2); - expect(spy1.mock.calls).toHaveLength(0); - expect(spy2.mock.calls).toHaveLength(0); + expect(spy1.mock.calls).toHaveLength(1); + expect(spy2.mock.calls).toHaveLength(1); }); it('should work with getters on the prototype chain', () => { @@ -1842,34 +1778,6 @@ describe('moduleMocker', () => { expect(obj.property).toBe(true); }); - it('supports restoring a spy on the prototype chain', () => { - let methodOneCalls = 0; - const prototype = { - get methodOne() { - return function () { - methodOneCalls++; - }; - }, - }; - const obj = Object.create(prototype, {}); - - const spy1 = moduleMocker.spyOn(obj, 'methodOne', 'get'); - - obj.methodOne(); - - // The spy and the original function are called. - expect(methodOneCalls).toBe(1); - expect(spy1.mock.calls).toHaveLength(1); - - spy1.mockRestore(); - - obj.methodOne(); - - // After restoring the spy only the real method bumps its call count, not the spy. - expect(methodOneCalls).toBe(2); - expect(spy1.mock.calls).toHaveLength(0); - }); - it('supports restoring all spies on the prototype chain', () => { let methodOneCalls = 0; let methodTwoCalls = 0; @@ -1890,10 +1798,10 @@ describe('moduleMocker', () => { const spy1 = moduleMocker.spyOn(obj, 'methodOne', 'get'); const spy2 = moduleMocker.spyOn(obj, 'methodTwo', 'get'); + // First, we call with the spies: both spies and both original functions + // should be called. obj.methodOne(); obj.methodTwo(); - - // Both spies and both original functions got called. expect(methodOneCalls).toBe(1); expect(methodTwoCalls).toBe(1); expect(spy1.mock.calls).toHaveLength(1); @@ -1901,14 +1809,14 @@ describe('moduleMocker', () => { moduleMocker.restoreAllMocks(); + // Then, after resetting all mocks, we call methods again. Only the real + // methods should bump their count, not the spies. obj.methodOne(); obj.methodTwo(); - - // After restoring all mocks only the real methods bump their count, not the spies. expect(methodOneCalls).toBe(2); expect(methodTwoCalls).toBe(2); - expect(spy1.mock.calls).toHaveLength(0); - expect(spy2.mock.calls).toHaveLength(0); + expect(spy1.mock.calls).toHaveLength(1); + expect(spy2.mock.calls).toHaveLength(1); }); }); @@ -1947,7 +1855,7 @@ describe('moduleMocker', () => { expect(obj.property).toBe(1); }); - it('should allow mocking with value of different type', () => { + it('should allow mocking with value of different value', () => { const obj = { property: 1, }; diff --git a/packages/jest-mock/src/index.ts b/packages/jest-mock/src/index.ts index ea021839ebf8..21556c612c03 100644 --- a/packages/jest-mock/src/index.ts +++ b/packages/jest-mock/src/index.ts @@ -1424,7 +1424,6 @@ export class ModuleMocker { } restoreAllMocks(): void { - this._mockState = new WeakMap(); this._spyState.forEach(restore => restore()); this._spyState = new Set(); } From 091ed55f4f281ff0d4e5af10b9ed2cbadb337fd0 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Tue, 15 Aug 2023 14:15:14 +0100 Subject: [PATCH 3/7] Revert "fix: stop changing the behaviour of spies on mockFn.mockReset (#13692)" This reverts commit eace3a1fefe95af3325b2de5374939788bbf6d34. --- docs/MockFunctionAPI.md | 2 +- .../jest-mock/src/__tests__/index.test.ts | 24 ------------------- packages/jest-mock/src/index.ts | 12 ++-------- 3 files changed, 3 insertions(+), 35 deletions(-) diff --git a/docs/MockFunctionAPI.md b/docs/MockFunctionAPI.md index 5ffb11992b84..6bc4bcd24120 100644 --- a/docs/MockFunctionAPI.md +++ b/docs/MockFunctionAPI.md @@ -130,7 +130,7 @@ Beware that `mockFn.mockClear()` will replace `mockFn.mock`, not just reset the Does everything that [`mockFn.mockClear()`](#mockfnmockclear) does, and also removes any mocked return values or implementations. -This is useful when you want to completely reset a _mock_ back to its initial state. +This is useful when you want to completely reset a _mock_ back to its initial state. (Note that resetting a _spy_ will result in a function with no return value). The [`resetMocks`](configuration#resetmocks-boolean) configuration option is available to reset mocks automatically before each test. diff --git a/packages/jest-mock/src/__tests__/index.test.ts b/packages/jest-mock/src/__tests__/index.test.ts index 6ba5685e89f8..a52f5b4921bc 100644 --- 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()'); diff --git a/packages/jest-mock/src/index.ts b/packages/jest-mock/src/index.ts index 21556c612c03..296934a66a85 100644 --- a/packages/jest-mock/src/index.ts +++ b/packages/jest-mock/src/index.ts @@ -504,7 +504,6 @@ export class ModuleMocker { private _mockConfigRegistry: WeakMap; private _spyState: Set<() => void>; private _invocationCallCounter: number; - private _originalFn: WeakMap; /** * @see README.md @@ -517,7 +516,6 @@ export class ModuleMocker { this._mockConfigRegistry = new WeakMap(); this._spyState = new Set(); this._invocationCallCounter = 1; - this._originalFn = new WeakMap(); } private _getSlots(object?: Record): Array { @@ -771,12 +769,7 @@ 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); return f; }; @@ -1225,7 +1218,7 @@ export class ModuleMocker { return original.apply(this, arguments); }); } - this._originalFn.set(object[methodKey] as Mock, original); + return object[methodKey] as Mock; } @@ -1418,7 +1411,6 @@ export class ModuleMocker { } resetAllMocks(): void { - this._spyState.forEach(reset => reset()); this._mockConfigRegistry = new WeakMap(); this._mockState = new WeakMap(); } From 56ddb9f11e20f242b8b27e2440b72acaea8e2d4d Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Thu, 17 Aug 2023 17:03:36 +0100 Subject: [PATCH 4/7] Re-add tests with amended (29.3) expectations --- .../jest-mock/src/__tests__/index.test.ts | 339 ++++++++++++++++++ 1 file changed, 339 insertions(+) diff --git a/packages/jest-mock/src/__tests__/index.test.ts b/packages/jest-mock/src/__tests__/index.test.ts index a52f5b4921bc..1beaecb7c7cd 100644 --- a/packages/jest-mock/src/__tests__/index.test.ts +++ b/packages/jest-mock/src/__tests__/index.test.ts @@ -1481,6 +1481,185 @@ describe('moduleMocker', () => { expect(spy).toHaveBeenCalled(); }); + 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 = 10; + let methodOneRealCalls = 0; + const obj = { + methodOne() { + methodOneRealCalls++; + return methodOneReturn; + }, + }; + + const spy1 = moduleMocker.spyOn(obj, 'methodOne').mockReturnValue(100); + + // Return value is mocked. + expect(obj.methodOne()).toBe(100); + // Real impl has not been used. + expect(methodOneRealCalls).toBe(0); + + 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 undefined. + expect(obj.methodOne()).toBeUndefined(); + + // Real implementation has still not been called. + expect(methodOneRealCalls).toBe(0); + }); + + it('supports resetting all spies', () => { + const methodOneReturn = 10; + const methodTwoReturn = {}; + let methodOneRealCalls = 0; + let methodTwoRealCalls = 0; + const obj = { + methodOne() { + methodOneRealCalls++; + return methodOneReturn; + }, + methodTwo() { + methodTwoRealCalls++; + return methodTwoReturn; + }, + }; + + // methodOne is spied on and mocked. + moduleMocker.spyOn(obj, 'methodOne').mockReturnValue(100); + // methodTwo is spied on but not mocked. + moduleMocker.spyOn(obj, 'methodTwo'); + + // Return values are mocked. + expect(obj.methodOne()).toBe(100); + expect(obj.methodTwo()).toBe(methodTwoReturn); + + // The real implementation has not been called when mocked. + expect(methodOneRealCalls).toBe(0); + + // But has for the unmocked spy. + expect(methodTwoRealCalls).toBe(1); + + // Both are mock functions. + 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 are stubs returning undefined. + expect(obj.methodOne()).toBeUndefined(); + + // NB: It may not be desirable for reset to stub a spy that was never mocked - + // consider changing in a future major. + expect(obj.methodTwo()).toBeUndefined(); + + // Real functions have not been called any more times. + expect(methodOneRealCalls).toBe(0); + expect(methodTwoRealCalls).toBe(1); + }); + + it('supports restoring a spy', () => { + let methodOneCalls = 0; + const obj = { + methodOne() { + methodOneCalls++; + }, + }; + + const spy1 = moduleMocker.spyOn(obj, 'methodOne'); + + obj.methodOne(); + + // The spy and the original function got called. + expect(methodOneCalls).toBe(1); + expect(spy1.mock.calls).toHaveLength(1); + + expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(true); + + spy1.mockRestore(); + + // After restoring the spy, the method is not mock function. + expect(moduleMocker.isMockFunction(obj.methodOne)).toBe(false); + + obj.methodOne(); + + // After restoring the spy only the real method bumps its call count, not the spy. + expect(methodOneCalls).toBe(2); + expect(spy1.mock.calls).toHaveLength(0); + }); + it('supports restoring all spies', () => { let methodOneCalls = 0; let methodTwoCalls = 0; @@ -1649,6 +1828,85 @@ describe('moduleMocker', () => { ); }); + it('supports resetting a spy', () => { + const methodOneReturn = 0; + let methodOneRealCalls = 0; + const obj = { + get methodOne() { + methodOneRealCalls++; + return methodOneReturn; + }, + }; + + const spy1 = moduleMocker + .spyOn(obj, 'methodOne', 'get') + .mockReturnValue(10); + + // Return value is mocked. + expect(obj.methodOne).toBe(10); + + spy1.mockReset(); + + // After resetting the spy, the getter is a stub returning undefined + expect(obj.methodOne).toBeUndefined(); + expect(methodOneRealCalls).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 are stubs + expect(obj.methodOne).toBeUndefined(); + expect(obj.methodTwo).toBeUndefined(); + }); + + it('supports restoring a spy', () => { + let methodOneCalls = 0; + const obj = { + get methodOne() { + return function () { + methodOneCalls++; + }; + }, + }; + + const spy1 = moduleMocker.spyOn(obj, 'methodOne', 'get'); + + obj.methodOne(); + + // The spy and the original function are called. + expect(methodOneCalls).toBe(1); + expect(spy1.mock.calls).toHaveLength(1); + + spy1.mockRestore(); + + obj.methodOne(); + + // After restoring the spy only the real method bumps its call count, not the spy. + expect(methodOneCalls).toBe(2); + expect(spy1.mock.calls).toHaveLength(0); + }); + it('supports restoring all spies', () => { let methodOneCalls = 0; let methodTwoCalls = 0; @@ -1754,6 +2012,87 @@ describe('moduleMocker', () => { expect(obj.property).toBe(true); }); + it('supports resetting a spy on the prototype chain', () => { + let methodOneRealCalls = 0; + const prototype = { + get methodOne() { + methodOneRealCalls++; + return 1; + }, + }; + const obj = Object.create(prototype, {}); + + const spy1 = moduleMocker + .spyOn(obj, 'methodOne', 'get') + .mockReturnValue(10); + + // Return value is mocked. + expect(obj.methodOne).toBe(10); + + spy1.mockReset(); + + // After resetting the spy, the method is a stub. + expect(obj.methodOne).toBeUndefined(); + + // The real implementation has not been used. + expect(methodOneRealCalls).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(obj.methodOne).toBe(100); + expect(obj.methodTwo).toBe(200); + + moduleMocker.resetAllMocks(); + + // After resetting all mocks, the methods are stubs + expect(obj.methodOne).toBeUndefined(); + expect(obj.methodTwo).toBeUndefined(); + }); + + it('supports restoring a spy on the prototype chain', () => { + let methodOneCalls = 0; + const prototype = { + get methodOne() { + return function () { + methodOneCalls++; + }; + }, + }; + const obj = Object.create(prototype, {}); + + const spy1 = moduleMocker.spyOn(obj, 'methodOne', 'get'); + + obj.methodOne(); + + // The spy and the original function are called, because we have not mocked it. + expect(methodOneCalls).toBe(1); + expect(spy1.mock.calls).toHaveLength(1); + + spy1.mockRestore(); + + obj.methodOne(); + + // After restoring the spy only the real method bumps its call count, not the spy. + expect(methodOneCalls).toBe(2); + expect(spy1.mock.calls).toHaveLength(0); + }); + it('supports restoring all spies on the prototype chain', () => { let methodOneCalls = 0; let methodTwoCalls = 0; From 436f51808c8ece23f168827037ceb92f613baf9a Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Thu, 17 Aug 2023 17:04:03 +0100 Subject: [PATCH 5/7] Add new tests to assert 29.3 behaviour --- .../jest-mock/src/__tests__/index.test.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/jest-mock/src/__tests__/index.test.ts b/packages/jest-mock/src/__tests__/index.test.ts index 1beaecb7c7cd..c0efe908e7d9 100644 --- a/packages/jest-mock/src/__tests__/index.test.ts +++ b/packages/jest-mock/src/__tests__/index.test.ts @@ -598,6 +598,26 @@ describe('moduleMocker', () => { expect(fn2()).not.toBe('abcd'); }); + it('is not affected by restoreAllMocks', () => { + const fn1 = moduleMocker.fn(); + fn1.mockImplementation(() => 'abcd'); + fn1(1, 2, 3); + expect(fn1.mock.calls).toEqual([[1, 2, 3]]); + moduleMocker.restoreAllMocks(); + expect(fn1(1)).toBe('abcd'); + expect(fn1.mock.calls).toEqual([[1, 2, 3], [1]]); + }); + + it('is cleared and stubbed when restored explicitly', () => { + const fn1 = moduleMocker.fn(); + fn1.mockImplementation(() => 'abcd'); + fn1(1, 2, 3); + expect(fn1.mock.calls).toEqual([[1, 2, 3]]); + fn1.mockRestore(); + expect(fn1(1)).toBeUndefined(); + expect(fn1.mock.calls).toEqual([[1]]); + }); + it('maintains function arity', () => { const mockFunctionArity1 = moduleMocker.fn(x => x); const mockFunctionArity2 = moduleMocker.fn((x, y) => y); From a3e19c9d2fe2806d85964f5fc70d1f73b39a39fb Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Fri, 18 Aug 2023 18:17:00 +0100 Subject: [PATCH 6/7] Add changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bbc4accd8ca..010e2c3aa459 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ - `[expect]` Remove `@types/node` from dependencies ([#14385](https://github.com/jestjs/jest/pull/14385)) - `[jest-core]` Use workers in watch mode by default to avoid crashes ([#14059](https://github.com/facebook/jest/pull/14059) & [#14085](https://github.com/facebook/jest/pull/14085)). - `[jest-reporters]` Update `istanbul-lib-instrument` dependency to v6. ([#14401](https://github.com/jestjs/jest/pull/14401)) +- `[jest-mock]` Revert [#13692](https://github.com/jestjs/jest/pull/13692) as it was a breaking change ([#14429](https://github.com/jestjs/jest/pull/14429)) +- `[jest-mock]` Revert [#13866](https://github.com/jestjs/jest/pull/13866) as it was a breaking change ([#14429](https://github.com/jestjs/jest/pull/14429)) +- `[jest-mock]` Revert [#13867](https://github.com/jestjs/jest/pull/13867) as it was a breaking change ([#14429](https://github.com/jestjs/jest/pull/14429)) ### Chore & Maintenance From abced97744102f5f51a98eddb531289f4240e8ce Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Mon, 21 Aug 2023 10:54:45 +0100 Subject: [PATCH 7/7] Clarify docs - wording of mockRestore --- docs/MockFunctionAPI.md | 4 +--- website/versioned_docs/version-29.4/MockFunctionAPI.md | 4 +--- website/versioned_docs/version-29.5/MockFunctionAPI.md | 4 +--- website/versioned_docs/version-29.6/MockFunctionAPI.md | 4 +--- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/docs/MockFunctionAPI.md b/docs/MockFunctionAPI.md index 6bc4bcd24120..6282e1538f2d 100644 --- a/docs/MockFunctionAPI.md +++ b/docs/MockFunctionAPI.md @@ -128,9 +128,7 @@ Beware that `mockFn.mockClear()` will replace `mockFn.mock`, not just reset the ### `mockFn.mockReset()` -Does everything that [`mockFn.mockClear()`](#mockfnmockclear) does, and also removes any mocked return values or implementations. - -This is useful when you want to completely reset a _mock_ back to its initial state. (Note that resetting a _spy_ will result in a function with no return value). +Does everything that [`mockFn.mockClear()`](#mockfnmockclear) does, and also replaces the mock implementation with an empty function, returning `undefined`. The [`resetMocks`](configuration#resetmocks-boolean) configuration option is available to reset mocks automatically before each test. diff --git a/website/versioned_docs/version-29.4/MockFunctionAPI.md b/website/versioned_docs/version-29.4/MockFunctionAPI.md index 5ffb11992b84..6282e1538f2d 100644 --- a/website/versioned_docs/version-29.4/MockFunctionAPI.md +++ b/website/versioned_docs/version-29.4/MockFunctionAPI.md @@ -128,9 +128,7 @@ Beware that `mockFn.mockClear()` will replace `mockFn.mock`, not just reset the ### `mockFn.mockReset()` -Does everything that [`mockFn.mockClear()`](#mockfnmockclear) does, and also removes any mocked return values or implementations. - -This is useful when you want to completely reset a _mock_ back to its initial state. +Does everything that [`mockFn.mockClear()`](#mockfnmockclear) does, and also replaces the mock implementation with an empty function, returning `undefined`. The [`resetMocks`](configuration#resetmocks-boolean) configuration option is available to reset mocks automatically before each test. diff --git a/website/versioned_docs/version-29.5/MockFunctionAPI.md b/website/versioned_docs/version-29.5/MockFunctionAPI.md index 5ffb11992b84..6282e1538f2d 100644 --- a/website/versioned_docs/version-29.5/MockFunctionAPI.md +++ b/website/versioned_docs/version-29.5/MockFunctionAPI.md @@ -128,9 +128,7 @@ Beware that `mockFn.mockClear()` will replace `mockFn.mock`, not just reset the ### `mockFn.mockReset()` -Does everything that [`mockFn.mockClear()`](#mockfnmockclear) does, and also removes any mocked return values or implementations. - -This is useful when you want to completely reset a _mock_ back to its initial state. +Does everything that [`mockFn.mockClear()`](#mockfnmockclear) does, and also replaces the mock implementation with an empty function, returning `undefined`. The [`resetMocks`](configuration#resetmocks-boolean) configuration option is available to reset mocks automatically before each test. diff --git a/website/versioned_docs/version-29.6/MockFunctionAPI.md b/website/versioned_docs/version-29.6/MockFunctionAPI.md index 5ffb11992b84..6282e1538f2d 100644 --- a/website/versioned_docs/version-29.6/MockFunctionAPI.md +++ b/website/versioned_docs/version-29.6/MockFunctionAPI.md @@ -128,9 +128,7 @@ Beware that `mockFn.mockClear()` will replace `mockFn.mock`, not just reset the ### `mockFn.mockReset()` -Does everything that [`mockFn.mockClear()`](#mockfnmockclear) does, and also removes any mocked return values or implementations. - -This is useful when you want to completely reset a _mock_ back to its initial state. +Does everything that [`mockFn.mockClear()`](#mockfnmockclear) does, and also replaces the mock implementation with an empty function, returning `undefined`. The [`resetMocks`](configuration#resetmocks-boolean) configuration option is available to reset mocks automatically before each test.