From 2b96a04728d43db6e882aab6b7b1ba9c1d24fb7c Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Nov 2023 18:12:52 +0900 Subject: [PATCH 01/15] fix: mock each class instance's method separately --- packages/vitest/src/runtime/mocker.ts | 16 +++++++++++++++- test/core/src/mockedE.ts | 11 +++++++++++ test/core/test/mocked-wip.test.ts | 21 +++++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 test/core/src/mockedE.ts create mode 100644 test/core/test/mocked-wip.test.ts diff --git a/packages/vitest/src/runtime/mocker.ts b/packages/vitest/src/runtime/mocker.ts index 44812b8b3cb0..028f8ced4cad 100644 --- a/packages/vitest/src/runtime/mocker.ts +++ b/packages/vitest/src/runtime/mocker.ts @@ -328,7 +328,21 @@ export class VitestMocker { const spyModule = this.spyModule if (!spyModule) throw this.createError('[vitest] `spyModule` is not defined. This is Vitest error. Please open a new issue with reproduction.') - const mock = spyModule.spyOn(newContainer, property).mockImplementation(() => undefined) + + const mock = spyModule.spyOn(newContainer, property).mockImplementation(function (this: any) { + // do similar to ject? https://github.com/jestjs/jest/blob/2c3d2409879952157433de215ae0eee5188a4384/packages/jest-mock/src/index.ts#L678-L691 + // check constructor call + if (this instanceof newContainer[property]) { + // mock each class instance's method + // TODO: more sound way to loop prorotype methods? + for (const key in this) { + if (typeof this[key] === 'function') { + // TODO: ability to restore? + spyModule.spyOn(this, key).mockImplementation(() => undefined) + } + } + } + }) mock.mockRestore = () => { mock.mockReset() mock.mockImplementation(() => undefined) diff --git a/test/core/src/mockedE.ts b/test/core/src/mockedE.ts new file mode 100644 index 000000000000..84bc09271609 --- /dev/null +++ b/test/core/src/mockedE.ts @@ -0,0 +1,11 @@ +export class MockedE { + public value: number + + constructor() { + this.value = 42 + } + + public doSomething() { + return 'hello' + } +} diff --git a/test/core/test/mocked-wip.test.ts b/test/core/test/mocked-wip.test.ts new file mode 100644 index 000000000000..3e1274b353ed --- /dev/null +++ b/test/core/test/mocked-wip.test.ts @@ -0,0 +1,21 @@ +import { expect, test, vi } from 'vitest' + +import { MockedE } from '../src/mockedE' + +vi.mock('../src/mockedE') + +// TODO: move to mocked.test.ts + +test('should not delete the prototype', () => { + expect(MockedE).toBeTypeOf('function') + + const instance1 = new MockedE() + const instance2 = new MockedE() + expect(instance1).not.toBe(instance2) + expect(instance1.doSomething).not.toBe(instance2.doSomething) + expect(vi.mocked(instance1.doSomething).mock).not.toBe(vi.mocked(instance2.doSomething).mock) + + instance1.doSomething() + expect(instance1.doSomething).toBeCalledTimes(1) + expect(instance2.doSomething).toBeCalledTimes(0) +}) From 5dbd3ef9eeda31363554f64c538d133535ba2711 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Nov 2023 18:20:43 +0900 Subject: [PATCH 02/15] test: update assertion (breaking change?) --- test/core/test/mocked.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/core/test/mocked.test.ts b/test/core/test/mocked.test.ts index 80ec56518296..d2b7c5219d17 100644 --- a/test/core/test/mocked.test.ts +++ b/test/core/test/mocked.test.ts @@ -76,7 +76,8 @@ describe('mocked classes', () => { expect(instance.doSomething).toBeTypeOf('function') expect(instance.doSomething()).not.toBe('A') - expect(MockedC.prototype.doSomething).toHaveBeenCalledOnce() + // TODO: check what jest does + expect(MockedC.prototype.doSomething).not.toHaveBeenCalledOnce() expect(MockedC.prototype.doSomething).not.toHaveReturnedWith('A') }) From fabcc8e61f1dc2fa9225dbb8ce93647f8b18e946 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Nov 2023 18:38:17 +0900 Subject: [PATCH 03/15] refactor: minor --- packages/vitest/src/runtime/mocker.ts | 1 - test/core/src/mockedE.ts | 6 ------ test/core/test/mocked-wip.test.ts | 3 ++- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/vitest/src/runtime/mocker.ts b/packages/vitest/src/runtime/mocker.ts index 028f8ced4cad..332ad8415400 100644 --- a/packages/vitest/src/runtime/mocker.ts +++ b/packages/vitest/src/runtime/mocker.ts @@ -328,7 +328,6 @@ export class VitestMocker { const spyModule = this.spyModule if (!spyModule) throw this.createError('[vitest] `spyModule` is not defined. This is Vitest error. Please open a new issue with reproduction.') - const mock = spyModule.spyOn(newContainer, property).mockImplementation(function (this: any) { // do similar to ject? https://github.com/jestjs/jest/blob/2c3d2409879952157433de215ae0eee5188a4384/packages/jest-mock/src/index.ts#L678-L691 // check constructor call diff --git a/test/core/src/mockedE.ts b/test/core/src/mockedE.ts index 84bc09271609..2ec3c17093ee 100644 --- a/test/core/src/mockedE.ts +++ b/test/core/src/mockedE.ts @@ -1,10 +1,4 @@ export class MockedE { - public value: number - - constructor() { - this.value = 42 - } - public doSomething() { return 'hello' } diff --git a/test/core/test/mocked-wip.test.ts b/test/core/test/mocked-wip.test.ts index 3e1274b353ed..c9f5d29d1d1c 100644 --- a/test/core/test/mocked-wip.test.ts +++ b/test/core/test/mocked-wip.test.ts @@ -6,7 +6,7 @@ vi.mock('../src/mockedE') // TODO: move to mocked.test.ts -test('should not delete the prototype', () => { +test('mock each instance method separately', () => { expect(MockedE).toBeTypeOf('function') const instance1 = new MockedE() @@ -18,4 +18,5 @@ test('should not delete the prototype', () => { instance1.doSomething() expect(instance1.doSomething).toBeCalledTimes(1) expect(instance2.doSomething).toBeCalledTimes(0) + expect(MockedE.prototype.doSomething).toBeCalledTimes(0) }) From fa9e458fbbf49d70b6c697c12e1c897d61063512 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Nov 2023 18:52:40 +0900 Subject: [PATCH 04/15] test: tweak test --- test/core/test/mocked-wip.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/core/test/mocked-wip.test.ts b/test/core/test/mocked-wip.test.ts index c9f5d29d1d1c..b28182232e0b 100644 --- a/test/core/test/mocked-wip.test.ts +++ b/test/core/test/mocked-wip.test.ts @@ -13,6 +13,7 @@ test('mock each instance method separately', () => { const instance2 = new MockedE() expect(instance1).not.toBe(instance2) expect(instance1.doSomething).not.toBe(instance2.doSomething) + expect(instance1.doSomething).not.toBe(MockedE.prototype.doSomething) expect(vi.mocked(instance1.doSomething).mock).not.toBe(vi.mocked(instance2.doSomething).mock) instance1.doSomething() From 5d5d0c182814df0e684eae5dd34fe74b128f084b Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Nov 2023 19:19:33 +0900 Subject: [PATCH 05/15] fix: delegate prototype mock call --- packages/vitest/src/runtime/mocker.ts | 10 +++++++--- test/core/src/mockedE.ts | 4 ++-- test/core/test/mocked-wip.test.ts | 17 ++++++++++++++--- test/core/test/mocked.test.ts | 3 +-- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/packages/vitest/src/runtime/mocker.ts b/packages/vitest/src/runtime/mocker.ts index 332ad8415400..105c5c145a06 100644 --- a/packages/vitest/src/runtime/mocker.ts +++ b/packages/vitest/src/runtime/mocker.ts @@ -328,8 +328,10 @@ export class VitestMocker { const spyModule = this.spyModule if (!spyModule) throw this.createError('[vitest] `spyModule` is not defined. This is Vitest error. Please open a new issue with reproduction.') - const mock = spyModule.spyOn(newContainer, property).mockImplementation(function (this: any) { - // do similar to ject? https://github.com/jestjs/jest/blob/2c3d2409879952157433de215ae0eee5188a4384/packages/jest-mock/src/index.ts#L678-L691 + const mock = spyModule.spyOn(newContainer, property).mockImplementation(function (this: any, ...args: any[]) { + // jest reference + // https://github.com/jestjs/jest/blob/2c3d2409879952157433de215ae0eee5188a4384/packages/jest-mock/src/index.ts#L678-L691 + // check constructor call if (this instanceof newContainer[property]) { // mock each class instance's method @@ -337,7 +339,9 @@ export class VitestMocker { for (const key in this) { if (typeof this[key] === 'function') { // TODO: ability to restore? - spyModule.spyOn(this, key).mockImplementation(() => undefined) + // mock but delegate to original prototype method, which should be also mocked already + const original = this[key] + spyModule.spyOn(this, key).mockImplementation(() => original.apply(this, args)) } } } diff --git a/test/core/src/mockedE.ts b/test/core/src/mockedE.ts index 2ec3c17093ee..dcdeb479f319 100644 --- a/test/core/src/mockedE.ts +++ b/test/core/src/mockedE.ts @@ -1,5 +1,5 @@ export class MockedE { - public doSomething() { - return 'hello' + public doSomething(arg: string) { + return arg.repeat(2) } } diff --git a/test/core/test/mocked-wip.test.ts b/test/core/test/mocked-wip.test.ts index b28182232e0b..4e9327bd3b40 100644 --- a/test/core/test/mocked-wip.test.ts +++ b/test/core/test/mocked-wip.test.ts @@ -4,7 +4,7 @@ import { MockedE } from '../src/mockedE' vi.mock('../src/mockedE') -// TODO: move to mocked.test.ts +// TODO: move to mocked.test.ts? test('mock each instance method separately', () => { expect(MockedE).toBeTypeOf('function') @@ -16,8 +16,19 @@ test('mock each instance method separately', () => { expect(instance1.doSomething).not.toBe(MockedE.prototype.doSomething) expect(vi.mocked(instance1.doSomething).mock).not.toBe(vi.mocked(instance2.doSomething).mock) - instance1.doSomething() + // TODO: check input/output + instance1.doSomething('a') expect(instance1.doSomething).toBeCalledTimes(1) expect(instance2.doSomething).toBeCalledTimes(0) - expect(MockedE.prototype.doSomething).toBeCalledTimes(0) + expect(MockedE.prototype.doSomething).toBeCalledTimes(1) + + instance2.doSomething('b') + expect(instance1.doSomething).toBeCalledTimes(1) + expect(instance2.doSomething).toBeCalledTimes(1) + expect(MockedE.prototype.doSomething).toBeCalledTimes(2) + + instance1.doSomething('c') + expect(instance1.doSomething).toBeCalledTimes(2) + expect(instance2.doSomething).toBeCalledTimes(1) + expect(MockedE.prototype.doSomething).toBeCalledTimes(3) }) diff --git a/test/core/test/mocked.test.ts b/test/core/test/mocked.test.ts index d2b7c5219d17..80ec56518296 100644 --- a/test/core/test/mocked.test.ts +++ b/test/core/test/mocked.test.ts @@ -76,8 +76,7 @@ describe('mocked classes', () => { expect(instance.doSomething).toBeTypeOf('function') expect(instance.doSomething()).not.toBe('A') - // TODO: check what jest does - expect(MockedC.prototype.doSomething).not.toHaveBeenCalledOnce() + expect(MockedC.prototype.doSomething).toHaveBeenCalledOnce() expect(MockedC.prototype.doSomething).not.toHaveReturnedWith('A') }) From 66c48af2c00a072e319892ab770c8dcc0b3a71d4 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Nov 2023 19:37:45 +0900 Subject: [PATCH 06/15] fix: fix arguments delegation --- packages/vitest/src/runtime/mocker.ts | 6 +- test/core/src/mockedE.ts | 2 +- test/core/test/mocked-wip.test.ts | 98 +++++++++++++++++++++------ 3 files changed, 83 insertions(+), 23 deletions(-) diff --git a/packages/vitest/src/runtime/mocker.ts b/packages/vitest/src/runtime/mocker.ts index 105c5c145a06..b3543c731719 100644 --- a/packages/vitest/src/runtime/mocker.ts +++ b/packages/vitest/src/runtime/mocker.ts @@ -328,7 +328,7 @@ export class VitestMocker { const spyModule = this.spyModule if (!spyModule) throw this.createError('[vitest] `spyModule` is not defined. This is Vitest error. Please open a new issue with reproduction.') - const mock = spyModule.spyOn(newContainer, property).mockImplementation(function (this: any, ...args: any[]) { + const mock = spyModule.spyOn(newContainer, property).mockImplementation(function (this: any) { // jest reference // https://github.com/jestjs/jest/blob/2c3d2409879952157433de215ae0eee5188a4384/packages/jest-mock/src/index.ts#L678-L691 @@ -339,9 +339,9 @@ export class VitestMocker { for (const key in this) { if (typeof this[key] === 'function') { // TODO: ability to restore? - // mock but delegate to original prototype method, which should be also mocked already + // mock by delegating calls to original prototype method, which should be also mocked already const original = this[key] - spyModule.spyOn(this, key).mockImplementation(() => original.apply(this, args)) + const _mockInner = spyModule.spyOn(this, key).mockImplementation((...args: any[]) => original.apply(this, args)) } } } diff --git a/test/core/src/mockedE.ts b/test/core/src/mockedE.ts index dcdeb479f319..607a90bdc1b3 100644 --- a/test/core/src/mockedE.ts +++ b/test/core/src/mockedE.ts @@ -1,5 +1,5 @@ export class MockedE { - public doSomething(arg: string) { + public testFn(arg: string) { return arg.repeat(2) } } diff --git a/test/core/test/mocked-wip.test.ts b/test/core/test/mocked-wip.test.ts index 4e9327bd3b40..ba7ceef7803a 100644 --- a/test/core/test/mocked-wip.test.ts +++ b/test/core/test/mocked-wip.test.ts @@ -12,23 +12,83 @@ test('mock each instance method separately', () => { const instance1 = new MockedE() const instance2 = new MockedE() expect(instance1).not.toBe(instance2) - expect(instance1.doSomething).not.toBe(instance2.doSomething) - expect(instance1.doSomething).not.toBe(MockedE.prototype.doSomething) - expect(vi.mocked(instance1.doSomething).mock).not.toBe(vi.mocked(instance2.doSomething).mock) - - // TODO: check input/output - instance1.doSomething('a') - expect(instance1.doSomething).toBeCalledTimes(1) - expect(instance2.doSomething).toBeCalledTimes(0) - expect(MockedE.prototype.doSomething).toBeCalledTimes(1) - - instance2.doSomething('b') - expect(instance1.doSomething).toBeCalledTimes(1) - expect(instance2.doSomething).toBeCalledTimes(1) - expect(MockedE.prototype.doSomething).toBeCalledTimes(2) - - instance1.doSomething('c') - expect(instance1.doSomething).toBeCalledTimes(2) - expect(instance2.doSomething).toBeCalledTimes(1) - expect(MockedE.prototype.doSomething).toBeCalledTimes(3) + expect(instance1.testFn).not.toBe(instance2.testFn) + expect(instance1.testFn).not.toBe(MockedE.prototype.testFn) + expect(vi.mocked(instance1.testFn).mock).not.toBe(vi.mocked(instance2.testFn).mock) + + instance1.testFn('a') + expect(instance1.testFn).toBeCalledTimes(1) + expect(instance2.testFn).toBeCalledTimes(0) + expect(MockedE.prototype.testFn).toBeCalledTimes(1) + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + ] + `) + expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + ] + `) + + instance2.testFn('b') + expect(instance1.testFn).toBeCalledTimes(1) + expect(instance2.testFn).toBeCalledTimes(1) + expect(MockedE.prototype.testFn).toBeCalledTimes(2) + expect(vi.mocked(instance2.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "b", + ], + ] + `) + expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + [ + "b", + ], + ] + `) + + instance1.testFn('c') + expect(instance1.testFn).toBeCalledTimes(2) + expect(instance2.testFn).toBeCalledTimes(1) + expect(MockedE.prototype.testFn).toBeCalledTimes(3) + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + [ + "c", + ], + ] + `) + expect(vi.mocked(instance2.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "b", + ], + ] + `) + expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + [ + "b", + ], + [ + "c", + ], + ] + `) }) From 9171b4e84d811cd6260acfacaa9141a46b61e9d4 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 21 Nov 2023 19:51:35 +0900 Subject: [PATCH 07/15] chore: rename --- test/core/test/{mocked-wip.test.ts => mocked-class.test.ts} | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) rename test/core/test/{mocked-wip.test.ts => mocked-class.test.ts} (95%) diff --git a/test/core/test/mocked-wip.test.ts b/test/core/test/mocked-class.test.ts similarity index 95% rename from test/core/test/mocked-wip.test.ts rename to test/core/test/mocked-class.test.ts index ba7ceef7803a..87a26ae569ff 100644 --- a/test/core/test/mocked-wip.test.ts +++ b/test/core/test/mocked-class.test.ts @@ -4,9 +4,7 @@ import { MockedE } from '../src/mockedE' vi.mock('../src/mockedE') -// TODO: move to mocked.test.ts? - -test('mock each instance method separately', () => { +test(`each instance's method of mocked class should have independent mock function state`, () => { expect(MockedE).toBeTypeOf('function') const instance1 = new MockedE() From ffd7c53afae4257de4b0f3e4e3faedbbec60cce1 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 22 Nov 2023 14:01:26 +0900 Subject: [PATCH 08/15] chore: minor --- test/core/test/mocked-class.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/core/test/mocked-class.test.ts b/test/core/test/mocked-class.test.ts index 87a26ae569ff..93cf430a6aa1 100644 --- a/test/core/test/mocked-class.test.ts +++ b/test/core/test/mocked-class.test.ts @@ -4,9 +4,7 @@ import { MockedE } from '../src/mockedE' vi.mock('../src/mockedE') -test(`each instance's method of mocked class should have independent mock function state`, () => { - expect(MockedE).toBeTypeOf('function') - +test(`each instance's methods of mocked class should have independent mock function state`, () => { const instance1 = new MockedE() const instance2 = new MockedE() expect(instance1).not.toBe(instance2) From 3936c76093fcfd95eafdfc4f9a6a78baad9e48af Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 22 Nov 2023 14:32:04 +0900 Subject: [PATCH 09/15] fix: support symbol key method --- packages/vitest/src/runtime/mocker.ts | 20 +++++++++++++------- test/core/src/mockedE.ts | 6 ++++++ test/core/test/mocked-class.test.ts | 27 ++++++++++++++++++++++++++- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/packages/vitest/src/runtime/mocker.ts b/packages/vitest/src/runtime/mocker.ts index b3543c731719..011436ea2157 100644 --- a/packages/vitest/src/runtime/mocker.ts +++ b/packages/vitest/src/runtime/mocker.ts @@ -328,20 +328,26 @@ export class VitestMocker { const spyModule = this.spyModule if (!spyModule) throw this.createError('[vitest] `spyModule` is not defined. This is Vitest error. Please open a new issue with reproduction.') - const mock = spyModule.spyOn(newContainer, property).mockImplementation(function (this: any) { + + const primitives = this.primitives + const mock = spyModule.spyOn(newContainer, property).mockImplementation(function (this: unknown) { // jest reference // https://github.com/jestjs/jest/blob/2c3d2409879952157433de215ae0eee5188a4384/packages/jest-mock/src/index.ts#L678-L691 // check constructor call if (this instanceof newContainer[property]) { + const instance = this as any // mock each class instance's method - // TODO: more sound way to loop prorotype methods? - for (const key in this) { - if (typeof this[key] === 'function') { + for (const { key } of getAllMockableProperties(instance, false, primitives)) { + const value = instance[key] + const type = getType(value) + const isFunction = type.includes('Function') && typeof value === 'function' + if (isFunction) { // TODO: ability to restore? - // mock by delegating calls to original prototype method, which should be also mocked already - const original = this[key] - const _mockInner = spyModule.spyOn(this, key).mockImplementation((...args: any[]) => original.apply(this, args)) + // mock and delegate calls to original prototype method, which should be also mocked already + const original = instance[key] + // TODO: fix type error for symbol key? + spyModule.spyOn(instance, key as string).mockImplementation((...args: any[]) => original.apply(this, args)) } } } diff --git a/test/core/src/mockedE.ts b/test/core/src/mockedE.ts index 607a90bdc1b3..99890956d185 100644 --- a/test/core/src/mockedE.ts +++ b/test/core/src/mockedE.ts @@ -1,5 +1,11 @@ +export const symbolFn = Symbol.for('symbolFn') + export class MockedE { public testFn(arg: string) { return arg.repeat(2) } + + public [symbolFn](arg: string) { + return arg.repeat(2) + } } diff --git a/test/core/test/mocked-class.test.ts b/test/core/test/mocked-class.test.ts index 93cf430a6aa1..719ea7c52095 100644 --- a/test/core/test/mocked-class.test.ts +++ b/test/core/test/mocked-class.test.ts @@ -1,6 +1,6 @@ import { expect, test, vi } from 'vitest' -import { MockedE } from '../src/mockedE' +import { MockedE, symbolFn } from '../src/mockedE' vi.mock('../src/mockedE') @@ -87,4 +87,29 @@ test(`each instance's methods of mocked class should have independent mock funct ], ] `) + + // test same things for symbol key method + expect(instance1[symbolFn]).not.toBe(instance2[symbolFn]) + expect(instance1[symbolFn]).not.toBe(MockedE.prototype[symbolFn]) + expect(vi.mocked(instance1[symbolFn]).mock).not.toBe(vi.mocked(instance2[symbolFn]).mock) + + instance1[symbolFn]('d') + expect(instance1[symbolFn]).toBeCalledTimes(1) + expect(instance2[symbolFn]).toBeCalledTimes(0) + expect(MockedE.prototype[symbolFn]).toBeCalledTimes(1) + expect(vi.mocked(instance1[symbolFn]).mock.calls).toMatchInlineSnapshot(` + [ + [ + "d", + ], + ] + `) + expect(vi.mocked(instance2[symbolFn]).mock.calls).toMatchInlineSnapshot(`[]`) + expect(vi.mocked(MockedE.prototype[symbolFn]).mock.calls).toMatchInlineSnapshot(` + [ + [ + "d", + ], + ] + `) }) From 943d2acdad8174ffb6724e03aa136b70e56878fc Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 22 Nov 2023 14:35:00 +0900 Subject: [PATCH 10/15] refactor: minor --- packages/vitest/src/runtime/mocker.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/vitest/src/runtime/mocker.ts b/packages/vitest/src/runtime/mocker.ts index 011436ea2157..a320ff70c60f 100644 --- a/packages/vitest/src/runtime/mocker.ts +++ b/packages/vitest/src/runtime/mocker.ts @@ -330,24 +330,23 @@ export class VitestMocker { throw this.createError('[vitest] `spyModule` is not defined. This is Vitest error. Please open a new issue with reproduction.') const primitives = this.primitives - const mock = spyModule.spyOn(newContainer, property).mockImplementation(function (this: unknown) { + const mock = spyModule.spyOn(newContainer, property).mockImplementation(function (this: any) { // jest reference // https://github.com/jestjs/jest/blob/2c3d2409879952157433de215ae0eee5188a4384/packages/jest-mock/src/index.ts#L678-L691 // check constructor call if (this instanceof newContainer[property]) { - const instance = this as any - // mock each class instance's method - for (const { key } of getAllMockableProperties(instance, false, primitives)) { - const value = instance[key] + // mock each mothod of mocked class instance + for (const { key } of getAllMockableProperties(this, false, primitives)) { + const value = this[key] const type = getType(value) const isFunction = type.includes('Function') && typeof value === 'function' if (isFunction) { // TODO: ability to restore? // mock and delegate calls to original prototype method, which should be also mocked already - const original = instance[key] + const original = this[key] // TODO: fix type error for symbol key? - spyModule.spyOn(instance, key as string).mockImplementation((...args: any[]) => original.apply(this, args)) + spyModule.spyOn(this, key as string).mockImplementation((...args: any[]) => original.apply(this, args)) } } } From aae45a7747cb31c51f3b978544e34ca5aa7a57d7 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 28 Nov 2023 08:10:11 +0900 Subject: [PATCH 11/15] chore: comment --- packages/vitest/src/runtime/mocker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vitest/src/runtime/mocker.ts b/packages/vitest/src/runtime/mocker.ts index a320ff70c60f..2c01a2ac5da2 100644 --- a/packages/vitest/src/runtime/mocker.ts +++ b/packages/vitest/src/runtime/mocker.ts @@ -345,7 +345,7 @@ export class VitestMocker { // TODO: ability to restore? // mock and delegate calls to original prototype method, which should be also mocked already const original = this[key] - // TODO: fix type error for symbol key? + // mocking "symbol" method works but uses "string" cast to silence type-error spyModule.spyOn(this, key as string).mockImplementation((...args: any[]) => original.apply(this, args)) } } From 5b60bbccea4b726ccb393f3732c0db9925678015 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Tue, 28 Nov 2023 09:03:24 +0900 Subject: [PATCH 12/15] test: test mockRestore --- packages/vitest/src/runtime/mocker.ts | 3 +- test/core/test/mocked-class-restore.test.ts | 68 +++++++++++++++++++++ test/core/test/mocked-class.test.ts | 8 +-- 3 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 test/core/test/mocked-class-restore.test.ts diff --git a/packages/vitest/src/runtime/mocker.ts b/packages/vitest/src/runtime/mocker.ts index 2c01a2ac5da2..87a715d58912 100644 --- a/packages/vitest/src/runtime/mocker.ts +++ b/packages/vitest/src/runtime/mocker.ts @@ -342,10 +342,9 @@ export class VitestMocker { const type = getType(value) const isFunction = type.includes('Function') && typeof value === 'function' if (isFunction) { - // TODO: ability to restore? // mock and delegate calls to original prototype method, which should be also mocked already const original = this[key] - // mocking "symbol" method works but uses "string" cast to silence type-error + // mocking "symbol" method works but casts to "string" to silence type-error spyModule.spyOn(this, key as string).mockImplementation((...args: any[]) => original.apply(this, args)) } } diff --git a/test/core/test/mocked-class-restore.test.ts b/test/core/test/mocked-class-restore.test.ts new file mode 100644 index 000000000000..24c7e189d38c --- /dev/null +++ b/test/core/test/mocked-class-restore.test.ts @@ -0,0 +1,68 @@ +import { expect, test, vi } from 'vitest' + +import { MockedE } from '../src/mockedE' + +vi.mock('../src/mockedE') + +// this behavior looks odd but jest also doesn't seem to support this use case properly +test(`mocked class method not restorable`, () => { + const instance1 = new MockedE() + + expect(instance1.testFn('a')).toMatchInlineSnapshot(`undefined`) + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + ] + `) + expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + ] + `) + + // restoring instance method + vi.mocked(instance1.testFn).mockRestore() + expect(instance1.testFn('b')).toMatchInlineSnapshot(`undefined`) + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + [ + "b", + ], + ] + `) + expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + [ + "b", + ], + ] + `) + + // restoring prototype doesn't restore instance + vi.mocked(MockedE.prototype.testFn).mockRestore() + expect(instance1.testFn('c')).toMatchInlineSnapshot(`undefined`) + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "c", + ], + ] + `) + expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "c", + ], + ] + `) +}) diff --git a/test/core/test/mocked-class.test.ts b/test/core/test/mocked-class.test.ts index 719ea7c52095..0eaa6b398791 100644 --- a/test/core/test/mocked-class.test.ts +++ b/test/core/test/mocked-class.test.ts @@ -12,7 +12,7 @@ test(`each instance's methods of mocked class should have independent mock funct expect(instance1.testFn).not.toBe(MockedE.prototype.testFn) expect(vi.mocked(instance1.testFn).mock).not.toBe(vi.mocked(instance2.testFn).mock) - instance1.testFn('a') + expect(instance1.testFn('a')).toMatchInlineSnapshot(`undefined`) expect(instance1.testFn).toBeCalledTimes(1) expect(instance2.testFn).toBeCalledTimes(0) expect(MockedE.prototype.testFn).toBeCalledTimes(1) @@ -31,7 +31,7 @@ test(`each instance's methods of mocked class should have independent mock funct ] `) - instance2.testFn('b') + expect(instance2.testFn('b')).toMatchInlineSnapshot(`undefined`) expect(instance1.testFn).toBeCalledTimes(1) expect(instance2.testFn).toBeCalledTimes(1) expect(MockedE.prototype.testFn).toBeCalledTimes(2) @@ -53,7 +53,7 @@ test(`each instance's methods of mocked class should have independent mock funct ] `) - instance1.testFn('c') + expect(instance1.testFn('c')).toMatchInlineSnapshot(`undefined`) expect(instance1.testFn).toBeCalledTimes(2) expect(instance2.testFn).toBeCalledTimes(1) expect(MockedE.prototype.testFn).toBeCalledTimes(3) @@ -93,7 +93,7 @@ test(`each instance's methods of mocked class should have independent mock funct expect(instance1[symbolFn]).not.toBe(MockedE.prototype[symbolFn]) expect(vi.mocked(instance1[symbolFn]).mock).not.toBe(vi.mocked(instance2[symbolFn]).mock) - instance1[symbolFn]('d') + expect(instance1[symbolFn]('d')).toMatchInlineSnapshot(`undefined`) expect(instance1[symbolFn]).toBeCalledTimes(1) expect(instance2[symbolFn]).toBeCalledTimes(0) expect(MockedE.prototype[symbolFn]).toBeCalledTimes(1) From 736b61cde36a3db008c721138111c68aaddac992 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 29 Nov 2023 08:42:12 +0900 Subject: [PATCH 13/15] test: more check --- test/core/test/mocked-class-restore.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/core/test/mocked-class-restore.test.ts b/test/core/test/mocked-class-restore.test.ts index 24c7e189d38c..f2af585391e4 100644 --- a/test/core/test/mocked-class-restore.test.ts +++ b/test/core/test/mocked-class-restore.test.ts @@ -26,6 +26,21 @@ test(`mocked class method not restorable`, () => { // restoring instance method vi.mocked(instance1.testFn).mockRestore() + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + ] + `) + expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + ] + `) + expect(instance1.testFn('b')).toMatchInlineSnapshot(`undefined`) expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` [ @@ -50,6 +65,9 @@ test(`mocked class method not restorable`, () => { // restoring prototype doesn't restore instance vi.mocked(MockedE.prototype.testFn).mockRestore() + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(`[]`) + expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(`[]`) + expect(instance1.testFn('c')).toMatchInlineSnapshot(`undefined`) expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` [ From 24d5acc77a49c9e5aece3ae083a1b36861be21d3 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 29 Nov 2023 10:00:27 +0900 Subject: [PATCH 14/15] test: restoreAllMocks for mocked class methods (wip) --- .../test/mocked-class-restore-all.test.ts | 74 +++++++++++++++++++ ... => mocked-class-restore-explicit.test.ts} | 2 +- 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 test/core/test/mocked-class-restore-all.test.ts rename test/core/test/{mocked-class-restore.test.ts => mocked-class-restore-explicit.test.ts} (95%) diff --git a/test/core/test/mocked-class-restore-all.test.ts b/test/core/test/mocked-class-restore-all.test.ts new file mode 100644 index 000000000000..1eadcfc5bc10 --- /dev/null +++ b/test/core/test/mocked-class-restore-all.test.ts @@ -0,0 +1,74 @@ +import { expect, test, vi } from 'vitest' + +import { MockedE } from '../src/mockedE' + +vi.mock('../src/mockedE') + +test(`mocked class are not affected by restoreAllMocks`, () => { + const instance1 = new MockedE() + expect(instance1.testFn('a')).toMatchInlineSnapshot(`undefined`) + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + ] + `) + expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "a", + ], + ] + `) + + vi.restoreAllMocks() + expect(instance1.testFn('b')).toMatchInlineSnapshot(`undefined`) + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "b", + ], + ] + `) + expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "b", + ], + ] + `) + + const instance2 = new MockedE() + expect(instance2.testFn('c')).toMatchInlineSnapshot(`undefined`) + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "b", + ], + [ + "c", + ], + ] + `) + expect(vi.mocked(instance2.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "b", + ], + [ + "c", + ], + ] + `) + expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "b", + ], + [ + "c", + ], + ] + `) +}) diff --git a/test/core/test/mocked-class-restore.test.ts b/test/core/test/mocked-class-restore-explicit.test.ts similarity index 95% rename from test/core/test/mocked-class-restore.test.ts rename to test/core/test/mocked-class-restore-explicit.test.ts index f2af585391e4..96a155d804c5 100644 --- a/test/core/test/mocked-class-restore.test.ts +++ b/test/core/test/mocked-class-restore-explicit.test.ts @@ -5,7 +5,7 @@ import { MockedE } from '../src/mockedE' vi.mock('../src/mockedE') // this behavior looks odd but jest also doesn't seem to support this use case properly -test(`mocked class method not restorable`, () => { +test(`mocked class methods are not restorable by explicit mockRestore calls`, () => { const instance1 = new MockedE() expect(instance1.testFn('a')).toMatchInlineSnapshot(`undefined`) From a5c3a06ae77210fbbea6793a0df6833211e030c9 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 29 Nov 2023 10:27:32 +0900 Subject: [PATCH 15/15] fix: handle vi.restoreAllMocks --- packages/vitest/src/runtime/mocker.ts | 28 ++++++++-------- .../test/mocked-class-restore-all.test.ts | 32 +++++++++++-------- .../mocked-class-restore-explicit.test.ts | 24 +++++++------- 3 files changed, 45 insertions(+), 39 deletions(-) diff --git a/packages/vitest/src/runtime/mocker.ts b/packages/vitest/src/runtime/mocker.ts index 87a715d58912..ff30d2de5833 100644 --- a/packages/vitest/src/runtime/mocker.ts +++ b/packages/vitest/src/runtime/mocker.ts @@ -325,18 +325,15 @@ export class VitestMocker { continue if (isFunction) { - const spyModule = this.spyModule - if (!spyModule) + if (!this.spyModule) throw this.createError('[vitest] `spyModule` is not defined. This is Vitest error. Please open a new issue with reproduction.') - + const spyModule = this.spyModule const primitives = this.primitives - const mock = spyModule.spyOn(newContainer, property).mockImplementation(function (this: any) { - // jest reference - // https://github.com/jestjs/jest/blob/2c3d2409879952157433de215ae0eee5188a4384/packages/jest-mock/src/index.ts#L678-L691 - - // check constructor call + function mockFunction(this: any) { + // detect constructor call and mock each instance's methods + // so that mock states between prototype/instances don't affect each other + // (jest reference https://github.com/jestjs/jest/blob/2c3d2409879952157433de215ae0eee5188a4384/packages/jest-mock/src/index.ts#L678-L691) if (this instanceof newContainer[property]) { - // mock each mothod of mocked class instance for (const { key } of getAllMockableProperties(this, false, primitives)) { const value = this[key] const type = getType(value) @@ -344,15 +341,20 @@ export class VitestMocker { if (isFunction) { // mock and delegate calls to original prototype method, which should be also mocked already const original = this[key] - // mocking "symbol" method works but casts to "string" to silence type-error - spyModule.spyOn(this, key as string).mockImplementation((...args: any[]) => original.apply(this, args)) + const mock = spyModule.spyOn(this, key as string).mockImplementation(original) + mock.mockRestore = () => { + mock.mockReset() + mock.mockImplementation(original) + return mock + } } } } - }) + } + const mock = spyModule.spyOn(newContainer, property).mockImplementation(mockFunction) mock.mockRestore = () => { mock.mockReset() - mock.mockImplementation(() => undefined) + mock.mockImplementation(mockFunction) return mock } // tinyspy retains length, but jest doesn't. diff --git a/test/core/test/mocked-class-restore-all.test.ts b/test/core/test/mocked-class-restore-all.test.ts index 1eadcfc5bc10..09bb1a83eb01 100644 --- a/test/core/test/mocked-class-restore-all.test.ts +++ b/test/core/test/mocked-class-restore-all.test.ts @@ -5,9 +5,9 @@ import { MockedE } from '../src/mockedE' vi.mock('../src/mockedE') test(`mocked class are not affected by restoreAllMocks`, () => { - const instance1 = new MockedE() - expect(instance1.testFn('a')).toMatchInlineSnapshot(`undefined`) - expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + const instance0 = new MockedE() + expect(instance0.testFn('a')).toMatchInlineSnapshot(`undefined`) + expect(vi.mocked(instance0.testFn).mock.calls).toMatchInlineSnapshot(` [ [ "a", @@ -23,8 +23,10 @@ test(`mocked class are not affected by restoreAllMocks`, () => { `) vi.restoreAllMocks() - expect(instance1.testFn('b')).toMatchInlineSnapshot(`undefined`) - expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + + // reset only history after restoreAllMocks + expect(instance0.testFn('b')).toMatchInlineSnapshot(`undefined`) + expect(vi.mocked(instance0.testFn).mock.calls).toMatchInlineSnapshot(` [ [ "b", @@ -39,28 +41,30 @@ test(`mocked class are not affected by restoreAllMocks`, () => { ] `) + // mocked constructor is still effective after restoreAllMocks + const instance1 = new MockedE() const instance2 = new MockedE() - expect(instance2.testFn('c')).toMatchInlineSnapshot(`undefined`) - expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + expect(instance1).not.toBe(instance2) + expect(instance1.testFn).not.toBe(instance2.testFn) + expect(instance1.testFn).not.toBe(MockedE.prototype.testFn) + expect(vi.mocked(instance1.testFn).mock).not.toBe(vi.mocked(instance2.testFn).mock) + + expect(instance1.testFn('c')).toMatchInlineSnapshot(`undefined`) + expect(vi.mocked(instance0.testFn).mock.calls).toMatchInlineSnapshot(` [ [ "b", ], - [ - "c", - ], ] `) - expect(vi.mocked(instance2.testFn).mock.calls).toMatchInlineSnapshot(` + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` [ - [ - "b", - ], [ "c", ], ] `) + expect(vi.mocked(instance2.testFn).mock.calls).toMatchInlineSnapshot(`[]`) expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` [ [ diff --git a/test/core/test/mocked-class-restore-explicit.test.ts b/test/core/test/mocked-class-restore-explicit.test.ts index 96a155d804c5..fc9d7d4e6a46 100644 --- a/test/core/test/mocked-class-restore-explicit.test.ts +++ b/test/core/test/mocked-class-restore-explicit.test.ts @@ -26,13 +26,7 @@ test(`mocked class methods are not restorable by explicit mockRestore calls`, () // restoring instance method vi.mocked(instance1.testFn).mockRestore() - expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` - [ - [ - "a", - ], - ] - `) + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(`[]`) expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(` [ [ @@ -44,9 +38,6 @@ test(`mocked class methods are not restorable by explicit mockRestore calls`, () expect(instance1.testFn('b')).toMatchInlineSnapshot(`undefined`) expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` [ - [ - "a", - ], [ "b", ], @@ -63,14 +54,23 @@ test(`mocked class methods are not restorable by explicit mockRestore calls`, () ] `) - // restoring prototype doesn't restore instance + // restoring prototype method vi.mocked(MockedE.prototype.testFn).mockRestore() - expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(`[]`) + expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` + [ + [ + "b", + ], + ] + `) expect(vi.mocked(MockedE.prototype.testFn).mock.calls).toMatchInlineSnapshot(`[]`) expect(instance1.testFn('c')).toMatchInlineSnapshot(`undefined`) expect(vi.mocked(instance1.testFn).mock.calls).toMatchInlineSnapshot(` [ + [ + "b", + ], [ "c", ],