From 4a8f06c7879c524ef86f592967b3fd2c64f16948 Mon Sep 17 00:00:00 2001 From: James Keesey Date: Fri, 19 Jun 2020 16:05:55 -0700 Subject: [PATCH 1/3] Exclude .then() and .catch() from automatic handling. By default, all symbols except .hasOwnProperty() are stubbed even if they are not assigned a value. This is to be able to track references to all invocations. However, the way that Promise.resolve() works, it checks for the presence of a .then() property and if it is there assumes that the resolve value is a promise so it tries to get its resolved value. But this is a mock reference so the stub does not return a resolved value so the promise never resolves. This fix excludes .then() and .catch() from automatically creating a stub so that Promise will not try to resolve it, but the user can still override the call with a when() clause. This fix also adds "Symbol(Symbol.toPrimitive)" to the exclude list so that by default toString() on a mock will return something other than null. --- src/Mock.ts | 7 +++++ test/mocking.types.spec.ts | 56 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/src/Mock.ts b/src/Mock.ts index 3c98f72..db5d3fb 100644 --- a/src/Mock.ts +++ b/src/Mock.ts @@ -18,6 +18,7 @@ export class Mocker { private mockableFunctionsFinder = new MockableFunctionsFinder(); private objectPropertyCodeRetriever = new ObjectPropertyCodeRetriever(); private excludedPropertyNames: string[] = ["hasOwnProperty"]; + private defaultedPropertyNames: string[] = ["Symbol(Symbol.toPrimitive)", "then", "catch"]; constructor(private clazz: any, public instance: any = {}) { this.mock.__tsmockitoInstance = this.instance; @@ -39,6 +40,9 @@ export class Mocker { const hasMethodStub = name in target; if (!hasMethodStub) { + if (this.defaultedPropertyNames.indexOf(name.toString()) >= 0) { + return undefined; + } return this.createActionListener(name.toString()); } return target[name]; @@ -64,6 +68,9 @@ export class Mocker { const hasMethodStub = name in target; if (!hasMethodStub) { + if (this.defaultedPropertyNames.indexOf(name.toString()) >= 0) { + return undefined; + } this.createMethodStub(name.toString()); this.createInstanceActionListener(name.toString(), {}); } diff --git a/test/mocking.types.spec.ts b/test/mocking.types.spec.ts index ecae9c5..ba4c90b 100644 --- a/test/mocking.types.spec.ts +++ b/test/mocking.types.spec.ts @@ -62,6 +62,52 @@ describe("mocking", () => { // then expect(foo.sampleString).toBe("42"); }); + + it("does not create then() descriptor", () => { + // given + mockedFoo = mock(SampleAbstractClass); + const woof: any = instance(mockedFoo); + + // when + + // then + expect(woof.then == null).toBe(true); + }); + + it("does create then() descriptor", () => { + // given + const mockedThenable = mock(SampleThenable); + const thenable = instance(mockedThenable); + + // when + when(mockedThenable.then()).thenReturn("42"); + + // then + expect(thenable.then()).toEqual("42"); + }); + + it("does not create catch() descriptor", () => { + // given + mockedFoo = mock(SampleAbstractClass); + const woof: any = instance(mockedFoo); + + // when + + // then + expect(woof.catch == null).toBe(true); + }); + + it("does create catch() descriptor", () => { + // given + const mockedThenable = mock(SampleThenable); + const thenable = instance(mockedThenable); + + // when + when(mockedThenable.catch()).thenReturn("42"); + + // then + expect(thenable.catch()).toEqual("42"); + }); }); describe("mocking class with hasOwnProperty", () => { @@ -231,3 +277,13 @@ class SampleGeneric { return null; } } + +abstract class SampleThenable { + public then(): string { + return "bob"; + } + + public catch(): string { + return "bob"; + } +} From 4d3ce94975001fe52e8f19da3c14afb9a853b71f Mon Sep 17 00:00:00 2001 From: James Keesey Date: Fri, 19 Jun 2020 16:47:51 -0700 Subject: [PATCH 2/3] Added test for toString() conversion Added test to verify that adding "Symbol(Symbol.toPrimitive)" to the exclude list will allow toString() conversion to work properly. --- test/mocking.types.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/mocking.types.spec.ts b/test/mocking.types.spec.ts index ba4c90b..184ca38 100644 --- a/test/mocking.types.spec.ts +++ b/test/mocking.types.spec.ts @@ -108,6 +108,18 @@ describe("mocking", () => { // then expect(thenable.catch()).toEqual("42"); }); + + it("formats as [object Object]", () => { + // given + const mockedThenable = mock(SampleThenable); + const thenable = instance(mockedThenable); + + // when + + // then + const str = `"${thenable}"`; + expect(str).toEqual('"[object Object]"'); + }); }); describe("mocking class with hasOwnProperty", () => { From dc19b3ae0879244e640d1a3e45fe11aa82c3e8ef Mon Sep 17 00:00:00 2001 From: James Keesey Date: Wed, 24 Jun 2020 15:51:35 -0700 Subject: [PATCH 3/3] Moved code to correct location In the prior commit, the location of one of the new tests was in the wrong location. This prevented the code from working and the coverage went down. Moved the test to the correct location and everything passes and the coverage went up-- at least on my computer. --- src/Mock.ts | 6 ++-- test/interface.spec.ts | 71 +++++++++++++++++++++++++++++++------- test/mocking.types.spec.ts | 64 +++++++++++++++------------------- test/utils/Thenable.ts | 15 ++++++++ 4 files changed, 105 insertions(+), 51 deletions(-) create mode 100644 test/utils/Thenable.ts diff --git a/src/Mock.ts b/src/Mock.ts index db5d3fb..8d6c500 100644 --- a/src/Mock.ts +++ b/src/Mock.ts @@ -68,9 +68,6 @@ export class Mocker { const hasMethodStub = name in target; if (!hasMethodStub) { - if (this.defaultedPropertyNames.indexOf(name.toString()) >= 0) { - return undefined; - } this.createMethodStub(name.toString()); this.createInstanceActionListener(name.toString(), {}); } @@ -84,6 +81,9 @@ export class Mocker { get: (target: any, name: PropertyKey) => { const hasMethodStub = name in target; if (!hasMethodStub) { + if (this.defaultedPropertyNames.indexOf(name.toString()) >= 0) { + return undefined; + } this.createPropertyStub(name.toString()); this.createInstancePropertyDescriptorListener(name.toString(), {}, this.clazz.prototype); } diff --git a/test/interface.spec.ts b/test/interface.spec.ts index b1e4a3f..e5fdb67 100644 --- a/test/interface.spec.ts +++ b/test/interface.spec.ts @@ -1,26 +1,73 @@ import { instance, mock, when } from "../src/ts-mockito"; import { FooInterface } from "./utils/FooInterface"; +import { ThenableInterface } from "./utils/Thenable"; describe("Interface", () => { - let mockedFoo: FooInterface; - let foo: FooInterface; - if (typeof Proxy === "undefined") { pending("Testing browser doesn't support Proxy."); } - beforeEach(() => { - mockedFoo = mock(); - foo = instance(mockedFoo); + describe("Basic", () => { + let mockedFoo: FooInterface; + let foo: FooInterface; + + beforeEach(() => { + mockedFoo = mock(); + foo = instance(mockedFoo); + }); + + it("should mock interface function", () => { + // Rest cases for interfaces are tested in verification.spec.ts + + // when + when(mockedFoo.sumByInterface(2, 3)).thenReturn(1000); + + // then + expect(foo.sumByInterface(2, 3)).toEqual(1000); + }); }); + describe("Promise", () => { + let mockedThen: ThenableInterface; + let inst: ThenableInterface; + + beforeEach(() => { + mockedThen = mock(); + inst = instance(mockedThen); + }); + + it("does not create then descriptor for interface", () => { + // given + + // when + + // then + expect(inst.then).toBeUndefined(); + }); + it("creates then descriptor for interface", () => { + // given + + // when + when(mockedThen.then()).thenReturn("woof"); + + // then + expect(inst.then).toBeDefined(); + }); + it("does not create catch descriptor for interface", () => { + // given + + // when - it("should mock interface function", () => { - // Rest cases for interfaces are tested in verification.spec.ts + // then + expect(inst.catch).toBeUndefined(); + }); + it("creates catch descriptor for interface", () => { + // given - // when - when(mockedFoo.sumByInterface(2, 3)).thenReturn(1000); + // when + when(mockedThen.catch()).thenReturn("woof"); - // then - expect(foo.sumByInterface(2, 3)).toEqual(1000); + // then + expect(inst.catch).toBeDefined(); + }); }); }); diff --git a/test/mocking.types.spec.ts b/test/mocking.types.spec.ts index 184ca38..b7a8c62 100644 --- a/test/mocking.types.spec.ts +++ b/test/mocking.types.spec.ts @@ -1,6 +1,7 @@ -import {MethodToStub} from "../src/MethodToStub"; -import {instance, mock, when} from "../src/ts-mockito"; -import {Bar} from "./utils/Bar"; +import { MethodToStub } from "../src/MethodToStub"; +import { instance, mock, when } from "../src/ts-mockito"; +import { Bar } from "./utils/Bar"; +import { ThenableClass } from "./utils/Thenable"; describe("mocking", () => { describe("mocking abstract class", () => { @@ -62,62 +63,63 @@ describe("mocking", () => { // then expect(foo.sampleString).toBe("42"); }); - - it("does not create then() descriptor", () => { + }); + describe("mocking promise methods", () => { + it("does not create then descriptor for class", () => { // given - mockedFoo = mock(SampleAbstractClass); - const woof: any = instance(mockedFoo); + const mocked = mock(SampleAbstractClass); + const inst = instance(mocked); // when // then - expect(woof.then == null).toBe(true); + expect((inst as any).then).toBeUndefined(); }); - it("does create then() descriptor", () => { + it("does create then descriptor", () => { // given - const mockedThenable = mock(SampleThenable); - const thenable = instance(mockedThenable); + const mocked = mock(ThenableClass); + const inst = instance(mocked); // when - when(mockedThenable.then()).thenReturn("42"); + when(mocked.then()).thenReturn("42"); // then - expect(thenable.then()).toEqual("42"); + expect(inst.then()).toEqual("42"); }); - it("does not create catch() descriptor", () => { + it("does not create catch descriptor", () => { // given - mockedFoo = mock(SampleAbstractClass); - const woof: any = instance(mockedFoo); + const mocked = mock(SampleAbstractClass); + const inst = instance(mocked); // when // then - expect(woof.catch == null).toBe(true); + expect((inst as any).catch).toBeUndefined(); }); - it("does create catch() descriptor", () => { + it("does create catch descriptor", () => { // given - const mockedThenable = mock(SampleThenable); - const thenable = instance(mockedThenable); + const mocked = mock(ThenableClass); + const inst = instance(mocked); // when - when(mockedThenable.catch()).thenReturn("42"); + when(mocked.catch()).thenReturn("42"); // then - expect(thenable.catch()).toEqual("42"); + expect(inst.catch()).toEqual("42"); }); - it("formats as [object Object]", () => { + it("default object formatting works", () => { // given - const mockedThenable = mock(SampleThenable); - const thenable = instance(mockedThenable); + const mocked = mock(ThenableClass); + const inst = instance(mocked); // when // then - const str = `"${thenable}"`; + const str = `"${inst}"`; expect(str).toEqual('"[object Object]"'); }); }); @@ -289,13 +291,3 @@ class SampleGeneric { return null; } } - -abstract class SampleThenable { - public then(): string { - return "bob"; - } - - public catch(): string { - return "bob"; - } -} diff --git a/test/utils/Thenable.ts b/test/utils/Thenable.ts new file mode 100644 index 0000000..7a3ed47 --- /dev/null +++ b/test/utils/Thenable.ts @@ -0,0 +1,15 @@ +export abstract class ThenableClass { + public then(): string { + return "bob"; + } + + public catch(): string { + return "bob"; + } +} + +export interface ThenableInterface { + then(): string; + + catch(): string; +}