expect: Improve report when mock-spy matcher fails, part 3 (#8697)

* expect: Improve report when mock-spy matcher fails, part 3 * Update CHANGELOG.md
jestjs · Jul 16, 2019 · 7988912 · 7988912
1 parent 4a7b28c
commit 7988912
Show file tree

Hide file tree

Showing 4 changed files with 152 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 - `[expect]` Highlight substring differences when matcher fails, part 2 ([#8528](https://github.com/facebook/jest/pull/8528))
 - `[expect]` Improve report when mock-spy matcher fails, part 1 ([#8640](https://github.com/facebook/jest/pull/8640))
 - `[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))
 - `[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
@@ -272,9 +272,32 @@ Expected mock function \\"named-mock\\" first call to not have been called with:
   <green>[\\"foo\\", \\"bar\\"]</>"
 `;
 
-exports[`nthCalledWith should reject non integer nth value 1`] = `"nth value <red>0.1</> must be a positive integer greater than <green>0</>"`;
+exports[`nthCalledWith negative throw matcher error for n that is not integer 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>nthCalledWith<dim>(</><green>n</><dim>, </><green>...expected</><dim>)</>
 
-exports[`nthCalledWith should reject nth value smaller than 1 1`] = `"nth value <red>0</> must be a positive integer greater than <green>0</>"`;
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has type:  number
+n has value: <green>Infinity</>"
+`;
+
+exports[`nthCalledWith positive throw matcher error for n that is not integer 1`] = `
+"<dim>expect(</><red>received</><dim>).</>nthCalledWith<dim>(</><green>n</><dim>, </><green>...expected</><dim>)</>
+
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has type:  number
+n has value: <green>0.1</>"
+`;
+
+exports[`nthCalledWith positive throw matcher error for n that is not positive integer 1`] = `
+"<dim>expect(</><red>received</><dim>).</>nthCalledWith<dim>(</><green>n</><dim>, </><green>...expected</><dim>)</>
+
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has type:  number
+n has value: <green>0</>"
+`;
 
 exports[`nthCalledWith should replace 1st, 2nd, 3rd with first, second, third 1`] = `
 "<dim>expect(</><red>jest.fn()</><dim>).nthCalledWith(</><green>expected</><dim>)</>
@@ -460,7 +483,31 @@ But the 4th call returned exactly:
   <red>0</>"
 `;
 
-exports[`nthReturnedWith nthReturnedWith should reject non integer nth value 1`] = `"nth value <red>0.1</> must be a positive integer greater than <green>0</>"`;
+exports[`nthReturnedWith nthReturnedWith negative throw matcher error for n that is not number 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>nthReturnedWith<dim>(</><green>n</><dim>, </><green>expected</><dim>)</>
+
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has value: <green>undefined</>"
+`;
+
+exports[`nthReturnedWith nthReturnedWith positive throw matcher error for n that is not integer 1`] = `
+"<dim>expect(</><red>received</><dim>).</>nthReturnedWith<dim>(</><green>n</><dim>, </><green>expected</><dim>)</>
+
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has type:  number
+n has value: <green>0.1</>"
+`;
+
+exports[`nthReturnedWith nthReturnedWith positive throw matcher error for n that is not positive integer 1`] = `
+"<dim>expect(</><red>received</><dim>).</>nthReturnedWith<dim>(</><green>n</><dim>, </><green>expected</><dim>)</>
+
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has type:  number
+n has value: <green>0</>"
+`;
 
 exports[`nthReturnedWith nthReturnedWith should reject nth value greater than number of calls 1`] = `
 "<dim>expect(</><red>jest.fn()</><dim>).nthReturnedWith(</><green>expected</><dim>)</>
@@ -470,8 +517,6 @@ Expected mock function 4th call to have returned with:
 But it was only called <red>3</> times"
 `;
 
-exports[`nthReturnedWith nthReturnedWith should reject nth value smaller than 1 1`] = `"nth value <red>0</> must be a positive integer greater than <green>0</>"`;
-
 exports[`nthReturnedWith nthReturnedWith should replace 1st, 2nd, 3rd with first, second, third 1`] = `
 "<dim>expect(</><red>jest.fn()</><dim>).nthReturnedWith(</><green>expected</><dim>)</>
 
@@ -1390,9 +1435,32 @@ Expected mock function \\"named-mock\\" first call to not have been called with:
   <green>[\\"foo\\", \\"bar\\"]</>"
 `;
 
-exports[`toHaveBeenNthCalledWith should reject non integer nth value 1`] = `"nth value <red>0.1</> must be a positive integer greater than <green>0</>"`;
+exports[`toHaveBeenNthCalledWith negative throw matcher error for n that is not integer 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toHaveBeenNthCalledWith<dim>(</><green>n</><dim>, </><green>...expected</><dim>)</>
 
-exports[`toHaveBeenNthCalledWith should reject nth value smaller than 1 1`] = `"nth value <red>0</> must be a positive integer greater than <green>0</>"`;
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has type:  number
+n has value: <green>Infinity</>"
+`;
+
+exports[`toHaveBeenNthCalledWith positive throw matcher error for n that is not integer 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toHaveBeenNthCalledWith<dim>(</><green>n</><dim>, </><green>...expected</><dim>)</>
+
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has type:  number
+n has value: <green>0.1</>"
+`;
+
+exports[`toHaveBeenNthCalledWith positive throw matcher error for n that is not positive integer 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toHaveBeenNthCalledWith<dim>(</><green>n</><dim>, </><green>...expected</><dim>)</>
+
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has type:  number
+n has value: <green>0</>"
+`;
 
 exports[`toHaveBeenNthCalledWith should replace 1st, 2nd, 3rd with first, second, third 1`] = `
 "<dim>expect(</><red>jest.fn()</><dim>).toHaveBeenNthCalledWith(</><green>expected</><dim>)</>
@@ -1717,7 +1785,31 @@ But the 4th call returned exactly:
   <red>0</>"
 `;
 
-exports[`toHaveNthReturnedWith nthReturnedWith should reject non integer nth value 1`] = `"nth value <red>0.1</> must be a positive integer greater than <green>0</>"`;
+exports[`toHaveNthReturnedWith nthReturnedWith negative throw matcher error for n that is not number 1`] = `
+"<dim>expect(</><red>received</><dim>).</>not<dim>.</>toHaveNthReturnedWith<dim>(</><green>n</><dim>, </><green>expected</><dim>)</>
+
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has value: <green>undefined</>"
+`;
+
+exports[`toHaveNthReturnedWith nthReturnedWith positive throw matcher error for n that is not integer 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toHaveNthReturnedWith<dim>(</><green>n</><dim>, </><green>expected</><dim>)</>
+
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has type:  number
+n has value: <green>0.1</>"
+`;
+
+exports[`toHaveNthReturnedWith nthReturnedWith positive throw matcher error for n that is not positive integer 1`] = `
+"<dim>expect(</><red>received</><dim>).</>toHaveNthReturnedWith<dim>(</><green>n</><dim>, </><green>expected</><dim>)</>
+
+<bold>Matcher error</>: <green>n</> must be a positive integer
+
+n has type:  number
+n has value: <green>0</>"
+`;
 
 exports[`toHaveNthReturnedWith nthReturnedWith should reject nth value greater than number of calls 1`] = `
 "<dim>expect(</><red>jest.fn()</><dim>).toHaveNthReturnedWith(</><green>expected</><dim>)</>
@@ -1727,8 +1819,6 @@ Expected mock function 4th call to have returned with:
 But it was only called <red>3</> times"
 `;
 
-exports[`toHaveNthReturnedWith nthReturnedWith should reject nth value smaller than 1 1`] = `"nth value <red>0</> must be a positive integer greater than <green>0</>"`;
-
 exports[`toHaveNthReturnedWith nthReturnedWith should replace 1st, 2nd, 3rd with first, second, third 1`] = `
 "<dim>expect(</><red>jest.fn()</><dim>).toHaveNthReturnedWith(</><green>expected</><dim>)</>
 

diff --git a/packages/expect/src/__tests__/spyMatchers.test.js b/packages/expect/src/__tests__/spyMatchers.test.js
@@ -338,7 +338,7 @@ const jestExpect = require('../');
         }).toThrowErrorMatchingSnapshot();
       });
 
-      test('should reject nth value smaller than 1', async () => {
+      test('positive throw matcher error for n that is not positive integer', async () => {
         const fn = jest.fn();
         fn('foo1', 'bar');
 
@@ -347,14 +347,23 @@ const jestExpect = require('../');
         }).toThrowErrorMatchingSnapshot();
       });
 
-      test('should reject non integer nth value', async () => {
+      test('positive throw matcher error for n that is not integer', async () => {
         const fn = jest.fn();
         fn('foo1', 'bar');
 
         expect(() => {
           jestExpect(fn)[calledWith](0.1, 'foo1', 'bar');
         }).toThrowErrorMatchingSnapshot();
       });
+
+      test('negative throw matcher error for n that is not integer', async () => {
+        const fn = jest.fn();
+        fn('foo1', 'bar');
+
+        expect(() => {
+          jestExpect(fn).not[calledWith](Infinity, 'foo1', 'bar');
+        }).toThrowErrorMatchingSnapshot();
+      });
     }
 
     test(`includes the custom mock name in the error message`, () => {
@@ -963,7 +972,7 @@ const jestExpect = require('../');
           }).toThrowErrorMatchingSnapshot();
         });
 
-        test('should reject nth value smaller than 1', async () => {
+        test('positive throw matcher error for n that is not positive integer', async () => {
           const fn = jest.fn(() => 'foo');
           fn();
 
@@ -983,7 +992,7 @@ const jestExpect = require('../');
           }).toThrowErrorMatchingSnapshot();
         });
 
-        test('should reject non integer nth value', async () => {
+        test('positive throw matcher error for n that is not integer', async () => {
           const fn = jest.fn(() => 'foo');
           fn('foo');
 
@@ -992,6 +1001,15 @@ const jestExpect = require('../');
           }).toThrowErrorMatchingSnapshot();
         });
 
+        test('negative throw matcher error for n that is not number', async () => {
+          const fn = jest.fn(() => 'foo');
+          fn('foo');
+
+          expect(() => {
+            jestExpect(fn).not[returnedWith]();
+          }).toThrowErrorMatchingSnapshot();
+        });
+
         test(`incomplete recursive calls are handled properly`, () => {
           // sums up all integers from 0 -> value, using recursion
           const fn = jest.fn(value => {

diff --git a/packages/expect/src/spyMatchers.ts b/packages/expect/src/spyMatchers.ts
@@ -9,6 +9,7 @@ import {
   diff,
   ensureExpectedIsNumber,
   ensureNoExpected,
+  EXPECTED_COLOR,
   matcherErrorMessage,
   matcherHint,
   MatcherHintOptions,
@@ -422,19 +423,24 @@ const createNthCalledWithMatcher = (matcherName: string) =>
     };
     ensureMock(received, matcherName.slice(1), expectedArgument, options);
 
+    if (!Number.isSafeInteger(nth) || nth < 1) {
+      throw new Error(
+        matcherErrorMessage(
+          matcherHint(
+            matcherName.slice(1),
+            undefined,
+            expectedArgument,
+            options,
+          ),
+          `${EXPECTED_COLOR(expectedArgument)} must be a positive integer`,
+          printWithType(expectedArgument, nth, printExpected),
+        ),
+      );
+    }
+
     const receivedIsSpy = isSpy(received);
     const type = receivedIsSpy ? 'spy' : 'mock function';
 
-    // @ts-ignore
-    if (typeof nth !== 'number' || parseInt(nth, 10) !== nth || nth < 1) {
-      const message = () =>
-        `nth value ${printReceived(
-          nth,
-        )} must be a positive integer greater than ${printExpected(0)}`;
-      const pass = false;
-      return {message, pass};
-    }
-
     const receivedName = receivedIsSpy ? 'spy' : received.getMockName();
     const identifier =
       receivedIsSpy || receivedName === 'jest.fn()'
@@ -483,14 +489,19 @@ const createNthReturnedWithMatcher = (matcherName: string) =>
     };
     ensureMock(received, matcherName.slice(1), expectedArgument, options);
 
-    //@ts-ignore
-    if (typeof nth !== 'number' || parseInt(nth, 10) !== nth || nth < 1) {
-      const message = () =>
-        `nth value ${printReceived(
-          nth,
-        )} must be a positive integer greater than ${printExpected(0)}`;
-      const pass = false;
-      return {message, pass};
+    if (!Number.isSafeInteger(nth) || nth < 1) {
+      throw new Error(
+        matcherErrorMessage(
+          matcherHint(
+            matcherName.slice(1),
+            undefined,
+            expectedArgument,
+            options,
+          ),
+          `${EXPECTED_COLOR(expectedArgument)} must be a positive integer`,
+          printWithType(expectedArgument, nth, printExpected),
+        ),
+      );
     }
 
     const receivedName = received.getMockName();