From 8d3172568e3769fac3c3e6bb3f077b871e6f66ad Mon Sep 17 00:00:00 2001 From: Peter Staples Date: Thu, 6 Oct 2022 12:22:41 +0100 Subject: [PATCH 01/13] Revert "Revert "fix: mocking of getters/setters on automatically mocked classes (#13145)"" This reverts commit 52d45be1b50fa38864d46c908a8a1116943a54b3. # Conflicts: # CHANGELOG.md --- jest.config.mjs | 3 + .../jest-mock/src/__tests__/SuperTestClass.ts | 36 +++ packages/jest-mock/src/__tests__/TestClass.ts | 11 + .../__tests__/class-mocks-dual-import.test.ts | 130 +++++++++ .../class-mocks-single-import.test.ts | 128 +++++++++ .../src/__tests__/class-mocks-types.ts | 38 +++ .../src/__tests__/class-mocks.test.ts | 175 ++++++++++++ .../jest-mock/src/__tests__/index.test.ts | 2 +- packages/jest-mock/src/index.ts | 270 ++++++++++-------- 9 files changed, 679 insertions(+), 114 deletions(-) create mode 100644 packages/jest-mock/src/__tests__/SuperTestClass.ts create mode 100644 packages/jest-mock/src/__tests__/TestClass.ts create mode 100644 packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts create mode 100644 packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts create mode 100644 packages/jest-mock/src/__tests__/class-mocks-types.ts create mode 100644 packages/jest-mock/src/__tests__/class-mocks.test.ts diff --git a/jest.config.mjs b/jest.config.mjs index 192ccd07dcfa..5459b96db2b2 100644 --- a/jest.config.mjs +++ b/jest.config.mjs @@ -58,6 +58,9 @@ export default { '/packages/jest-haste-map/src/__tests__/haste_impl.js', '/packages/jest-haste-map/src/__tests__/dependencyExtractor.js', '/packages/jest-haste-map/src/__tests__/test_dotfiles_root/', + '/packages/jest-mock/src/__tests__/class-mocks-types.ts', + '/packages/jest-mock/src/__tests__/TestClass.ts', + '/packages/jest-mock/src/__tests__/SuperTestClass.ts', '/packages/jest-repl/src/__tests__/test_root', '/packages/jest-resolve-dependencies/src/__tests__/__fixtures__/', '/packages/jest-runtime/src/__tests__/defaultResolver.js', diff --git a/packages/jest-mock/src/__tests__/SuperTestClass.ts b/packages/jest-mock/src/__tests__/SuperTestClass.ts new file mode 100644 index 000000000000..ae1c262e34f6 --- /dev/null +++ b/packages/jest-mock/src/__tests__/SuperTestClass.ts @@ -0,0 +1,36 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +export class SuperTestClass { + static staticTestProperty = 'staticTestProperty'; + + static get staticTestAccessor(): string { + return 'staticTestAccessor'; + } + + static set staticTestAccessor(_x: string) { + return; + } + + static staticTestMethod(): string { + return 'staticTestMethod'; + } + + testProperty = 'testProperty'; + + get testAccessor(): string { + return 'testAccessor'; + } + set testAccessor(_x: string) { + return; + } + + testMethod(): string { + return 'testMethod'; + } +} diff --git a/packages/jest-mock/src/__tests__/TestClass.ts b/packages/jest-mock/src/__tests__/TestClass.ts new file mode 100644 index 000000000000..e4197db98abe --- /dev/null +++ b/packages/jest-mock/src/__tests__/TestClass.ts @@ -0,0 +1,11 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +import {SuperTestClass} from './SuperTestClass'; + +export default class TestClass extends SuperTestClass {} diff --git a/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts b/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts new file mode 100644 index 000000000000..510484d06700 --- /dev/null +++ b/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts @@ -0,0 +1,130 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +import {SuperTestClass} from './SuperTestClass'; +import TestClass from './TestClass'; +jest.mock('./SuperTestClass'); +jest.mock('./TestClass'); + +describe('Testing the mocking of a class hierarchy defined in multiple imports', () => { + it('can call an instance method - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass.prototype, 'testMethod') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new SuperTestClass(); + expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can call a superclass instance method - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass.prototype, 'testMethod') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can read a value from an instance getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass.prototype, 'testAccessor', 'get') + .mockImplementation(() => { + return 'mockTestAccessor'; + }); + const testClassInstance = new SuperTestClass(); + expect(testClassInstance.testAccessor).toEqual('mockTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can read a value from a superclass instance getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass.prototype, 'testAccessor', 'get') + .mockImplementation(() => { + return 'mockTestAccessor'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.testAccessor).toEqual('mockTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can write a value to an instance setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass.prototype, 'testAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + const testClassInstance = new SuperTestClass(); + testClassInstance.testAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can write a value to a superclass instance setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass.prototype, 'testAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + const testClassInstance = new TestClass(); + testClassInstance.testAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can read a value from a static getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass, 'staticTestAccessor', 'get') + .mockImplementation(() => { + return 'mockStaticTestAccessor'; + }); + expect(SuperTestClass.staticTestAccessor).toEqual('mockStaticTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can read a value from a superclass static getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass, 'staticTestAccessor', 'get') + .mockImplementation(() => { + return 'mockStaticTestAccessor'; + }); + expect(TestClass.staticTestAccessor).toEqual('mockStaticTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can write a value to a static setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass, 'staticTestAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + SuperTestClass.staticTestAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can write a value to a superclass static setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass, 'staticTestAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + TestClass.staticTestAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts b/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts new file mode 100644 index 000000000000..2b326b971ea5 --- /dev/null +++ b/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts @@ -0,0 +1,128 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +import SuperTestClass, {TestClass} from './class-mocks-types'; +jest.mock('./class-mocks-types'); + +describe('Testing the mocking of a class hierarchy defined in a single import', () => { + it('can call an instance method - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass.prototype, 'testMethod') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new SuperTestClass(); + expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can call a superclass instance method - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass.prototype, 'testMethod') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can read a value from an instance getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass.prototype, 'testAccessor', 'get') + .mockImplementation(() => { + return 'mockTestAccessor'; + }); + const testClassInstance = new SuperTestClass(); + expect(testClassInstance.testAccessor).toEqual('mockTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can read a value from a superclass instance getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass.prototype, 'testAccessor', 'get') + .mockImplementation(() => { + return 'mockTestAccessor'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.testAccessor).toEqual('mockTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can write a value to an instance setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass.prototype, 'testAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + const testClassInstance = new SuperTestClass(); + testClassInstance.testAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can write a value to a superclass instance setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass.prototype, 'testAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + const testClassInstance = new TestClass(); + testClassInstance.testAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can read a value from a static getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass, 'staticTestAccessor', 'get') + .mockImplementation(() => { + return 'mockStaticTestAccessor'; + }); + expect(SuperTestClass.staticTestAccessor).toEqual('mockStaticTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can read a value from a superclass static getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass, 'staticTestAccessor', 'get') + .mockImplementation(() => { + return 'mockStaticTestAccessor'; + }); + expect(TestClass.staticTestAccessor).toEqual('mockStaticTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can write a value to a static setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass, 'staticTestAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + SuperTestClass.staticTestAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can write a value to a superclass static setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass, 'staticTestAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + TestClass.staticTestAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/jest-mock/src/__tests__/class-mocks-types.ts b/packages/jest-mock/src/__tests__/class-mocks-types.ts new file mode 100644 index 000000000000..4e277903940f --- /dev/null +++ b/packages/jest-mock/src/__tests__/class-mocks-types.ts @@ -0,0 +1,38 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +export default class SuperTestClass { + static staticTestProperty = 'staticTestProperty'; + + static get staticTestAccessor(): string { + return 'staticTestAccessor'; + } + + static set staticTestAccessor(_x: string) { + return; + } + + static staticTestMethod(): string { + return 'staticTestMethod'; + } + + testProperty = 'testProperty'; + + get testAccessor(): string { + return 'testAccessor'; + } + set testAccessor(_x: string) { + return; + } + + testMethod(): string { + return 'testMethod'; + } +} + +export class TestClass extends SuperTestClass {} diff --git a/packages/jest-mock/src/__tests__/class-mocks.test.ts b/packages/jest-mock/src/__tests__/class-mocks.test.ts new file mode 100644 index 000000000000..a0120958b54b --- /dev/null +++ b/packages/jest-mock/src/__tests__/class-mocks.test.ts @@ -0,0 +1,175 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + */ + +describe('Testing the mocking of a class', () => { + it('can call an instance method', () => { + class TestClass { + testMethod(): string { + return 'testMethod'; + } + } + + jest.spyOn(TestClass.prototype, 'testMethod').mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + }); + + it('can call a superclass instance method', () => { + class SuperTestClass { + testMethod(): string { + return 'testMethod'; + } + } + + class TestClass extends SuperTestClass {} + + jest.spyOn(TestClass.prototype, 'testMethod').mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + }); + + it('can read a value from an instance getter', () => { + class TestClass { + get testMethod(): string { + return 'testMethod'; + } + } + + jest + .spyOn(TestClass.prototype, 'testMethod', 'get') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.testMethod).toEqual('mockTestMethod'); + }); + + it('can read a value from an superclass instance getter', () => { + class SuperTestClass { + get testMethod(): string { + return 'testMethod'; + } + } + + class TestClass extends SuperTestClass {} + + jest + .spyOn(TestClass.prototype, 'testMethod', 'get') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.testMethod).toEqual('mockTestMethod'); + }); + + it('can write a value to an instance setter', () => { + class TestClass { + // eslint-disable-next-line accessor-pairs + set testMethod(_x: string) { + return; + } + } + + const mocktestMethod = jest + .spyOn(TestClass.prototype, 'testMethod', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + const testClassInstance = new TestClass(); + testClassInstance.testMethod = ''; + expect(mocktestMethod).toHaveBeenCalledTimes(1); + }); + + it('can write a value to a superclass instance setter', () => { + class SuperTestClass { + // eslint-disable-next-line accessor-pairs + set testMethod(_x: string) { + return; + } + } + + class TestClass extends SuperTestClass {} + + const mocktestMethod = jest + .spyOn(TestClass.prototype, 'testMethod', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + const testClassInstance = new TestClass(); + testClassInstance.testMethod = ''; + expect(mocktestMethod).toHaveBeenCalledTimes(1); + }); + + it('can read a value from a static getter', () => { + class TestClass { + static get testMethod(): string { + return 'testMethod'; + } + } + + jest.spyOn(TestClass, 'testMethod', 'get').mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(TestClass.testMethod).toEqual('mockTestMethod'); + }); + + it('can read a value from a superclass static getter', () => { + class SuperTestClass { + static get testMethod(): string { + return 'testMethod'; + } + } + + class TestClass extends SuperTestClass {} + + jest.spyOn(TestClass, 'testMethod', 'get').mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(TestClass.testMethod).toEqual('mockTestMethod'); + }); + + it('can write a value to a static setter', () => { + class TestClass { + // eslint-disable-next-line accessor-pairs + static set testMethod(_x: string) { + return; + } + } + + const mocktestMethod = jest + .spyOn(TestClass, 'testMethod', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + TestClass.testMethod = ''; + expect(mocktestMethod).toHaveBeenCalledTimes(1); + }); + + it('can write a value to a superclass static setter', () => { + class SuperTestClass { + // eslint-disable-next-line accessor-pairs + static set testMethod(_x: string) { + return; + } + } + + class TestClass extends SuperTestClass {} + + const mocktestMethod = jest + .spyOn(TestClass, 'testMethod', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + TestClass.testMethod = ''; + expect(mocktestMethod).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/jest-mock/src/__tests__/index.test.ts b/packages/jest-mock/src/__tests__/index.test.ts index fc965fa06e48..a085b9182b99 100644 --- a/packages/jest-mock/src/__tests__/index.test.ts +++ b/packages/jest-mock/src/__tests__/index.test.ts @@ -1325,7 +1325,7 @@ describe('moduleMocker', () => { }, }; - const spy = moduleMocker.spyOn(obj, 'method'); + const spy = moduleMocker.spyOn(obj, 'method', 'get'); const thisArg = {this: true}; const firstArg = {first: true}; diff --git a/packages/jest-mock/src/index.ts b/packages/jest-mock/src/index.ts index 157b47c76c67..446e282ada2f 100644 --- a/packages/jest-mock/src/index.ts +++ b/packages/jest-mock/src/index.ts @@ -516,7 +516,10 @@ export class ModuleMocker { if (!isReadonlyProp(object, prop)) { const propDesc = Object.getOwnPropertyDescriptor(object, prop); - if ((propDesc !== undefined && !propDesc.get) || object.__esModule) { + if ( + propDesc !== undefined && + !(propDesc.get && prop == '__proto__') + ) { slots.add(prop); } } @@ -906,7 +909,9 @@ export class ModuleMocker { } this._getSlots(metadata.members).forEach(slot => { + let slotMock: Mocked; const slotMetadata = (metadata.members && metadata.members[slot]) || {}; + if (slotMetadata.ref != null) { callbacks.push( (function (ref) { @@ -914,7 +919,37 @@ export class ModuleMocker { })(slotMetadata.ref), ); } else { - mock[slot] = this._generateMock(slotMetadata, callbacks, refs); + slotMock = this._generateMock(slotMetadata, callbacks, refs); + + // For superclass accessor properties the subclass metadata contains the definitions + // for the getter and setter methods, and the superclass refs to them. + // The mock implementations are not available until the callbacks have been executed. + // Missing getter and setter refs will be resolved as their callbacks have been + // stacked before the setting of the accessor definition is stacked. + if ( + slotMetadata.members?.get?.ref !== undefined || + slotMetadata.members?.set?.ref !== undefined + ) { + callbacks.push( + (function (ref) { + return () => Object.defineProperty(mock, slot, ref); + })(slotMock as PropertyDescriptor), + ); + } else if ( + (slotMetadata.members?.get || slotMetadata.members?.set) && + slotMetadata.members?.configurable && + slotMetadata.members?.enumerable + ) { + // In some cases, e.g. third-party APIs, a 'prototype' ancestor to be + // mocked has a function property called 'get'. In this circumstance + // the 'prototype' property cannot be redefined and doing so causes an + // exception. Checks have been added for the 'configurable' and + // 'enumberable' properties present on true accessor property + // descriptors to prevent the attempt to replace the API. + Object.defineProperty(mock, slot, slotMock as PropertyDescriptor); + } else { + mock[slot] = slotMock; + } } }); @@ -998,8 +1033,33 @@ export class ModuleMocker { ) { return; } - // @ts-expect-error no index signature - const slotMetadata = this.getMetadata(component[slot], refs); + + let descriptor = Object.getOwnPropertyDescriptor(component, slot); + let proto = Object.getPrototypeOf(component); + while (!descriptor && proto !== null) { + descriptor = Object.getOwnPropertyDescriptor(proto, slot); + proto = Object.getPrototypeOf(proto); + } + + let slotMetadata: MockMetadata | null = null; + if (descriptor?.get || descriptor?.set) { + // Specific case required for mocking class definitions imported via modules. + // In this case the class definitions are stored in accessor properties. + // All getters were previously ignored except where the containing object had __esModule == true + // Now getters are mocked the class definitions must still be read. + // @ts-expect-error ignore type mismatch + if (component.__esModule) { + // @ts-expect-error no index signature + slotMetadata = this.getMetadata(component[slot], refs); + } else { + // @ts-expect-error ignore type mismatch + slotMetadata = this.getMetadata(descriptor, refs); + } + } else { + // @ts-expect-error no index signature + slotMetadata = this.getMetadata(component[slot], refs); + } + if (slotMetadata) { if (!members) { members = {}; @@ -1080,146 +1140,130 @@ export class ModuleMocker { methodKey: K, accessType?: 'get' | 'set', ) { - if (accessType) { - return this._spyOnProperty(object, methodKey, accessType); - } - - if (typeof object !== 'object' && typeof object !== 'function') { + if (!object) { throw new Error( - `Cannot spyOn on a primitive value; ${this._typeOf(object)} given`, + `spyOn could not find an object to spy upon for ${String(methodKey)}`, ); } - const original = object[methodKey]; - - if (!this.isMockFunction(original)) { - if (typeof original !== 'function') { - throw new Error( - `Cannot spy the ${String( - methodKey, - )} property because it is not a function; ${this._typeOf( - original, - )} given instead`, - ); - } - - const isMethodOwner = Object.prototype.hasOwnProperty.call( - object, - methodKey, - ); - - let descriptor = Object.getOwnPropertyDescriptor(object, methodKey); - let proto = Object.getPrototypeOf(object); - - while (!descriptor && proto !== null) { - descriptor = Object.getOwnPropertyDescriptor(proto, methodKey); - proto = Object.getPrototypeOf(proto); - } - - let mock: Mock; - - if (descriptor && descriptor.get) { - const originalGet = descriptor.get; - 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'}, () => { - 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); - }); + if (!methodKey) { + throw new Error('No property name supplied'); } - return object[methodKey]; - } - - private _spyOnProperty>( - obj: T, - propertyKey: K, - accessType: 'get' | 'set', - ): Mock<() => T> { - if (typeof obj !== 'object' && typeof obj !== 'function') { - throw new Error( - `Cannot spyOn on a primitive value; ${this._typeOf(obj)} given`, - ); + if (accessType && accessType != 'get' && accessType != 'set') { + throw new Error('Invalid accessType supplied'); } - if (!obj) { + if (typeof object !== 'object' && typeof object !== 'function') { throw new Error( - `spyOn could not find an object to spy upon for ${String(propertyKey)}`, + `Cannot spyOn on a primitive value; ${this._typeOf(object)} given`, ); } - if (!propertyKey) { - throw new Error('No property name supplied'); - } - - let descriptor = Object.getOwnPropertyDescriptor(obj, propertyKey); - let proto = Object.getPrototypeOf(obj); - + let descriptor = Object.getOwnPropertyDescriptor(object, methodKey); + let proto = Object.getPrototypeOf(object); while (!descriptor && proto !== null) { - descriptor = Object.getOwnPropertyDescriptor(proto, propertyKey); + descriptor = Object.getOwnPropertyDescriptor(proto, methodKey); proto = Object.getPrototypeOf(proto); } - if (!descriptor) { - throw new Error(`${String(propertyKey)} property does not exist`); + throw new Error(`${String(methodKey)} property does not exist`); } - if (!descriptor.configurable) { - throw new Error(`${String(propertyKey)} is not declared configurable`); + throw new Error(`${String(methodKey)} is not declared configurable`); } - if (!descriptor[accessType]) { - throw new Error( - `Property ${String( - propertyKey, - )} does not have access type ${accessType}`, - ); + if (this.isMockFunction(descriptor.value)) { + return object[methodKey]; + } else if (accessType == 'get' && this.isMockFunction(descriptor.get)) { + return descriptor.get; + } else if (accessType == 'set' && this.isMockFunction(descriptor.set)) { + return descriptor.set; } - const original = descriptor[accessType]; - - if (!this.isMockFunction(original)) { - if (typeof original !== 'function') { + if (accessType) { + if (typeof descriptor[accessType] !== 'function') { throw new Error( - `Cannot spy the ${String( - propertyKey, - )} property because it is not a function; ${this._typeOf( - original, - )} given instead`, + `Cannot spy the ${String(accessType)} ${String( + methodKey, + )} property because it is not a function; + ${this._typeOf(descriptor?.[accessType])} given instead`, ); } + } else if (typeof descriptor.value !== 'function') { + throw new Error( + `Cannot spy the ${String( + methodKey, + )} property because it is not a function; ${this._typeOf( + descriptor.value, + )} given instead`, + ); + } + + let mock: Mock; - descriptor[accessType] = this._makeComponent({type: 'function'}, () => { - // @ts-expect-error: mock is assignable - descriptor![accessType] = original; - Object.defineProperty(obj, propertyKey, descriptor!); + if (accessType == 'get' && descriptor['get']) { + const originalAccessor = descriptor['get']; + mock = this._makeComponent( + { + type: 'function', + }, + () => { + descriptor![accessType] = originalAccessor; + Object.defineProperty(object, methodKey, descriptor!); + }, + ); + + descriptor[accessType] = mock; + mock.mockImplementation(function (this: unknown) { + return originalAccessor.call(this); }); + Object.defineProperty(object, methodKey, descriptor); + } else if (accessType == 'set' && descriptor['set']) { + const originalAccessor = descriptor['set']; + mock = this._makeComponent( + { + type: 'function', + }, + () => { + descriptor![accessType] = originalAccessor; + Object.defineProperty(object, methodKey, descriptor!); + }, + ); - (descriptor[accessType] as Mock<() => T>).mockImplementation(function ( - this: unknown, - ) { - // @ts-expect-error - wrong context - return original.apply(this, arguments); + descriptor[accessType] = mock; + mock.mockImplementation(function (this: unknown) { + return originalAccessor.call(this, arguments[0]); + }); + Object.defineProperty(object, methodKey, descriptor); + } else { + const isMethodOwner = Object.prototype.hasOwnProperty.call( + object, + methodKey, + ); + const original = descriptor; + + mock = this._makeComponent( + { + type: 'function', + }, + () => { + if (isMethodOwner) { + object[methodKey] = original.value; + } else { + delete object[methodKey]; + } + }, + ); + + // @ts-expect-error overriding original method with a Mock + object[methodKey] = mock; + mock.mockImplementation(function (this: unknown) { + return original.value.apply(this, arguments); }); } - Object.defineProperty(obj, propertyKey, descriptor); - return descriptor[accessType] as Mock<() => T>; + return mock; } clearAllMocks(): void { From 46c92d6b1dcfdeb5fe1119891569fb0f5ff6e37f Mon Sep 17 00:00:00 2001 From: Peter Staples Date: Fri, 7 Oct 2022 00:43:19 +0100 Subject: [PATCH 02/13] bug 13140 - Added condition to prevent refs to auto-mocked superclass 'get'/'set' functions being mocked as accessor objects. Added extra tests for this, and for some other conditions. Corrected expected test error messages due to the refactored code for the fix to this bug. --- .../class-mocks-single-import.test.ts | 134 ++++++++++++++- .../src/__tests__/class-mocks-types.ts | 16 ++ .../src/__tests__/class-mocks.test.ts | 156 +++++++++++++++++- .../jest-mock/src/__tests__/index.test.ts | 12 +- packages/jest-mock/src/index.ts | 19 ++- 5 files changed, 309 insertions(+), 28 deletions(-) diff --git a/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts b/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts index 2b326b971ea5..bd1e33a7c27a 100644 --- a/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts +++ b/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts @@ -17,7 +17,7 @@ describe('Testing the mocking of a class hierarchy defined in a single import', return 'mockTestMethod'; }); const testClassInstance = new SuperTestClass(); - expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + expect(testClassInstance.testMethod()).toBe('mockTestMethod'); expect(mockTestMethod).toHaveBeenCalledTimes(1); mockTestMethod.mockClear(); @@ -30,10 +30,62 @@ describe('Testing the mocking of a class hierarchy defined in a single import', return 'mockTestMethod'; }); const testClassInstance = new TestClass(); - expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + expect(testClassInstance.testMethod()).toBe('mockTestMethod'); expect(mockTestMethod).toHaveBeenCalledTimes(1); }); + it('can call an instance method named "get" - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass.prototype, 'get') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new SuperTestClass(); + expect(testClassInstance.get()).toBe('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can call a superclass instance method named "get" - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass.prototype, 'get') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.get()).toBe('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can call an instance method named "set" - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass.prototype, 'set') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new SuperTestClass(); + expect(testClassInstance.set()).toBe('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can call a superclass instance method named "set" - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass.prototype, 'set') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.set()).toBe('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + it('can read a value from an instance getter - Auto-mocked class', () => { const mockTestMethod = jest .spyOn(SuperTestClass.prototype, 'testAccessor', 'get') @@ -41,7 +93,7 @@ describe('Testing the mocking of a class hierarchy defined in a single import', return 'mockTestAccessor'; }); const testClassInstance = new SuperTestClass(); - expect(testClassInstance.testAccessor).toEqual('mockTestAccessor'); + expect(testClassInstance.testAccessor).toBe('mockTestAccessor'); expect(mockTestMethod).toHaveBeenCalledTimes(1); mockTestMethod.mockClear(); @@ -54,7 +106,7 @@ describe('Testing the mocking of a class hierarchy defined in a single import', return 'mockTestAccessor'; }); const testClassInstance = new TestClass(); - expect(testClassInstance.testAccessor).toEqual('mockTestAccessor'); + expect(testClassInstance.testAccessor).toBe('mockTestAccessor'); expect(mockTestMethod).toHaveBeenCalledTimes(1); }); @@ -82,13 +134,83 @@ describe('Testing the mocking of a class hierarchy defined in a single import', expect(mockTestMethod).toHaveBeenCalledTimes(1); }); + it('can call a static method - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass, 'staticTestMethod') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(SuperTestClass.staticTestMethod()).toBe('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can call a superclass static method - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass, 'staticTestMethod') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(TestClass.staticTestMethod()).toBe('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can call a static method named "get" - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass, 'get') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(SuperTestClass.get()).toBe('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can call a superclass static method named "get" - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass, 'get') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(TestClass.get()).toBe('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can call a static method named "set" - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass, 'set') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(SuperTestClass.set()).toBe('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can call a superclass static method named "set" - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass, 'set') + .mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(TestClass.set()).toBe('mockTestMethod'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + it('can read a value from a static getter - Auto-mocked class', () => { const mockTestMethod = jest .spyOn(SuperTestClass, 'staticTestAccessor', 'get') .mockImplementation(() => { return 'mockStaticTestAccessor'; }); - expect(SuperTestClass.staticTestAccessor).toEqual('mockStaticTestAccessor'); + expect(SuperTestClass.staticTestAccessor).toBe('mockStaticTestAccessor'); expect(mockTestMethod).toHaveBeenCalledTimes(1); mockTestMethod.mockClear(); @@ -100,7 +222,7 @@ describe('Testing the mocking of a class hierarchy defined in a single import', .mockImplementation(() => { return 'mockStaticTestAccessor'; }); - expect(TestClass.staticTestAccessor).toEqual('mockStaticTestAccessor'); + expect(TestClass.staticTestAccessor).toBe('mockStaticTestAccessor'); expect(mockTestMethod).toHaveBeenCalledTimes(1); }); diff --git a/packages/jest-mock/src/__tests__/class-mocks-types.ts b/packages/jest-mock/src/__tests__/class-mocks-types.ts index 4e277903940f..ccb2eae1fc4f 100644 --- a/packages/jest-mock/src/__tests__/class-mocks-types.ts +++ b/packages/jest-mock/src/__tests__/class-mocks-types.ts @@ -21,6 +21,14 @@ export default class SuperTestClass { return 'staticTestMethod'; } + static get(): string { + return 'get'; + } + + static set(): string { + return 'set'; + } + testProperty = 'testProperty'; get testAccessor(): string { @@ -33,6 +41,14 @@ export default class SuperTestClass { testMethod(): string { return 'testMethod'; } + + get(): string { + return 'get'; + } + + set(): string { + return 'set'; + } } export class TestClass extends SuperTestClass {} diff --git a/packages/jest-mock/src/__tests__/class-mocks.test.ts b/packages/jest-mock/src/__tests__/class-mocks.test.ts index a0120958b54b..4b33c6c532dd 100644 --- a/packages/jest-mock/src/__tests__/class-mocks.test.ts +++ b/packages/jest-mock/src/__tests__/class-mocks.test.ts @@ -18,7 +18,7 @@ describe('Testing the mocking of a class', () => { return 'mockTestMethod'; }); const testClassInstance = new TestClass(); - expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + expect(testClassInstance.testMethod()).toBe('mockTestMethod'); }); it('can call a superclass instance method', () => { @@ -34,7 +34,67 @@ describe('Testing the mocking of a class', () => { return 'mockTestMethod'; }); const testClassInstance = new TestClass(); - expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + expect(testClassInstance.testMethod()).toBe('mockTestMethod'); + }); + + it('can call an instance method named "get"', () => { + class TestClass { + get(): string { + return 'get'; + } + } + + jest.spyOn(TestClass.prototype, 'get').mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.get()).toBe('mockTestMethod'); + }); + + it('can call a superclass instance method named "get"', () => { + class SuperTestClass { + get(): string { + return 'get'; + } + } + + class TestClass extends SuperTestClass {} + + jest.spyOn(TestClass.prototype, 'get').mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.get()).toBe('mockTestMethod'); + }); + + it('can call an instance method named "set"', () => { + class TestClass { + set(): string { + return 'set'; + } + } + + jest.spyOn(TestClass.prototype, 'set').mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.set()).toBe('mockTestMethod'); + }); + + it('can call a superclass instance method named "set"', () => { + class SuperTestClass { + set(): string { + return 'set'; + } + } + + class TestClass extends SuperTestClass {} + + jest.spyOn(TestClass.prototype, 'set').mockImplementation(() => { + return 'mockTestMethod'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.set()).toBe('mockTestMethod'); }); it('can read a value from an instance getter', () => { @@ -50,7 +110,7 @@ describe('Testing the mocking of a class', () => { return 'mockTestMethod'; }); const testClassInstance = new TestClass(); - expect(testClassInstance.testMethod).toEqual('mockTestMethod'); + expect(testClassInstance.testMethod).toBe('mockTestMethod'); }); it('can read a value from an superclass instance getter', () => { @@ -68,7 +128,7 @@ describe('Testing the mocking of a class', () => { return 'mockTestMethod'; }); const testClassInstance = new TestClass(); - expect(testClassInstance.testMethod).toEqual('mockTestMethod'); + expect(testClassInstance.testMethod).toBe('mockTestMethod'); }); it('can write a value to an instance setter', () => { @@ -109,6 +169,90 @@ describe('Testing the mocking of a class', () => { expect(mocktestMethod).toHaveBeenCalledTimes(1); }); + it('can call a static method', () => { + class TestClass { + static testMethod(): string { + return 'testMethod'; + } + } + + jest.spyOn(TestClass, 'testMethod').mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(TestClass.testMethod()).toBe('mockTestMethod'); + }); + + it('can call a superclass static method', () => { + class SuperTestClass { + static testMethod(): string { + return 'testMethod'; + } + } + + class TestClass extends SuperTestClass {} + + jest.spyOn(TestClass, 'testMethod').mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(TestClass.testMethod()).toBe('mockTestMethod'); + }); + + it('can call a static method named "get"', () => { + class TestClass { + static get(): string { + return 'get'; + } + } + + jest.spyOn(TestClass, 'get').mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(TestClass.get()).toBe('mockTestMethod'); + }); + + it('can call a superclass static method named "get"', () => { + class SuperTestClass { + static get(): string { + return 'get'; + } + } + + class TestClass extends SuperTestClass {} + + jest.spyOn(TestClass, 'get').mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(TestClass.get()).toBe('mockTestMethod'); + }); + + it('can call a static method named "set"', () => { + class TestClass { + static set(): string { + return 'set'; + } + } + + jest.spyOn(TestClass, 'set').mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(TestClass.set()).toBe('mockTestMethod'); + }); + + it('can call a superclass static method named "set"', () => { + class SuperTestClass { + static set(): string { + return 'set'; + } + } + + class TestClass extends SuperTestClass {} + + jest.spyOn(TestClass, 'set').mockImplementation(() => { + return 'mockTestMethod'; + }); + expect(TestClass.set()).toBe('mockTestMethod'); + }); + it('can read a value from a static getter', () => { class TestClass { static get testMethod(): string { @@ -119,7 +263,7 @@ describe('Testing the mocking of a class', () => { jest.spyOn(TestClass, 'testMethod', 'get').mockImplementation(() => { return 'mockTestMethod'; }); - expect(TestClass.testMethod).toEqual('mockTestMethod'); + expect(TestClass.testMethod).toBe('mockTestMethod'); }); it('can read a value from a superclass static getter', () => { @@ -134,7 +278,7 @@ describe('Testing the mocking of a class', () => { jest.spyOn(TestClass, 'testMethod', 'get').mockImplementation(() => { return 'mockTestMethod'; }); - expect(TestClass.testMethod).toEqual('mockTestMethod'); + expect(TestClass.testMethod).toBe('mockTestMethod'); }); it('can write a value to a static setter', () => { diff --git a/packages/jest-mock/src/__tests__/index.test.ts b/packages/jest-mock/src/__tests__/index.test.ts index a085b9182b99..8f1436f4b042 100644 --- a/packages/jest-mock/src/__tests__/index.test.ts +++ b/packages/jest-mock/src/__tests__/index.test.ts @@ -1261,12 +1261,10 @@ describe('moduleMocker', () => { it('should throw on invalid input', () => { expect(() => { moduleMocker.spyOn(null, 'method'); - }).toThrow('Cannot read prop'); + }).toThrow('spyOn could not find an object to spy upon for method'); expect(() => { moduleMocker.spyOn({}, 'method'); - }).toThrow( - 'Cannot spy the method property because it is not a function; undefined given instead', - ); + }).toThrow('method property does not exist'); expect(() => { moduleMocker.spyOn({method: 10}, 'method'); }).toThrow( @@ -1431,12 +1429,10 @@ describe('moduleMocker', () => { it('should throw on invalid input', () => { expect(() => { moduleMocker.spyOn(null, 'method'); - }).toThrow('Cannot read prop'); + }).toThrow('spyOn could not find an object to spy upon for method'); expect(() => { moduleMocker.spyOn({}, 'method'); - }).toThrow( - 'Cannot spy the method property because it is not a function; undefined given instead', - ); + }).toThrow('method property does not exist'); expect(() => { moduleMocker.spyOn({method: 10}, 'method'); }).toThrow( diff --git a/packages/jest-mock/src/index.ts b/packages/jest-mock/src/index.ts index 446e282ada2f..408690efaaab 100644 --- a/packages/jest-mock/src/index.ts +++ b/packages/jest-mock/src/index.ts @@ -926,9 +926,18 @@ export class ModuleMocker { // The mock implementations are not available until the callbacks have been executed. // Missing getter and setter refs will be resolved as their callbacks have been // stacked before the setting of the accessor definition is stacked. + + // In some cases, e.g. third-party APIs, a 'prototype' ancestor to be + // mocked has a function property called 'get'. In this circumstance + // the 'prototype' property cannot be redefined and doing so causes an + // exception. Checks have been added for the 'configurable' and + // 'enumberable' properties present on true accessor property + // descriptors to prevent the attempt to replace the API. if ( - slotMetadata.members?.get?.ref !== undefined || - slotMetadata.members?.set?.ref !== undefined + (slotMetadata.members?.get?.ref !== undefined || + slotMetadata.members?.set?.ref !== undefined) && + slotMetadata.members?.configurable && + slotMetadata.members?.enumerable ) { callbacks.push( (function (ref) { @@ -940,12 +949,6 @@ export class ModuleMocker { slotMetadata.members?.configurable && slotMetadata.members?.enumerable ) { - // In some cases, e.g. third-party APIs, a 'prototype' ancestor to be - // mocked has a function property called 'get'. In this circumstance - // the 'prototype' property cannot be redefined and doing so causes an - // exception. Checks have been added for the 'configurable' and - // 'enumberable' properties present on true accessor property - // descriptors to prevent the attempt to replace the API. Object.defineProperty(mock, slot, slotMock as PropertyDescriptor); } else { mock[slot] = slotMock; From d315f785c8d017fd88d6cc4a8ef9d0e0c4442190 Mon Sep 17 00:00:00 2001 From: Peter Staples Date: Fri, 7 Oct 2022 01:42:50 +0100 Subject: [PATCH 03/13] bug 13140 - Lint changes --- .../src/__tests__/class-mocks-dual-import.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts b/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts index 510484d06700..39396be444d4 100644 --- a/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts +++ b/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts @@ -19,7 +19,7 @@ describe('Testing the mocking of a class hierarchy defined in multiple imports', return 'mockTestMethod'; }); const testClassInstance = new SuperTestClass(); - expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + expect(testClassInstance.testMethod()).toBe('mockTestMethod'); expect(mockTestMethod).toHaveBeenCalledTimes(1); mockTestMethod.mockClear(); @@ -32,7 +32,7 @@ describe('Testing the mocking of a class hierarchy defined in multiple imports', return 'mockTestMethod'; }); const testClassInstance = new TestClass(); - expect(testClassInstance.testMethod()).toEqual('mockTestMethod'); + expect(testClassInstance.testMethod()).toBe('mockTestMethod'); expect(mockTestMethod).toHaveBeenCalledTimes(1); }); @@ -43,7 +43,7 @@ describe('Testing the mocking of a class hierarchy defined in multiple imports', return 'mockTestAccessor'; }); const testClassInstance = new SuperTestClass(); - expect(testClassInstance.testAccessor).toEqual('mockTestAccessor'); + expect(testClassInstance.testAccessor).toBe('mockTestAccessor'); expect(mockTestMethod).toHaveBeenCalledTimes(1); mockTestMethod.mockClear(); @@ -56,7 +56,7 @@ describe('Testing the mocking of a class hierarchy defined in multiple imports', return 'mockTestAccessor'; }); const testClassInstance = new TestClass(); - expect(testClassInstance.testAccessor).toEqual('mockTestAccessor'); + expect(testClassInstance.testAccessor).toBe('mockTestAccessor'); expect(mockTestMethod).toHaveBeenCalledTimes(1); }); @@ -90,7 +90,7 @@ describe('Testing the mocking of a class hierarchy defined in multiple imports', .mockImplementation(() => { return 'mockStaticTestAccessor'; }); - expect(SuperTestClass.staticTestAccessor).toEqual('mockStaticTestAccessor'); + expect(SuperTestClass.staticTestAccessor).toBe('mockStaticTestAccessor'); expect(mockTestMethod).toHaveBeenCalledTimes(1); mockTestMethod.mockClear(); @@ -102,7 +102,7 @@ describe('Testing the mocking of a class hierarchy defined in multiple imports', .mockImplementation(() => { return 'mockStaticTestAccessor'; }); - expect(TestClass.staticTestAccessor).toEqual('mockStaticTestAccessor'); + expect(TestClass.staticTestAccessor).toBe('mockStaticTestAccessor'); expect(mockTestMethod).toHaveBeenCalledTimes(1); }); From 6c3af0d0d1aab6b9adc615ff99266351918664d1 Mon Sep 17 00:00:00 2001 From: Peter Staples Date: Fri, 7 Oct 2022 10:04:48 +0100 Subject: [PATCH 04/13] bug 13140 - requested repo reorg --- .../src/__tests__/{ => __fixtures__}/SuperTestClass.ts | 0 .../src/__tests__/{ => __fixtures__}/TestClass.ts | 0 .../src/__tests__/{ => __fixtures__}/class-mocks-types.ts | 0 .../src/__tests__/class-mocks-dual-import.test.ts | 8 ++++---- .../src/__tests__/class-mocks-single-import.test.ts | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) rename packages/jest-mock/src/__tests__/{ => __fixtures__}/SuperTestClass.ts (100%) rename packages/jest-mock/src/__tests__/{ => __fixtures__}/TestClass.ts (100%) rename packages/jest-mock/src/__tests__/{ => __fixtures__}/class-mocks-types.ts (100%) diff --git a/packages/jest-mock/src/__tests__/SuperTestClass.ts b/packages/jest-mock/src/__tests__/__fixtures__/SuperTestClass.ts similarity index 100% rename from packages/jest-mock/src/__tests__/SuperTestClass.ts rename to packages/jest-mock/src/__tests__/__fixtures__/SuperTestClass.ts diff --git a/packages/jest-mock/src/__tests__/TestClass.ts b/packages/jest-mock/src/__tests__/__fixtures__/TestClass.ts similarity index 100% rename from packages/jest-mock/src/__tests__/TestClass.ts rename to packages/jest-mock/src/__tests__/__fixtures__/TestClass.ts diff --git a/packages/jest-mock/src/__tests__/class-mocks-types.ts b/packages/jest-mock/src/__tests__/__fixtures__/class-mocks-types.ts similarity index 100% rename from packages/jest-mock/src/__tests__/class-mocks-types.ts rename to packages/jest-mock/src/__tests__/__fixtures__/class-mocks-types.ts diff --git a/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts b/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts index 39396be444d4..610469e9deba 100644 --- a/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts +++ b/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts @@ -6,10 +6,10 @@ * */ -import {SuperTestClass} from './SuperTestClass'; -import TestClass from './TestClass'; -jest.mock('./SuperTestClass'); -jest.mock('./TestClass'); +import {SuperTestClass} from './__fixtures__/SuperTestClass'; +import TestClass from './__fixtures__/TestClass'; +jest.mock('./__fixtures__/SuperTestClass'); +jest.mock('./__fixtures__/TestClass'); describe('Testing the mocking of a class hierarchy defined in multiple imports', () => { it('can call an instance method - Auto-mocked class', () => { diff --git a/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts b/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts index bd1e33a7c27a..9b354db44d95 100644 --- a/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts +++ b/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts @@ -6,8 +6,8 @@ * */ -import SuperTestClass, {TestClass} from './class-mocks-types'; -jest.mock('./class-mocks-types'); +import SuperTestClass, {TestClass} from './__fixtures__/class-mocks-types'; +jest.mock('./__fixtures__/class-mocks-types'); describe('Testing the mocking of a class hierarchy defined in a single import', () => { it('can call an instance method - Auto-mocked class', () => { From c36a19653df94199d5c704de7b4b549029bfbbed Mon Sep 17 00:00:00 2001 From: staplespeter Date: Fri, 7 Oct 2022 10:25:20 +0100 Subject: [PATCH 05/13] Update jest.config.mjs Co-authored-by: Tom Mrazauskas --- jest.config.mjs | 3 --- 1 file changed, 3 deletions(-) diff --git a/jest.config.mjs b/jest.config.mjs index 122977451782..59f81009fb15 100644 --- a/jest.config.mjs +++ b/jest.config.mjs @@ -57,9 +57,6 @@ export default { '/packages/jest-haste-map/src/__tests__/haste_impl.js', '/packages/jest-haste-map/src/__tests__/dependencyExtractor.js', '/packages/jest-haste-map/src/__tests__/test_dotfiles_root/', - '/packages/jest-mock/src/__tests__/class-mocks-types.ts', - '/packages/jest-mock/src/__tests__/TestClass.ts', - '/packages/jest-mock/src/__tests__/SuperTestClass.ts', '/packages/jest-repl/src/__tests__/test_root', '/packages/jest-runtime/src/__tests__/defaultResolver.js', '/packages/jest-runtime/src/__tests__/module_dir/', From e7e4242a13ffe0ea993b504aa1d734445e7b1145 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Fri, 7 Oct 2022 18:19:00 +0200 Subject: [PATCH 06/13] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30916146f9bb..082a3e166250 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Fixes - `[babel-plugin-jest-hoist]` Ignore `TSTypeQuery` when checking for hoisted references ([#13367](https://github.com/facebook/jest/pull/13367)) +- `[jest-mock]` Fix mocking of getters and setters on classes ([#13398](https://github.com/facebook/jest/pull/13398)) - `[@jest/types]` Infer type of `each` table correctly when the table is a tuple or array ([#13381](https://github.com/facebook/jest/pull/13381)) - `[@jest/types]` Rework typings to allow the `*ReturnedWith` matchers to be called with no argument ([#13385](https://github.com/facebook/jest/pull/13385)) From e7f1a1c79fbe60e9d50f0575d7d8dd1f5f215b74 Mon Sep 17 00:00:00 2001 From: Peter Staples Date: Sat, 15 Oct 2022 12:56:21 +0100 Subject: [PATCH 07/13] bug-13140 - added tests for function exports mocked using spyOn. These currently only test for functions exported as data properties, not as accessors/getters --- .../__fixtures__/class-mocks-types.ts | 13 ++++ .../class-mocks-single-import.test.ts | 67 +++++++++++++------ 2 files changed, 59 insertions(+), 21 deletions(-) diff --git a/packages/jest-mock/src/__tests__/__fixtures__/class-mocks-types.ts b/packages/jest-mock/src/__tests__/__fixtures__/class-mocks-types.ts index ccb2eae1fc4f..a6f8aa5634d8 100644 --- a/packages/jest-mock/src/__tests__/__fixtures__/class-mocks-types.ts +++ b/packages/jest-mock/src/__tests__/__fixtures__/class-mocks-types.ts @@ -52,3 +52,16 @@ export default class SuperTestClass { } export class TestClass extends SuperTestClass {} + +export function testFunction1() { + return 'testFunction1'; +} + +function testFunction() { + return 'testFunction2'; +} +export const testFunction2 = testFunction; + +export const testFunction3 = () => { + return 'testFunction3'; +}; diff --git a/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts b/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts index 9b354db44d95..1d587ef2b156 100644 --- a/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts +++ b/packages/jest-mock/src/__tests__/class-mocks-single-import.test.ts @@ -6,9 +6,32 @@ * */ -import SuperTestClass, {TestClass} from './__fixtures__/class-mocks-types'; +import SuperTestClass, * as testTypes from './__fixtures__/class-mocks-types'; jest.mock('./__fixtures__/class-mocks-types'); +describe('Testing the mocking of exported functions', () => { + it('can mock a directly exported function', () => { + jest.spyOn(testTypes, 'testFunction1').mockImplementation(() => { + return 'mockTestFunction'; + }); + expect(testTypes.testFunction1()).toBe('mockTestFunction'); + }); + + it('can mock an indirectly exported function', () => { + jest.spyOn(testTypes, 'testFunction2').mockImplementation(() => { + return 'mockTestFunction'; + }); + expect(testTypes.testFunction2()).toBe('mockTestFunction'); + }); + + it('can mock an indirectly exported anonymous function', () => { + jest.spyOn(testTypes, 'testFunction3').mockImplementation(() => { + return 'mockTestFunction'; + }); + expect(testTypes.testFunction3()).toBe('mockTestFunction'); + }); +}); + describe('Testing the mocking of a class hierarchy defined in a single import', () => { it('can call an instance method - Auto-mocked class', () => { const mockTestMethod = jest @@ -25,11 +48,11 @@ describe('Testing the mocking of a class hierarchy defined in a single import', it('can call a superclass instance method - Auto-mocked class', () => { const mockTestMethod = jest - .spyOn(TestClass.prototype, 'testMethod') + .spyOn(testTypes.TestClass.prototype, 'testMethod') .mockImplementation(() => { return 'mockTestMethod'; }); - const testClassInstance = new TestClass(); + const testClassInstance = new testTypes.TestClass(); expect(testClassInstance.testMethod()).toBe('mockTestMethod'); expect(mockTestMethod).toHaveBeenCalledTimes(1); }); @@ -49,11 +72,11 @@ describe('Testing the mocking of a class hierarchy defined in a single import', it('can call a superclass instance method named "get" - Auto-mocked class', () => { const mockTestMethod = jest - .spyOn(TestClass.prototype, 'get') + .spyOn(testTypes.TestClass.prototype, 'get') .mockImplementation(() => { return 'mockTestMethod'; }); - const testClassInstance = new TestClass(); + const testClassInstance = new testTypes.TestClass(); expect(testClassInstance.get()).toBe('mockTestMethod'); expect(mockTestMethod).toHaveBeenCalledTimes(1); @@ -75,11 +98,11 @@ describe('Testing the mocking of a class hierarchy defined in a single import', it('can call a superclass instance method named "set" - Auto-mocked class', () => { const mockTestMethod = jest - .spyOn(TestClass.prototype, 'set') + .spyOn(testTypes.TestClass.prototype, 'set') .mockImplementation(() => { return 'mockTestMethod'; }); - const testClassInstance = new TestClass(); + const testClassInstance = new testTypes.TestClass(); expect(testClassInstance.set()).toBe('mockTestMethod'); expect(mockTestMethod).toHaveBeenCalledTimes(1); @@ -101,11 +124,11 @@ describe('Testing the mocking of a class hierarchy defined in a single import', it('can read a value from a superclass instance getter - Auto-mocked class', () => { const mockTestMethod = jest - .spyOn(TestClass.prototype, 'testAccessor', 'get') + .spyOn(testTypes.TestClass.prototype, 'testAccessor', 'get') .mockImplementation(() => { return 'mockTestAccessor'; }); - const testClassInstance = new TestClass(); + const testClassInstance = new testTypes.TestClass(); expect(testClassInstance.testAccessor).toBe('mockTestAccessor'); expect(mockTestMethod).toHaveBeenCalledTimes(1); }); @@ -125,11 +148,11 @@ describe('Testing the mocking of a class hierarchy defined in a single import', it('can write a value to a superclass instance setter - Auto-mocked class', () => { const mockTestMethod = jest - .spyOn(TestClass.prototype, 'testAccessor', 'set') + .spyOn(testTypes.TestClass.prototype, 'testAccessor', 'set') .mockImplementation((_x: string) => { return () => {}; }); - const testClassInstance = new TestClass(); + const testClassInstance = new testTypes.TestClass(); testClassInstance.testAccessor = ''; expect(mockTestMethod).toHaveBeenCalledTimes(1); }); @@ -148,11 +171,11 @@ describe('Testing the mocking of a class hierarchy defined in a single import', it('can call a superclass static method - Auto-mocked class', () => { const mockTestMethod = jest - .spyOn(TestClass, 'staticTestMethod') + .spyOn(testTypes.TestClass, 'staticTestMethod') .mockImplementation(() => { return 'mockTestMethod'; }); - expect(TestClass.staticTestMethod()).toBe('mockTestMethod'); + expect(testTypes.TestClass.staticTestMethod()).toBe('mockTestMethod'); expect(mockTestMethod).toHaveBeenCalledTimes(1); }); @@ -170,11 +193,11 @@ describe('Testing the mocking of a class hierarchy defined in a single import', it('can call a superclass static method named "get" - Auto-mocked class', () => { const mockTestMethod = jest - .spyOn(TestClass, 'get') + .spyOn(testTypes.TestClass, 'get') .mockImplementation(() => { return 'mockTestMethod'; }); - expect(TestClass.get()).toBe('mockTestMethod'); + expect(testTypes.TestClass.get()).toBe('mockTestMethod'); expect(mockTestMethod).toHaveBeenCalledTimes(1); mockTestMethod.mockClear(); @@ -194,11 +217,11 @@ describe('Testing the mocking of a class hierarchy defined in a single import', it('can call a superclass static method named "set" - Auto-mocked class', () => { const mockTestMethod = jest - .spyOn(TestClass, 'set') + .spyOn(testTypes.TestClass, 'set') .mockImplementation(() => { return 'mockTestMethod'; }); - expect(TestClass.set()).toBe('mockTestMethod'); + expect(testTypes.TestClass.set()).toBe('mockTestMethod'); expect(mockTestMethod).toHaveBeenCalledTimes(1); mockTestMethod.mockClear(); @@ -218,11 +241,13 @@ describe('Testing the mocking of a class hierarchy defined in a single import', it('can read a value from a superclass static getter - Auto-mocked class', () => { const mockTestMethod = jest - .spyOn(TestClass, 'staticTestAccessor', 'get') + .spyOn(testTypes.TestClass, 'staticTestAccessor', 'get') .mockImplementation(() => { return 'mockStaticTestAccessor'; }); - expect(TestClass.staticTestAccessor).toBe('mockStaticTestAccessor'); + expect(testTypes.TestClass.staticTestAccessor).toBe( + 'mockStaticTestAccessor', + ); expect(mockTestMethod).toHaveBeenCalledTimes(1); }); @@ -240,11 +265,11 @@ describe('Testing the mocking of a class hierarchy defined in a single import', it('can write a value to a superclass static setter - Auto-mocked class', () => { const mockTestMethod = jest - .spyOn(TestClass, 'staticTestAccessor', 'set') + .spyOn(testTypes.TestClass, 'staticTestAccessor', 'set') .mockImplementation((_x: string) => { return () => {}; }); - TestClass.staticTestAccessor = ''; + testTypes.TestClass.staticTestAccessor = ''; expect(mockTestMethod).toHaveBeenCalledTimes(1); }); }); From d9efe2d392c85b9a4eb9c7827b8ff25daa0215d7 Mon Sep 17 00:00:00 2001 From: Peter Staples Date: Sat, 15 Oct 2022 13:00:08 +0100 Subject: [PATCH 08/13] bug-13140 - reinstating spyOn functionality that allowed mocking of functions exported as accessors/getters --- packages/jest-mock/src/index.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/jest-mock/src/index.ts b/packages/jest-mock/src/index.ts index 408690efaaab..573299067f40 100644 --- a/packages/jest-mock/src/index.ts +++ b/packages/jest-mock/src/index.ts @@ -1193,12 +1193,15 @@ export class ModuleMocker { ${this._typeOf(descriptor?.[accessType])} given instead`, ); } - } else if (typeof descriptor.value !== 'function') { + } else if ( + (descriptor.value && typeof descriptor.value !== 'function') || + (descriptor.get && typeof descriptor.get !== 'function') + ) { throw new Error( `Cannot spy the ${String( methodKey, )} property because it is not a function; ${this._typeOf( - descriptor.value, + descriptor.value ? descriptor.value : descriptor.get, )} given instead`, ); } @@ -1239,6 +1242,14 @@ export class ModuleMocker { return originalAccessor.call(this, arguments[0]); }); Object.defineProperty(object, methodKey, descriptor); + } else if (descriptor.get) { + const originalGet = descriptor.get; + mock = this._makeComponent({type: 'function'}, () => { + descriptor!.get = originalGet; + Object.defineProperty(object, methodKey, descriptor!); + }); + descriptor.get = () => mock; + Object.defineProperty(object, methodKey, descriptor); } else { const isMethodOwner = Object.prototype.hasOwnProperty.call( object, From f329d8a8d62220981e3bc4eb516cbb86db4e5a46 Mon Sep 17 00:00:00 2001 From: Peter Staples Date: Mon, 17 Oct 2022 04:35:48 +0100 Subject: [PATCH 09/13] bug-13140: Added restoring of accessor properties. Updated tests for restoring of accessor properties. Test for function exports via getters. --- .../src/__tests__/class-mocks.test.ts | 183 ++++++++++++++---- packages/jest-mock/src/index.ts | 37 ++-- 2 files changed, 168 insertions(+), 52 deletions(-) diff --git a/packages/jest-mock/src/__tests__/class-mocks.test.ts b/packages/jest-mock/src/__tests__/class-mocks.test.ts index 4b33c6c532dd..6c6a8785e0fb 100644 --- a/packages/jest-mock/src/__tests__/class-mocks.test.ts +++ b/packages/jest-mock/src/__tests__/class-mocks.test.ts @@ -5,6 +5,27 @@ * LICENSE file in the root directory of this source tree. * */ +describe('Testing the mocking of the format of a function export exposed on import through an accessor/getter', () => { + it('can mock a function export', () => { + const exports = { + __esModule: true, + fn: () => {}, + }; + Object.defineProperty(exports, 'fn', { + configurable: true, + enumerable: true, + get: () => { + () => 'testFunction'; + }, + }); + + jest.spyOn(exports, 'fn').mockImplementation(() => 'mockTestFunction'); + expect(exports.fn()).toBe('mockTestFunction'); + + // mockFn.mockRestore(); + // expect(exports.fn()).toBe('testFunction'); + }); +}); describe('Testing the mocking of a class', () => { it('can call an instance method', () => { @@ -14,11 +35,14 @@ describe('Testing the mocking of a class', () => { } } - jest.spyOn(TestClass.prototype, 'testMethod').mockImplementation(() => { - return 'mockTestMethod'; - }); + const mockFn = jest + .spyOn(TestClass.prototype, 'testMethod') + .mockImplementation(() => 'mockTestMethod'); const testClassInstance = new TestClass(); expect(testClassInstance.testMethod()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(testClassInstance.testMethod()).toBe('testMethod'); }); it('can call a superclass instance method', () => { @@ -30,11 +54,17 @@ describe('Testing the mocking of a class', () => { class TestClass extends SuperTestClass {} - jest.spyOn(TestClass.prototype, 'testMethod').mockImplementation(() => { - return 'mockTestMethod'; - }); + const mockFn = jest + .spyOn(TestClass.prototype, 'testMethod') + .mockImplementation(() => { + return 'mockTestMethod'; + }); const testClassInstance = new TestClass(); expect(testClassInstance.testMethod()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(testClassInstance.testMethod()).toBe('testMethod'); + expect(Object.hasOwn(TestClass.prototype, 'testMethod')).toBe(false); }); it('can call an instance method named "get"', () => { @@ -44,11 +74,16 @@ describe('Testing the mocking of a class', () => { } } - jest.spyOn(TestClass.prototype, 'get').mockImplementation(() => { - return 'mockTestMethod'; - }); + const mockFn = jest + .spyOn(TestClass.prototype, 'get') + .mockImplementation(() => { + return 'mockTestMethod'; + }); const testClassInstance = new TestClass(); expect(testClassInstance.get()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(testClassInstance.get()).toBe('get'); }); it('can call a superclass instance method named "get"', () => { @@ -60,11 +95,17 @@ describe('Testing the mocking of a class', () => { class TestClass extends SuperTestClass {} - jest.spyOn(TestClass.prototype, 'get').mockImplementation(() => { - return 'mockTestMethod'; - }); + const mockFn = jest + .spyOn(TestClass.prototype, 'get') + .mockImplementation(() => { + return 'mockTestMethod'; + }); const testClassInstance = new TestClass(); expect(testClassInstance.get()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(testClassInstance.get()).toBe('get'); + expect(Object.hasOwn(TestClass.prototype, 'get')).toBe(false); }); it('can call an instance method named "set"', () => { @@ -74,11 +115,16 @@ describe('Testing the mocking of a class', () => { } } - jest.spyOn(TestClass.prototype, 'set').mockImplementation(() => { - return 'mockTestMethod'; - }); + const mockFn = jest + .spyOn(TestClass.prototype, 'set') + .mockImplementation(() => { + return 'mockTestMethod'; + }); const testClassInstance = new TestClass(); expect(testClassInstance.set()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(testClassInstance.set()).toBe('set'); }); it('can call a superclass instance method named "set"', () => { @@ -90,11 +136,17 @@ describe('Testing the mocking of a class', () => { class TestClass extends SuperTestClass {} - jest.spyOn(TestClass.prototype, 'set').mockImplementation(() => { - return 'mockTestMethod'; - }); + const mockFn = jest + .spyOn(TestClass.prototype, 'set') + .mockImplementation(() => { + return 'mockTestMethod'; + }); const testClassInstance = new TestClass(); expect(testClassInstance.set()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(testClassInstance.set()).toBe('set'); + expect(Object.hasOwn(TestClass.prototype, 'set')).toBe(false); }); it('can read a value from an instance getter', () => { @@ -104,13 +156,16 @@ describe('Testing the mocking of a class', () => { } } - jest + const mockFn = jest .spyOn(TestClass.prototype, 'testMethod', 'get') .mockImplementation(() => { return 'mockTestMethod'; }); const testClassInstance = new TestClass(); expect(testClassInstance.testMethod).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(testClassInstance.testMethod).toBe('testMethod'); }); it('can read a value from an superclass instance getter', () => { @@ -122,13 +177,17 @@ describe('Testing the mocking of a class', () => { class TestClass extends SuperTestClass {} - jest + const mockFn = jest .spyOn(TestClass.prototype, 'testMethod', 'get') .mockImplementation(() => { return 'mockTestMethod'; }); const testClassInstance = new TestClass(); expect(testClassInstance.testMethod).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(testClassInstance.testMethod).toBe('testMethod'); + expect(Object.hasOwn(TestClass.prototype, 'testMethod')).toBe(false); }); it('can write a value to an instance setter', () => { @@ -147,6 +206,10 @@ describe('Testing the mocking of a class', () => { const testClassInstance = new TestClass(); testClassInstance.testMethod = ''; expect(mocktestMethod).toHaveBeenCalledTimes(1); + + mocktestMethod.mockRestore(); + testClassInstance.testMethod = ''; + expect(mocktestMethod).toHaveBeenCalledTimes(0); }); it('can write a value to a superclass instance setter', () => { @@ -167,6 +230,11 @@ describe('Testing the mocking of a class', () => { const testClassInstance = new TestClass(); testClassInstance.testMethod = ''; expect(mocktestMethod).toHaveBeenCalledTimes(1); + + mocktestMethod.mockRestore(); + testClassInstance.testMethod = ''; + expect(mocktestMethod).toHaveBeenCalledTimes(0); + expect(Object.hasOwn(TestClass.prototype, 'testMethod')).toBe(false); }); it('can call a static method', () => { @@ -176,10 +244,15 @@ describe('Testing the mocking of a class', () => { } } - jest.spyOn(TestClass, 'testMethod').mockImplementation(() => { - return 'mockTestMethod'; - }); + const mockFn = jest + .spyOn(TestClass, 'testMethod') + .mockImplementation(() => { + return 'mockTestMethod'; + }); expect(TestClass.testMethod()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(TestClass.testMethod()).toBe('testMethod'); }); it('can call a superclass static method', () => { @@ -191,10 +264,16 @@ describe('Testing the mocking of a class', () => { class TestClass extends SuperTestClass {} - jest.spyOn(TestClass, 'testMethod').mockImplementation(() => { - return 'mockTestMethod'; - }); + const mockFn = jest + .spyOn(TestClass, 'testMethod') + .mockImplementation(() => { + return 'mockTestMethod'; + }); expect(TestClass.testMethod()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(TestClass.testMethod()).toBe('testMethod'); + expect(Object.hasOwn(TestClass, 'testMethod')).toBe(false); }); it('can call a static method named "get"', () => { @@ -204,10 +283,13 @@ describe('Testing the mocking of a class', () => { } } - jest.spyOn(TestClass, 'get').mockImplementation(() => { + const mockFn = jest.spyOn(TestClass, 'get').mockImplementation(() => { return 'mockTestMethod'; }); expect(TestClass.get()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(TestClass.get()).toBe('get'); }); it('can call a superclass static method named "get"', () => { @@ -219,10 +301,14 @@ describe('Testing the mocking of a class', () => { class TestClass extends SuperTestClass {} - jest.spyOn(TestClass, 'get').mockImplementation(() => { + const mockFn = jest.spyOn(TestClass, 'get').mockImplementation(() => { return 'mockTestMethod'; }); expect(TestClass.get()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(TestClass.get()).toBe('get'); + expect(Object.hasOwn(TestClass, 'get')).toBe(false); }); it('can call a static method named "set"', () => { @@ -232,10 +318,13 @@ describe('Testing the mocking of a class', () => { } } - jest.spyOn(TestClass, 'set').mockImplementation(() => { + const mockFn = jest.spyOn(TestClass, 'set').mockImplementation(() => { return 'mockTestMethod'; }); expect(TestClass.set()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(TestClass.set()).toBe('set'); }); it('can call a superclass static method named "set"', () => { @@ -247,10 +336,14 @@ describe('Testing the mocking of a class', () => { class TestClass extends SuperTestClass {} - jest.spyOn(TestClass, 'set').mockImplementation(() => { + const mockFn = jest.spyOn(TestClass, 'set').mockImplementation(() => { return 'mockTestMethod'; }); expect(TestClass.set()).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(TestClass.set()).toBe('set'); + expect(Object.hasOwn(TestClass, 'set')).toBe(false); }); it('can read a value from a static getter', () => { @@ -260,10 +353,15 @@ describe('Testing the mocking of a class', () => { } } - jest.spyOn(TestClass, 'testMethod', 'get').mockImplementation(() => { - return 'mockTestMethod'; - }); + const mockFn = jest + .spyOn(TestClass, 'testMethod', 'get') + .mockImplementation(() => { + return 'mockTestMethod'; + }); expect(TestClass.testMethod).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(TestClass.testMethod).toBe('testMethod'); }); it('can read a value from a superclass static getter', () => { @@ -275,10 +373,16 @@ describe('Testing the mocking of a class', () => { class TestClass extends SuperTestClass {} - jest.spyOn(TestClass, 'testMethod', 'get').mockImplementation(() => { - return 'mockTestMethod'; - }); + const mockFn = jest + .spyOn(TestClass, 'testMethod', 'get') + .mockImplementation(() => { + return 'mockTestMethod'; + }); expect(TestClass.testMethod).toBe('mockTestMethod'); + + mockFn.mockRestore(); + expect(TestClass.testMethod).toBe('testMethod'); + expect(Object.hasOwn(TestClass, 'testMethod')).toBe(false); }); it('can write a value to a static setter', () => { @@ -296,6 +400,9 @@ describe('Testing the mocking of a class', () => { }); TestClass.testMethod = ''; expect(mocktestMethod).toHaveBeenCalledTimes(1); + + mocktestMethod.mockRestore(); + expect(mocktestMethod).toHaveBeenCalledTimes(0); }); it('can write a value to a superclass static setter', () => { @@ -315,5 +422,9 @@ describe('Testing the mocking of a class', () => { }); TestClass.testMethod = ''; expect(mocktestMethod).toHaveBeenCalledTimes(1); + + mocktestMethod.mockRestore(); + expect(mocktestMethod).toHaveBeenCalledTimes(0); + expect(Object.hasOwn(TestClass, 'testMethod')).toBe(false); }); }); diff --git a/packages/jest-mock/src/index.ts b/packages/jest-mock/src/index.ts index 573299067f40..e846882768da 100644 --- a/packages/jest-mock/src/index.ts +++ b/packages/jest-mock/src/index.ts @@ -1165,9 +1165,11 @@ export class ModuleMocker { let descriptor = Object.getOwnPropertyDescriptor(object, methodKey); let proto = Object.getPrototypeOf(object); + let isMethodOwner = true; while (!descriptor && proto !== null) { descriptor = Object.getOwnPropertyDescriptor(proto, methodKey); proto = Object.getPrototypeOf(proto); + isMethodOwner = false; } if (!descriptor) { throw new Error(`${String(methodKey)} property does not exist`); @@ -1208,41 +1210,48 @@ export class ModuleMocker { let mock: Mock; - if (accessType == 'get' && descriptor['get']) { - const originalAccessor = descriptor['get']; + if (accessType == 'get' && descriptor.get) { + const originalAccessor = descriptor.get; mock = this._makeComponent( { type: 'function', }, () => { - descriptor![accessType] = originalAccessor; - Object.defineProperty(object, methodKey, descriptor!); + if (isMethodOwner) { + descriptor!.get = originalAccessor; + Object.defineProperty(object, methodKey, descriptor!); + } else { + delete object[methodKey]; + } }, ); - - descriptor[accessType] = mock; + descriptor.get = mock; mock.mockImplementation(function (this: unknown) { return originalAccessor.call(this); }); Object.defineProperty(object, methodKey, descriptor); - } else if (accessType == 'set' && descriptor['set']) { - const originalAccessor = descriptor['set']; + } else if (accessType == 'set' && descriptor.set) { + const originalAccessor = descriptor.set; mock = this._makeComponent( { type: 'function', }, () => { - descriptor![accessType] = originalAccessor; - Object.defineProperty(object, methodKey, descriptor!); + if (isMethodOwner) { + descriptor!.set = originalAccessor; + Object.defineProperty(object, methodKey, descriptor!); + } else { + delete object[methodKey]; + } }, ); - - descriptor[accessType] = mock; + descriptor.set = mock; mock.mockImplementation(function (this: unknown) { return originalAccessor.call(this, arguments[0]); }); Object.defineProperty(object, methodKey, descriptor); } else if (descriptor.get) { + //this branch exists for function exports exposed through accessors/getters during import const originalGet = descriptor.get; mock = this._makeComponent({type: 'function'}, () => { descriptor!.get = originalGet; @@ -1251,10 +1260,6 @@ export class ModuleMocker { descriptor.get = () => mock; Object.defineProperty(object, methodKey, descriptor); } else { - const isMethodOwner = Object.prototype.hasOwnProperty.call( - object, - methodKey, - ); const original = descriptor; mock = this._makeComponent( From bd5ab678d01cecbf96d2c77f6e2ea9259d897e43 Mon Sep 17 00:00:00 2001 From: Peter Staples Date: Mon, 17 Oct 2022 13:23:52 +0100 Subject: [PATCH 10/13] bug-13140: falling back to user of hasOwnProperty() in tests --- .../src/__tests__/class-mocks.test.ts | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/jest-mock/src/__tests__/class-mocks.test.ts b/packages/jest-mock/src/__tests__/class-mocks.test.ts index 6c6a8785e0fb..05926daf3c09 100644 --- a/packages/jest-mock/src/__tests__/class-mocks.test.ts +++ b/packages/jest-mock/src/__tests__/class-mocks.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable no-prototype-builtins */ /** * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. * @@ -64,7 +65,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(testClassInstance.testMethod()).toBe('testMethod'); - expect(Object.hasOwn(TestClass.prototype, 'testMethod')).toBe(false); + expect(TestClass.prototype.hasOwnProperty('testMethod')).toBe(false); }); it('can call an instance method named "get"', () => { @@ -105,7 +106,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(testClassInstance.get()).toBe('get'); - expect(Object.hasOwn(TestClass.prototype, 'get')).toBe(false); + expect(TestClass.prototype.hasOwnProperty('get')).toBe(false); }); it('can call an instance method named "set"', () => { @@ -146,7 +147,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(testClassInstance.set()).toBe('set'); - expect(Object.hasOwn(TestClass.prototype, 'set')).toBe(false); + expect(TestClass.prototype.hasOwnProperty('set')).toBe(false); }); it('can read a value from an instance getter', () => { @@ -187,7 +188,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(testClassInstance.testMethod).toBe('testMethod'); - expect(Object.hasOwn(TestClass.prototype, 'testMethod')).toBe(false); + expect(TestClass.prototype.hasOwnProperty('testMethod')).toBe(false); }); it('can write a value to an instance setter', () => { @@ -234,7 +235,7 @@ describe('Testing the mocking of a class', () => { mocktestMethod.mockRestore(); testClassInstance.testMethod = ''; expect(mocktestMethod).toHaveBeenCalledTimes(0); - expect(Object.hasOwn(TestClass.prototype, 'testMethod')).toBe(false); + expect(TestClass.prototype.hasOwnProperty('testMethod')).toBe(false); }); it('can call a static method', () => { @@ -273,7 +274,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(TestClass.testMethod()).toBe('testMethod'); - expect(Object.hasOwn(TestClass, 'testMethod')).toBe(false); + expect(TestClass.hasOwnProperty('testMethod')).toBe(false); }); it('can call a static method named "get"', () => { @@ -308,7 +309,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(TestClass.get()).toBe('get'); - expect(Object.hasOwn(TestClass, 'get')).toBe(false); + expect(TestClass.hasOwnProperty('get')).toBe(false); }); it('can call a static method named "set"', () => { @@ -343,7 +344,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(TestClass.set()).toBe('set'); - expect(Object.hasOwn(TestClass, 'set')).toBe(false); + expect(TestClass.hasOwnProperty('set')).toBe(false); }); it('can read a value from a static getter', () => { @@ -382,7 +383,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(TestClass.testMethod).toBe('testMethod'); - expect(Object.hasOwn(TestClass, 'testMethod')).toBe(false); + expect(TestClass.hasOwnProperty('testMethod')).toBe(false); }); it('can write a value to a static setter', () => { @@ -425,6 +426,6 @@ describe('Testing the mocking of a class', () => { mocktestMethod.mockRestore(); expect(mocktestMethod).toHaveBeenCalledTimes(0); - expect(Object.hasOwn(TestClass, 'testMethod')).toBe(false); + expect(TestClass.hasOwnProperty('testMethod')).toBe(false); }); }); From 80375e2994923712c2c5e8e54e71aa19ae9b4f58 Mon Sep 17 00:00:00 2001 From: Peter Staples Date: Mon, 17 Oct 2022 13:43:27 +0100 Subject: [PATCH 11/13] bug-13140: applying lint check exception per line --- packages/jest-mock/src/__tests__/class-mocks.test.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/jest-mock/src/__tests__/class-mocks.test.ts b/packages/jest-mock/src/__tests__/class-mocks.test.ts index 05926daf3c09..4f618cce74c9 100644 --- a/packages/jest-mock/src/__tests__/class-mocks.test.ts +++ b/packages/jest-mock/src/__tests__/class-mocks.test.ts @@ -1,4 +1,3 @@ -/* eslint-disable no-prototype-builtins */ /** * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. * @@ -65,6 +64,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(testClassInstance.testMethod()).toBe('testMethod'); + // eslint-disable-next-line no-prototype-builtins expect(TestClass.prototype.hasOwnProperty('testMethod')).toBe(false); }); @@ -106,6 +106,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(testClassInstance.get()).toBe('get'); + // eslint-disable-next-line no-prototype-builtins expect(TestClass.prototype.hasOwnProperty('get')).toBe(false); }); @@ -147,6 +148,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(testClassInstance.set()).toBe('set'); + // eslint-disable-next-line no-prototype-builtins expect(TestClass.prototype.hasOwnProperty('set')).toBe(false); }); @@ -188,6 +190,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(testClassInstance.testMethod).toBe('testMethod'); + // eslint-disable-next-line no-prototype-builtins expect(TestClass.prototype.hasOwnProperty('testMethod')).toBe(false); }); @@ -235,6 +238,7 @@ describe('Testing the mocking of a class', () => { mocktestMethod.mockRestore(); testClassInstance.testMethod = ''; expect(mocktestMethod).toHaveBeenCalledTimes(0); + // eslint-disable-next-line no-prototype-builtins expect(TestClass.prototype.hasOwnProperty('testMethod')).toBe(false); }); @@ -274,6 +278,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(TestClass.testMethod()).toBe('testMethod'); + // eslint-disable-next-line no-prototype-builtins expect(TestClass.hasOwnProperty('testMethod')).toBe(false); }); @@ -309,6 +314,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(TestClass.get()).toBe('get'); + // eslint-disable-next-line no-prototype-builtins expect(TestClass.hasOwnProperty('get')).toBe(false); }); @@ -344,6 +350,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(TestClass.set()).toBe('set'); + // eslint-disable-next-line no-prototype-builtins expect(TestClass.hasOwnProperty('set')).toBe(false); }); @@ -383,6 +390,7 @@ describe('Testing the mocking of a class', () => { mockFn.mockRestore(); expect(TestClass.testMethod).toBe('testMethod'); + // eslint-disable-next-line no-prototype-builtins expect(TestClass.hasOwnProperty('testMethod')).toBe(false); }); @@ -426,6 +434,7 @@ describe('Testing the mocking of a class', () => { mocktestMethod.mockRestore(); expect(mocktestMethod).toHaveBeenCalledTimes(0); + // eslint-disable-next-line no-prototype-builtins expect(TestClass.hasOwnProperty('testMethod')).toBe(false); }); }); From 2facdf166c7e6a7882737048c7351480a25f8d9c Mon Sep 17 00:00:00 2001 From: Peter Staples Date: Wed, 19 Oct 2022 00:39:17 +0100 Subject: [PATCH 12/13] bug-13140: reverting spyOn methods to the original format. --- .../__tests__/class-mocks-dual-import.test.ts | 92 ++++++++ .../src/__tests__/class-mocks.test.ts | 22 -- packages/jest-mock/src/index.ts | 211 +++++++++--------- 3 files changed, 201 insertions(+), 124 deletions(-) diff --git a/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts b/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts index 3bcc759e6d68..610469e9deba 100644 --- a/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts +++ b/packages/jest-mock/src/__tests__/class-mocks-dual-import.test.ts @@ -35,4 +35,96 @@ describe('Testing the mocking of a class hierarchy defined in multiple imports', expect(testClassInstance.testMethod()).toBe('mockTestMethod'); expect(mockTestMethod).toHaveBeenCalledTimes(1); }); + + it('can read a value from an instance getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass.prototype, 'testAccessor', 'get') + .mockImplementation(() => { + return 'mockTestAccessor'; + }); + const testClassInstance = new SuperTestClass(); + expect(testClassInstance.testAccessor).toBe('mockTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can read a value from a superclass instance getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass.prototype, 'testAccessor', 'get') + .mockImplementation(() => { + return 'mockTestAccessor'; + }); + const testClassInstance = new TestClass(); + expect(testClassInstance.testAccessor).toBe('mockTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can write a value to an instance setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass.prototype, 'testAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + const testClassInstance = new SuperTestClass(); + testClassInstance.testAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can write a value to a superclass instance setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass.prototype, 'testAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + const testClassInstance = new TestClass(); + testClassInstance.testAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can read a value from a static getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass, 'staticTestAccessor', 'get') + .mockImplementation(() => { + return 'mockStaticTestAccessor'; + }); + expect(SuperTestClass.staticTestAccessor).toBe('mockStaticTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can read a value from a superclass static getter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass, 'staticTestAccessor', 'get') + .mockImplementation(() => { + return 'mockStaticTestAccessor'; + }); + expect(TestClass.staticTestAccessor).toBe('mockStaticTestAccessor'); + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); + + it('can write a value to a static setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(SuperTestClass, 'staticTestAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + SuperTestClass.staticTestAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + + mockTestMethod.mockClear(); + }); + + it('can write a value to a superclass static setter - Auto-mocked class', () => { + const mockTestMethod = jest + .spyOn(TestClass, 'staticTestAccessor', 'set') + .mockImplementation((_x: string) => { + return () => {}; + }); + TestClass.staticTestAccessor = ''; + expect(mockTestMethod).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/jest-mock/src/__tests__/class-mocks.test.ts b/packages/jest-mock/src/__tests__/class-mocks.test.ts index 4f618cce74c9..0909e2040f4f 100644 --- a/packages/jest-mock/src/__tests__/class-mocks.test.ts +++ b/packages/jest-mock/src/__tests__/class-mocks.test.ts @@ -5,28 +5,6 @@ * LICENSE file in the root directory of this source tree. * */ -describe('Testing the mocking of the format of a function export exposed on import through an accessor/getter', () => { - it('can mock a function export', () => { - const exports = { - __esModule: true, - fn: () => {}, - }; - Object.defineProperty(exports, 'fn', { - configurable: true, - enumerable: true, - get: () => { - () => 'testFunction'; - }, - }); - - jest.spyOn(exports, 'fn').mockImplementation(() => 'mockTestFunction'); - expect(exports.fn()).toBe('mockTestFunction'); - - // mockFn.mockRestore(); - // expect(exports.fn()).toBe('testFunction'); - }); -}); - describe('Testing the mocking of a class', () => { it('can call an instance method', () => { class TestClass { diff --git a/packages/jest-mock/src/index.ts b/packages/jest-mock/src/index.ts index 639ca02a51c7..0c1523828ff1 100644 --- a/packages/jest-mock/src/index.ts +++ b/packages/jest-mock/src/index.ts @@ -1157,6 +1157,12 @@ export class ModuleMocker { ); } + if (typeof object !== 'object' && typeof object !== 'function') { + throw new Error( + `Cannot spyOn on a primitive value; ${this._typeOf(object)} given`, + ); + } + if (!methodKey) { throw new Error('No property name supplied'); } @@ -1165,132 +1171,133 @@ export class ModuleMocker { throw new Error('Invalid accessType supplied'); } - if (typeof object !== 'object' && typeof object !== 'function') { - throw new Error( - `Cannot spyOn on a primitive value; ${this._typeOf(object)} given`, - ); + if (accessType) { + return this._spyOnProperty(object, methodKey, accessType); + } + + //some properties do not return a property descriptor but do return + //a fn definition when read, e.g. window.dispatchEvent + const original = object[methodKey]; + + if (!this.isMockFunction(original)) { + if (typeof original !== 'function') { + throw new Error( + `Cannot spy the ${String( + methodKey, + )} property because it is not a function; ${this._typeOf( + original, + )} given instead`, + ); + } + + let mock: Mock; + //This descriptor is used to determine if a function/class import is exposed via a getter. + //No inheritance is possible, and no conditional check is made on ownership during restore, + //thus the property is expected directly on the object. + const descriptor = Object.getOwnPropertyDescriptor(object, methodKey); + + if (descriptor && descriptor.get) { + if (!descriptor.configurable) { + throw new Error(`${String(methodKey)} is not declared configurable`); + } + const originalGet = descriptor.get; + mock = this._makeComponent({type: 'function'}, () => { + descriptor!.get = originalGet; + Object.defineProperty(object, methodKey, descriptor!); + }); + descriptor.get = () => mock; + Object.defineProperty(object, methodKey, descriptor); + } else { + const isMethodOwner = Object.prototype.hasOwnProperty.call( + object, + methodKey, + ); + + 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; + } + + mock.mockImplementation(function (this: unknown) { + return original.apply(this, arguments); + }); } - let descriptor = Object.getOwnPropertyDescriptor(object, methodKey); - let proto = Object.getPrototypeOf(object); - let isMethodOwner = true; + return object[methodKey]; + } + + private _spyOnProperty>( + obj: T, + propertyKey: K, + accessType: 'get' | 'set', + ): Mock<() => T> { + let descriptor = Object.getOwnPropertyDescriptor(obj, propertyKey); + let proto = Object.getPrototypeOf(obj); + while (!descriptor && proto !== null) { - descriptor = Object.getOwnPropertyDescriptor(proto, methodKey); + descriptor = Object.getOwnPropertyDescriptor(proto, propertyKey); proto = Object.getPrototypeOf(proto); - isMethodOwner = false; } + if (!descriptor) { - throw new Error(`${String(methodKey)} property does not exist`); + throw new Error(`${String(propertyKey)} property does not exist`); } + if (!descriptor.configurable) { - throw new Error(`${String(methodKey)} is not declared configurable`); + throw new Error(`${String(propertyKey)} is not declared configurable`); } - if (this.isMockFunction(descriptor.value)) { - return object[methodKey]; - } else if (accessType == 'get' && this.isMockFunction(descriptor.get)) { - return descriptor.get; - } else if (accessType == 'set' && this.isMockFunction(descriptor.set)) { - return descriptor.set; + if (!descriptor[accessType]) { + throw new Error( + `Property ${String( + propertyKey, + )} does not have access type ${accessType}`, + ); } - if (accessType) { - if (typeof descriptor[accessType] !== 'function') { + const original = descriptor[accessType]; + + if (!this.isMockFunction(original)) { + if (typeof original !== 'function') { throw new Error( `Cannot spy the ${String(accessType)} ${String( - methodKey, + propertyKey, )} property because it is not a function; - ${this._typeOf(descriptor?.[accessType])} given instead`, + ${this._typeOf(descriptor.get)} given instead`, ); } - } else if ( - (descriptor.value && typeof descriptor.value !== 'function') || - (descriptor.get && typeof descriptor.get !== 'function') - ) { - throw new Error( - `Cannot spy the ${String( - methodKey, - )} property because it is not a function; ${this._typeOf( - descriptor.value ? descriptor.value : descriptor.get, - )} given instead`, - ); - } - - let mock: Mock; - if (accessType == 'get' && descriptor.get) { - const originalAccessor = descriptor.get; - mock = this._makeComponent( - { - type: 'function', - }, - () => { - if (isMethodOwner) { - descriptor!.get = originalAccessor; - Object.defineProperty(object, methodKey, descriptor!); - } else { - delete object[methodKey]; - } - }, + const isMethodOwner = Object.prototype.hasOwnProperty.call( + obj, + propertyKey, ); - descriptor.get = mock; - mock.mockImplementation(function (this: unknown) { - return originalAccessor.call(this); - }); - Object.defineProperty(object, methodKey, descriptor); - } else if (accessType == 'set' && descriptor.set) { - const originalAccessor = descriptor.set; - mock = this._makeComponent( - { - type: 'function', - }, - () => { - if (isMethodOwner) { - descriptor!.set = originalAccessor; - Object.defineProperty(object, methodKey, descriptor!); - } else { - delete object[methodKey]; - } - }, - ); - descriptor.set = mock; - mock.mockImplementation(function (this: unknown) { - return originalAccessor.call(this, arguments[0]); - }); - Object.defineProperty(object, methodKey, descriptor); - } else if (descriptor.get) { - //this branch exists for function exports exposed through accessors/getters during import - const originalGet = descriptor.get; - mock = this._makeComponent({type: 'function'}, () => { - descriptor!.get = originalGet; - Object.defineProperty(object, methodKey, descriptor!); - }); - descriptor.get = () => mock; - Object.defineProperty(object, methodKey, descriptor); - } else { - const original = descriptor; - mock = this._makeComponent( - { - type: 'function', - }, - () => { - if (isMethodOwner) { - object[methodKey] = original.value; - } else { - delete object[methodKey]; - } - }, - ); + descriptor[accessType] = this._makeComponent({type: 'function'}, () => { + if (isMethodOwner) { + // @ts-expect-error: mock is assignable + descriptor![accessType] = original; + Object.defineProperty(obj, propertyKey, descriptor!); + } else { + delete obj[propertyKey]; + } + }); - // @ts-expect-error overriding original method with a Mock - object[methodKey] = mock; - mock.mockImplementation(function (this: unknown) { - return original.value.apply(this, arguments); + (descriptor[accessType] as Mock<() => T>).mockImplementation(function ( + this: unknown, + ) { + // @ts-expect-error - wrong context + return original.apply(this, arguments); }); } - return mock; + Object.defineProperty(obj, propertyKey, descriptor); + return descriptor[accessType] as Mock<() => T>; } clearAllMocks(): void { From ee0013a1ddfe0a934b2e9ce84a04da41a9ee3634 Mon Sep 17 00:00:00 2001 From: Peter Staples Date: Wed, 19 Oct 2022 01:15:02 +0100 Subject: [PATCH 13/13] bug-13140: reinstating change removed from last merge frommain --- packages/jest-mock/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jest-mock/src/index.ts b/packages/jest-mock/src/index.ts index 0c1523828ff1..ec0eebef3a42 100644 --- a/packages/jest-mock/src/index.ts +++ b/packages/jest-mock/src/index.ts @@ -1314,7 +1314,7 @@ export class ModuleMocker { this._spyState = new Set(); } - private _typeOf(value: any): string { + private _typeOf(value: unknown): string { return value == null ? `${value}` : typeof value; }