From f857d309646154f6ae587ec744195453030d1f7e Mon Sep 17 00:00:00 2001
From: Mark Pedrotti <pedrottimark@gmail.com>
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: <red>3</>"
 exports[`lastReturnedWith works only on spies or jest.fn 1`] = `
 "<dim>expect(</><red>received</><dim>).</>lastReturnedWith<dim>(</><green>expected</><dim>)</>
 
-<bold>Matcher error</>: <red>received</> value must be a mock or spy function
+<bold>Matcher error</>: <red>received</> value must be a mock function
 
 Received has type:  function
 Received has value: <red>[Function fn]</>"
@@ -590,7 +590,7 @@ Number of returns: <red>3</>"
 exports[`nthReturnedWith works only on spies or jest.fn 1`] = `
 "<dim>expect(</><red>received</><dim>).</>nthReturnedWith<dim>(</><green>n</><dim>, </><green>expected</><dim>)</>
 
-<bold>Matcher error</>: <red>received</> value must be a mock or spy function
+<bold>Matcher error</>: <red>received</> value must be a mock function
 
 Received has type:  function
 Received has value: <red>[Function fn]</>"
@@ -699,7 +699,7 @@ Expected has value: <green>555</>"
 `;
 
 exports[`toBeCalled .not passes when called 1`] = `
-"<dim>expect(</><red>jest.fn()</><dim>).</>toBeCalled<dim>()</>
+"<dim>expect(</><red>spy</><dim>).</>toBeCalled<dim>()</>
 
 Expected number of calls: >= <green>1</>
 Received number of calls:    <red>0</>"
@@ -880,7 +880,7 @@ Expected has value: <green>[Function anonymous]</>"
 `;
 
 exports[`toBeCalledTimes passes if function called equal to expected times 1`] = `
-"<dim>expect(</><red>jest.fn()</><dim>).</>not<dim>.</>toBeCalledTimes<dim>(</><green>expected</><dim>)</>
+"<dim>expect(</><red>spy</><dim>).</>not<dim>.</>toBeCalledTimes<dim>(</><green>expected</><dim>)</>
 
 Expected number of calls: not <green>2</>"
 `;
@@ -1029,7 +1029,7 @@ Expected has value: <green>555</>"
 `;
 
 exports[`toHaveBeenCalled .not passes when called 1`] = `
-"<dim>expect(</><red>jest.fn()</><dim>).</>toHaveBeenCalled<dim>()</>
+"<dim>expect(</><red>spy</><dim>).</>toHaveBeenCalled<dim>()</>
 
 Expected number of calls: >= <green>1</>
 Received number of calls:    <red>0</>"
@@ -1210,7 +1210,7 @@ Expected has value: <green>[Function anonymous]</>"
 `;
 
 exports[`toHaveBeenCalledTimes passes if function called equal to expected times 1`] = `
-"<dim>expect(</><red>jest.fn()</><dim>).</>not<dim>.</>toHaveBeenCalledTimes<dim>(</><green>expected</><dim>)</>
+"<dim>expect(</><red>spy</><dim>).</>not<dim>.</>toHaveBeenCalledTimes<dim>(</><green>expected</><dim>)</>
 
 Expected number of calls: not <green>2</>"
 `;
@@ -1689,7 +1689,7 @@ Number of returns: <red>3</>"
 exports[`toHaveLastReturnedWith works only on spies or jest.fn 1`] = `
 "<dim>expect(</><red>received</><dim>).</>toHaveLastReturnedWith<dim>(</><green>expected</><dim>)</>
 
-<bold>Matcher error</>: <red>received</> value must be a mock or spy function
+<bold>Matcher error</>: <red>received</> value must be a mock function
 
 Received has type:  function
 Received has value: <red>[Function fn]</>"
@@ -1939,7 +1939,7 @@ Number of returns: <red>3</>"
 exports[`toHaveNthReturnedWith works only on spies or jest.fn 1`] = `
 "<dim>expect(</><red>received</><dim>).</>toHaveNthReturnedWith<dim>(</><green>n</><dim>, </><green>expected</><dim>)</>
 
-<bold>Matcher error</>: <red>received</> value must be a mock or spy function
+<bold>Matcher error</>: <red>received</> value must be a mock function
 
 Received has type:  function
 Received has value: <red>[Function fn]</>"
@@ -2073,7 +2073,7 @@ Received number of returns:    <red>0</>"
 exports[`toHaveReturned .not works only on jest.fn 1`] = `
 "<dim>expect(</><red>received</><dim>).</>not<dim>.</>toHaveReturned<dim>()</>
 
-<bold>Matcher error</>: <red>received</> value must be a mock or spy function
+<bold>Matcher error</>: <red>received</> value must be a mock function
 
 Received has type:  function
 Received has value: <red>[Function fn]</>"
@@ -2135,6 +2135,15 @@ Received number of returns: <red>1</>
 1: <red>undefined</>"
 `;
 
