From f857d309646154f6ae587ec744195453030d1f7e Mon Sep 17 00:00:00 2001 From: Mark Pedrotti Date: Thu, 25 Jul 2019 18:01:34 -0400 Subject: [PATCH] expect: Throw matcher error when received cannot be jasmine spy (#8747) * expect: Improve report when mock-spy matcher fails, part 5 * Update CHANGELOG.md * Edit CHANGELOG.md --- CHANGELOG.md | 1 + .../__snapshots__/spyMatchers.test.js.snap | 58 ++++++++++++------- .../expect/src/__tests__/spyMatchers.test.js | 49 ++++++++++++++-- packages/expect/src/spyMatchers.ts | 44 ++++++++++---- 4 files changed, 115 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa34e1685ea9..208b90faf636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - `[expect]` Improve report when mock-spy matcher fails, part 2 ([#8649](https://github.com/facebook/jest/pull/8649)) - `[expect]` Improve report when mock-spy matcher fails, part 3 ([#8697](https://github.com/facebook/jest/pull/8697)) - `[expect]` Improve report when mock-spy matcher fails, part 4 ([#8710](https://github.com/facebook/jest/pull/8710)) +- `[expect]` Throw matcher error when received cannot be jasmine spy ([#8747](https://github.com/facebook/jest/pull/8747)) - `[jest-snapshot]` Highlight substring differences when matcher fails, part 3 ([#8569](https://github.com/facebook/jest/pull/8569)) - `[jest-cli]` Improve chai support (with detailed output, to match jest exceptions) ([#8454](https://github.com/facebook/jest/pull/8454)) - `[*]` Manage the global timeout with `--testTimeout` command line argument. ([#8456](https://github.com/facebook/jest/pull/8456)) diff --git a/packages/expect/src/__tests__/__snapshots__/spyMatchers.test.js.snap b/packages/expect/src/__tests__/__snapshots__/spyMatchers.test.js.snap index bf6a4cf1042d..c25156a340cd 100644 --- a/packages/expect/src/__tests__/__snapshots__/spyMatchers.test.js.snap +++ b/packages/expect/src/__tests__/__snapshots__/spyMatchers.test.js.snap @@ -180,7 +180,7 @@ Number of returns: 3" exports[`lastReturnedWith works only on spies or jest.fn 1`] = ` "expect(received).lastReturnedWith(expected) -Matcher error: received value must be a mock or spy function +Matcher error: received value must be a mock function Received has type: function Received has value: [Function fn]" @@ -590,7 +590,7 @@ Number of returns: 3" exports[`nthReturnedWith works only on spies or jest.fn 1`] = ` "expect(received).nthReturnedWith(n, expected) -Matcher error: received value must be a mock or spy function +Matcher error: received value must be a mock function Received has type: function Received has value: [Function fn]" @@ -699,7 +699,7 @@ Expected has value: 555" `; exports[`toBeCalled .not passes when called 1`] = ` -"expect(jest.fn()).toBeCalled() +"expect(spy).toBeCalled() Expected number of calls: >= 1 Received number of calls: 0" @@ -880,7 +880,7 @@ Expected has value: [Function anonymous]" `; exports[`toBeCalledTimes passes if function called equal to expected times 1`] = ` -"expect(jest.fn()).not.toBeCalledTimes(expected) +"expect(spy).not.toBeCalledTimes(expected) Expected number of calls: not 2" `; @@ -1029,7 +1029,7 @@ Expected has value: 555" `; exports[`toHaveBeenCalled .not passes when called 1`] = ` -"expect(jest.fn()).toHaveBeenCalled() +"expect(spy).toHaveBeenCalled() Expected number of calls: >= 1 Received number of calls: 0" @@ -1210,7 +1210,7 @@ Expected has value: [Function anonymous]" `; exports[`toHaveBeenCalledTimes passes if function called equal to expected times 1`] = ` -"expect(jest.fn()).not.toHaveBeenCalledTimes(expected) +"expect(spy).not.toHaveBeenCalledTimes(expected) Expected number of calls: not 2" `; @@ -1689,7 +1689,7 @@ Number of returns: 3" exports[`toHaveLastReturnedWith works only on spies or jest.fn 1`] = ` "expect(received).toHaveLastReturnedWith(expected) -Matcher error: received value must be a mock or spy function +Matcher error: received value must be a mock function Received has type: function Received has value: [Function fn]" @@ -1939,7 +1939,7 @@ Number of returns: 3" exports[`toHaveNthReturnedWith works only on spies or jest.fn 1`] = ` "expect(received).toHaveNthReturnedWith(n, expected) -Matcher error: received value must be a mock or spy function +Matcher error: received value must be a mock function Received has type: function Received has value: [Function fn]" @@ -2073,7 +2073,7 @@ Received number of returns: 0" exports[`toHaveReturned .not works only on jest.fn 1`] = ` "expect(received).not.toHaveReturned() -Matcher error: received value must be a mock or spy function +Matcher error: received value must be a mock function Received has type: function Received has value: [Function fn]" @@ -2135,6 +2135,15 @@ Received number of returns: 1 1: undefined" `; +exports[`toHaveReturned throw matcher error if received is spy 1`] = ` +"expect(received).toHaveReturned() + +Matcher error: received value must be a mock function + +Received has type: function +Received has value: [Function spy]" +`; + exports[`toHaveReturnedTimes .not only accepts a number argument 1`] = ` "expect(received).not.toHaveReturnedTimes(expected) @@ -2300,13 +2309,13 @@ exports[`toHaveReturnedTimes passes if function returned equal to expected times Expected number of returns: not 2" `; -exports[`toHaveReturnedTimes works only on spies or jest.fn 1`] = ` -"expect(received).toHaveReturnedTimes(expected) +exports[`toHaveReturnedTimes throw matcher error if received is spy 1`] = ` +"expect(received).not.toHaveReturnedTimes(expected) -Matcher error: received value must be a mock or spy function +Matcher error: received value must be a mock function Received has type: function -Received has value: [Function fn]" +Received has value: [Function spy]" `; exports[`toHaveReturnedWith a call that throws is not considered to have returned 1`] = ` @@ -2365,7 +2374,7 @@ Number of returns: 6" exports[`toHaveReturnedWith works only on spies or jest.fn 1`] = ` "expect(received).toHaveReturnedWith(expected) -Matcher error: received value must be a mock or spy function +Matcher error: received value must be a mock function Received has type: function Received has value: [Function fn]" @@ -2489,7 +2498,7 @@ Received number of returns: 0" exports[`toReturn .not works only on jest.fn 1`] = ` "expect(received).not.toReturn() -Matcher error: received value must be a mock or spy function +Matcher error: received value must be a mock function Received has type: function Received has value: [Function fn]" @@ -2551,6 +2560,15 @@ Received number of returns: 1 1: undefined" `; +exports[`toReturn throw matcher error if received is spy 1`] = ` +"expect(received).toReturn() + +Matcher error: received value must be a mock function + +Received has type: function +Received has value: [Function spy]" +`; + exports[`toReturnTimes .not only accepts a number argument 1`] = ` "expect(received).not.toReturnTimes(expected) @@ -2716,13 +2734,13 @@ exports[`toReturnTimes passes if function returned equal to expected times 1`] = Expected number of returns: not 2" `; -exports[`toReturnTimes works only on spies or jest.fn 1`] = ` -"expect(received).toReturnTimes(expected) +exports[`toReturnTimes throw matcher error if received is spy 1`] = ` +"expect(received).not.toReturnTimes(expected) -Matcher error: received value must be a mock or spy function +Matcher error: received value must be a mock function Received has type: function -Received has value: [Function fn]" +Received has value: [Function spy]" `; exports[`toReturnWith a call that throws is not considered to have returned 1`] = ` @@ -2781,7 +2799,7 @@ Number of returns: 6" exports[`toReturnWith works only on spies or jest.fn 1`] = ` "expect(received).toReturnWith(expected) -Matcher error: received value must be a mock or spy function +Matcher error: received value must be a mock function Received has type: function Received has value: [Function fn]" diff --git a/packages/expect/src/__tests__/spyMatchers.test.js b/packages/expect/src/__tests__/spyMatchers.test.js index 1939557259f5..c6ce6366a124 100644 --- a/packages/expect/src/__tests__/spyMatchers.test.js +++ b/packages/expect/src/__tests__/spyMatchers.test.js @@ -8,6 +8,22 @@ const Immutable = require('immutable'); const jestExpect = require('../'); +// Given a Jest mock function, return a minimal mock of a Jasmine spy. +const createSpy = fn => { + const spy = function() {}; + + spy.calls = { + all() { + return fn.mock.calls.map(args => ({args})); + }, + count() { + return fn.mock.calls.length; + }, + }; + + return spy; +}; + ['toBeCalled', 'toHaveBeenCalled'].forEach(called => { describe(`${called}`, () => { test(`works only on spies or jest.fn`, () => { @@ -19,15 +35,18 @@ const jestExpect = require('../'); test(`passes when called`, () => { const fn = jest.fn(); fn('arg0', 'arg1', 'arg2'); + jestExpect(createSpy(fn))[called](); jestExpect(fn)[called](); expect(() => jestExpect(fn).not[called]()).toThrowErrorMatchingSnapshot(); }); test(`.not passes when called`, () => { const fn = jest.fn(); + const spy = createSpy(fn); + jestExpect(spy).not[called](); jestExpect(fn).not[called](); - expect(() => jestExpect(fn)[called]()).toThrowErrorMatchingSnapshot(); + expect(() => jestExpect(spy)[called]()).toThrowErrorMatchingSnapshot(); }); test(`fails with any argument passed`, () => { @@ -93,10 +112,12 @@ const jestExpect = require('../'); fn(); fn(); + const spy = createSpy(fn); + jestExpect(spy)[calledTimes](2); jestExpect(fn)[calledTimes](2); expect(() => - jestExpect(fn).not[calledTimes](2), + jestExpect(spy).not[calledTimes](2), ).toThrowErrorMatchingSnapshot(); }); @@ -106,6 +127,10 @@ const jestExpect = require('../'); fn(); fn(); + const spy = createSpy(fn); + jestExpect(spy)[calledTimes](3); + jestExpect(spy).not[calledTimes](2); + jestExpect(fn)[calledTimes](3); jestExpect(fn).not[calledTimes](2); @@ -118,6 +143,10 @@ const jestExpect = require('../'); const fn = jest.fn(); fn(); + const spy = createSpy(fn); + jestExpect(spy)[calledTimes](1); + jestExpect(spy).not[calledTimes](2); + jestExpect(fn)[calledTimes](1); jestExpect(fn).not[calledTimes](2); @@ -164,6 +193,7 @@ const jestExpect = require('../'); test(`works when not called`, () => { const fn = jest.fn(); + caller(jestExpect(createSpy(fn)).not[calledWith], 'foo', 'bar'); caller(jestExpect(fn).not[calledWith], 'foo', 'bar'); expect(() => @@ -174,6 +204,7 @@ const jestExpect = require('../'); test(`works with no arguments`, () => { const fn = jest.fn(); fn(); + caller(jestExpect(createSpy(fn))[calledWith]); caller(jestExpect(fn)[calledWith]); }); @@ -181,6 +212,7 @@ const jestExpect = require('../'); const fn = jest.fn(); fn('foo', 'bar1'); + caller(jestExpect(createSpy(fn)).not[calledWith], 'foo', 'bar'); caller(jestExpect(fn).not[calledWith], 'foo', 'bar'); expect(() => @@ -192,6 +224,7 @@ const jestExpect = require('../'); const fn = jest.fn(); fn('foo', 'bar'); + caller(jestExpect(createSpy(fn))[calledWith], 'foo', 'bar'); caller(jestExpect(fn)[calledWith], 'foo', 'bar'); expect(() => @@ -389,6 +422,12 @@ const jestExpect = require('../'); ).toThrowErrorMatchingSnapshot(); }); + test(`throw matcher error if received is spy`, () => { + const spy = createSpy(jest.fn()); + + expect(() => jestExpect(spy)[returned]()).toThrowErrorMatchingSnapshot(); + }); + test(`passes when returned`, () => { const fn = jest.fn(() => 42); fn(); @@ -525,11 +564,11 @@ const jestExpect = require('../'); ['toReturnTimes', 'toHaveReturnedTimes'].forEach(returnedTimes => { describe(`${returnedTimes}`, () => { - test('works only on spies or jest.fn', () => { - const fn = function fn() {}; + test('throw matcher error if received is spy', () => { + const spy = createSpy(jest.fn()); expect(() => - jestExpect(fn)[returnedTimes](2), + jestExpect(spy).not[returnedTimes](2), ).toThrowErrorMatchingSnapshot(); }); diff --git a/packages/expect/src/spyMatchers.ts b/packages/expect/src/spyMatchers.ts index 94079de8a117..3e6360332173 100644 --- a/packages/expect/src/spyMatchers.ts +++ b/packages/expect/src/spyMatchers.ts @@ -121,7 +121,7 @@ const createToBeCalledMatcher = (matcherName: string) => promise: this.promise, }; ensureNoExpected(expected, matcherName, options); - ensureMock(received, matcherName, expectedArgument, options); + ensureMockOrSpy(received, matcherName, expectedArgument, options); const receivedIsSpy = isSpy(received); const receivedName = receivedIsSpy ? 'spy' : received.getMockName(); @@ -226,7 +226,7 @@ const createToBeCalledTimesMatcher = (matcherName: string) => promise: this.promise, }; ensureExpectedIsNumber(expected, matcherName, options); - ensureMock(received, matcherName, expectedArgument, options); + ensureMockOrSpy(received, matcherName, expectedArgument, options); const receivedIsSpy = isSpy(received); const receivedName = receivedIsSpy ? 'spy' : received.getMockName(); @@ -309,7 +309,7 @@ const createToBeCalledWithMatcher = (matcherName: string) => isNot: this.isNot, promise: this.promise, }; - ensureMock(received, matcherName.slice(1), expectedArgument, options); + ensureMockOrSpy(received, matcherName.slice(1), expectedArgument, options); const receivedIsSpy = isSpy(received); const type = receivedIsSpy ? 'spy' : 'mock function'; @@ -425,7 +425,7 @@ const createLastCalledWithMatcher = (matcherName: string) => isNot: this.isNot, promise: this.promise, }; - ensureMock(received, matcherName.slice(1), expectedArgument, options); + ensureMockOrSpy(received, matcherName.slice(1), expectedArgument, options); const receivedIsSpy = isSpy(received); const type = receivedIsSpy ? 'spy' : 'mock function'; @@ -549,7 +549,7 @@ const createNthCalledWithMatcher = (matcherName: string) => promise: this.promise, secondArgument: '...expected', }; - ensureMock(received, matcherName.slice(1), expectedArgument, options); + ensureMockOrSpy(received, matcherName.slice(1), expectedArgument, options); if (!Number.isSafeInteger(nth) || nth < 1) { throw new Error( @@ -756,19 +756,22 @@ const spyMatchers: MatchersObject = { toReturnWith: createToReturnWithMatcher('toReturnWith'), }; -const isSpy = (spy: any) => spy.calls && typeof spy.calls.count === 'function'; +const isMock = (received: any) => + received != null && received._isMockFunction === true; -const ensureMock = ( +const isSpy = (received: any) => + received != null && + received.calls != null && + typeof received.calls.all === 'function' && + typeof received.calls.count === 'function'; + +const ensureMockOrSpy = ( received: any, matcherName: string, expectedArgument: string, options: MatcherHintOptions, ) => { - if ( - !received || - ((received.calls === undefined || received.calls.all === undefined) && - received._isMockFunction !== true) - ) { + if (!isMock(received) && !isSpy(received)) { throw new Error( matcherErrorMessage( matcherHint(matcherName, undefined, expectedArgument, options), @@ -779,6 +782,23 @@ const ensureMock = ( } }; +const ensureMock = ( + received: any, + matcherName: string, + expectedArgument: string, + options: MatcherHintOptions, +) => { + if (!isMock(received)) { + throw new Error( + matcherErrorMessage( + matcherHint(matcherName, undefined, expectedArgument, options), + `${RECEIVED_COLOR('received')} value must be a mock function`, + printWithType('Received', received, printReceived), + ), + ); + } +}; + const getPrintedCalls = ( calls: Array, limit: number,