+exports[`toHaveReturned throw matcher error if received is spy 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toHaveReturned<dim>()</>
+
+<bold>Matcher error</>: <red>received</> value must be a mock function
+
+Received has type:  function
+Received has value: <red>[Function spy]</>"
+`;
+
 exports[`toHaveReturnedTimes .not only accepts a number argument 1`] = `
 "<dim>expect(</><red>received</><dim>).</>not<dim>.</>toHaveReturnedTimes<dim>(</><green>expected</><dim>)</>
 
@@ -2300,13 +2309,13 @@ exports[`toHaveReturnedTimes passes if function returned equal to expected times
 Expected number of returns: not <green>2</>"
 `;
 
-exports[`toHaveReturnedTimes works only on spies or jest.fn 1`] = `
-"<dim>expect(</><red>received</><dim>).</>toHaveReturnedTimes<dim>(</><green>expected</><dim>)</>
+exports[`toHaveReturnedTimes throw matcher error if received is spy 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toHaveReturnedTimes<dim>(</><green>expected</><dim>)</>
 
-<bold>Matcher error</>: <red>received</> value must be a mock or spy function
+<bold>Matcher error</>: <red>received</> value must be a mock function
 
 Received has type:  function
-Received has value: <red>[Function fn]</>"
+Received has value: <red>[Function spy]</>"
 `;
 
 exports[`toHaveReturnedWith a call that throws is not considered to have returned 1`] = `
@@ -2365,7 +2374,7 @@ Number of returns: <red>6</>"
 exports[`toHaveReturnedWith works only on spies or jest.fn 1`] = `
 "<dim>expect(</><red>received</><dim>).</>toHaveReturnedWith<dim>(</><green>expected</><dim>)</>
 
-<bold>Matcher error</>: <red>received</> value must be a mock or spy function
+<bold>Matcher error</>: <red>received</> value must be a mock function
 
 Received has type:  function
 Received has value: <red>[Function fn]</>"
@@ -2489,7 +2498,7 @@ Received number of returns:    <red>0</>"
 exports[`toReturn .not works only on jest.fn 1`] = `
 "<dim>expect(</><red>received</><dim>).</>not<dim>.</>toReturn<dim>()</>
 
-<bold>Matcher error</>: <red>received</> value must be a mock or spy function
+<bold>Matcher error</>: <red>received</> value must be a mock function
 
 Received has type:  function
 Received has value: <red>[Function fn]</>"
@@ -2551,6 +2560,15 @@ Received number of returns: <red>1</>
 1: <red>undefined</>"
 `;
 
+exports[`toReturn throw matcher error if received is spy 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toReturn<dim>()</>
+
+<bold>Matcher error</>: <red>received</> value must be a mock function
+
+Received has type:  function
+Received has value: <red>[Function spy]</>"
+`;
+
 exports[`toReturnTimes .not only accepts a number argument 1`] = `
 "<dim>expect(</><red>received</><dim>).</>not<dim>.</>toReturnTimes<dim>(</><green>expected</><dim>)</>
 
@@ -2716,13 +2734,13 @@ exports[`toReturnTimes passes if function returned equal to expected times 1`] =
 Expected number of returns: not <green>2</>"
 `;
 
-exports[`toReturnTimes works only on spies or jest.fn 1`] = `
-"<dim>expect(</><red>received</><dim>).</>toReturnTimes<dim>(</><green>expected</><dim>)</>
+exports[`toReturnTimes throw matcher error if received is spy 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toReturnTimes<dim>(</><green>expected</><dim>)</>
 
-<bold>Matcher error</>: <red>received</> value must be a mock or spy function
+<bold>Matcher error</>: <red>received</> value must be a mock function
 
 Received has type:  function
-Received has value: <red>[Function fn]</>"
+Received has value: <red>[Function spy]</>"
 `;
 
 exports[`toReturnWith a call that throws is not considered to have returned 1`] = `
@@ -2781,7 +2799,7 @@ Number of returns: <red>6</>"
 exports[`toReturnWith works only on spies or jest.fn 1`] = `
 "<dim>expect(</><red>received</><dim>).</>toReturnWith<dim>(</><green>expected</><dim>)</>
 
-<bold>Matcher error</>: <red>received</> value must be a mock or spy function
+<bold>Matcher error</>: <red>received</> value must be a mock function
 
 Received has type:  function
 Received has value: <red>[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<any>,
   limit: number,