[WIP] Make toThrow matcher pass only if Error-like object is returned…

… from promises (#5670) * Fix rejects.not matcher * Check if resolved/rejected value is Error instance * Add test * Changelog * Change instanceof check * Add additional tests & update snaps * Add isError fn * Change err msg to also test partial string match * Move isError to expect utils * Add isError test * Update changelog
jestjs · Mar 2, 2018 · 27a1dc6 · 27a1dc6
1 parent aef82a2
commit 27a1dc6
Show file tree

Hide file tree

Showing 7 changed files with 231 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,10 @@
   ([#5558](https://github.com/facebook/jest/pull/5558))
 * `[jest-matcher-utils]` Add `isNot` option to `matcherHint` function
   ([#5512](https://github.com/facebook/jest/pull/5512))
+* `[expect]` Make toThrow matcher pass only if Error object is returned from
+  promises ([#5670](https://github.com/facebook/jest/pull/5670))
+* `[expect]` Add isError to utils
+  ([#5670](https://github.com/facebook/jest/pull/5670))
 
 ### Fixes
 
@@ -21,6 +25,8 @@
   ([#5692](https://github.com/facebook/jest/pull/5692))
 * `[jest-cli]` Fix update snapshot issue when using watchAll
   ([#5696](https://github.com/facebook/jest/pull/5696))
+* `[expect]` Fix rejects.not matcher
+  ([#5670](https://github.com/facebook/jest/pull/5670))
 
 ### Chore & Maintenance
 

diff --git a/packages/expect/src/__tests__/__snapshots__/to_throw_matchers.test.js.snap b/packages/expect/src/__tests__/__snapshots__/to_throw_matchers.test.js.snap
@@ -43,6 +43,32 @@ Got:
   string: <green>\\"111\\"</>"
 `;
 
+exports[`.toThrow() promise/async throws if Error-like object is returned did not throw at all 1`] = `
+[Error: [2mexpect([22m[31mfunction[39m[2m).toThrow([22m[32mundefined[39m[2m)[22m
+
+Expected the function to throw an error.
+But it didn't throw anything.]
+`;
+
+exports[`.toThrow() promise/async throws if Error-like object is returned threw, but class did not match 1`] = `
+[Error: [2mexpect([22m[31mfunction[39m[2m).toThrow([22m[32mtype[39m[2m)[22m
+
+Expected the function to throw an error of type:
+  [32m"Err2"[39m
+Instead, it threw:
+[31m  Error      [39m
+[31m      [2mat jestExpect ([22mpackages/expect/src/__tests__/toThrowMatchers-test.js[2m:24:74)[22m[39m]
+`;
+
+exports[`.toThrow() promise/async throws if Error-like object is returned threw, but should not have 1`] = `
+[Error: [2mexpect([22m[31mfunction[39m[2m).not.toThrow([22m[2m)[22m
+
+Expected the function not to throw an error.
+Instead, it threw:
+[31m  Error      [39m
+[31m      [2mat jestExpect ([22mpackages/expect/src/__tests__/toThrowMatchers-test.js[2m:24:74)[22m[39m]
+`;
+
 exports[`.toThrow() regexp did not throw at all 1`] = `
 "<dim>expect(</><red>function</><dim>).toThrow(</><green>regexp</><dim>)</>
 
@@ -142,6 +168,32 @@ Got:
   string: <green>\\"111\\"</>"
 `;
 
+exports[`.toThrowError() promise/async throws if Error-like object is returned did not throw at all 1`] = `
+[Error: [2mexpect([22m[31mfunction[39m[2m).toThrow([22m[32mundefined[39m[2m)[22m
+
+Expected the function to throw an error.
+But it didn't throw anything.]
+`;
+
+exports[`.toThrowError() promise/async throws if Error-like object is returned threw, but class did not match 1`] = `
+[Error: [2mexpect([22m[31mfunction[39m[2m).toThrow([22m[32mtype[39m[2m)[22m
+
+Expected the function to throw an error of type:
+  [32m"Err2"[39m
+Instead, it threw:
+[31m  Error      [39m
+[31m      [2mat jestExpect ([22mpackages/expect/src/__tests__/toThrowMatchers-test.js[2m:24:74)[22m[39m]
+`;
+
+exports[`.toThrowError() promise/async throws if Error-like object is returned threw, but should not have 1`] = `
+[Error: [2mexpect([22m[31mfunction[39m[2m).not.toThrow([22m[2m)[22m
+
+Expected the function not to throw an error.
+Instead, it threw:
+[31m  Error      [39m
+[31m      [2mat jestExpect ([22mpackages/expect/src/__tests__/toThrowMatchers-test.js[2m:24:74)[22m[39m]
+`;
+
 exports[`.toThrowError() regexp did not throw at all 1`] = `
 "<dim>expect(</><red>function</><dim>).toThrowError(</><green>regexp</><dim>)</>
 

diff --git a/packages/expect/src/__tests__/is_error.test.js b/packages/expect/src/__tests__/is_error.test.js
@@ -0,0 +1,45 @@
+/**
+ * @jest-environment jsdom
+ */
+/* eslint-env browser */
+
+import {isError} from '../utils';
+
+// Copied from https://github.com/graingert/angular.js/blob/a43574052e9775cbc1d7dd8a086752c979b0f020/test/AngularSpec.js#L1883
+describe('isError', () => {
+  function testErrorFromDifferentContext(createError) {
+    const iframe = document.createElement('iframe');
+    document.body.appendChild(iframe);
+    try {
+      const error = createError(iframe.contentWindow);
+      expect(isError(error)).toBe(true);
+    } finally {
+      iframe.parentElement.removeChild(iframe);
+    }
+  }
+
+  it('should not assume objects are errors', () => {
+    const fakeError = {message: 'A fake error', stack: 'no stack here'};
+    expect(isError(fakeError)).toBe(false);
+  });
+
+  it('should detect simple error instances', () => {
+    expect(isError(new Error())).toBe(true);
+  });
+
+  it('should detect errors from another context', () => {
+    testErrorFromDifferentContext(win => {
+      return new win.Error();
+    });
+  });
+
+  it('should detect DOMException errors from another context', () => {
+    testErrorFromDifferentContext(win => {
+      try {
+        win.document.querySelectorAll('');
+      } catch (e) {
+        return e;
+      }
+    });
+  });
+});
diff --git a/packages/expect/src/__tests__/to_throw_matchers.test.js b/packages/expect/src/__tests__/to_throw_matchers.test.js
@@ -11,8 +11,9 @@
 const jestExpect = require('../');
 
 // Custom Error class because node versions have different stack trace strings.
-class Error {
+class customError extends Error {
   constructor(message) {
+    super();
     this.message = message;
     this.name = 'Error';
     this.stack =
@@ -24,23 +25,23 @@ class Error {
 
 ['toThrowError', 'toThrow'].forEach(toThrow => {
   describe('.' + toThrow + '()', () => {
-    class Err extends Error {}
-    class Err2 extends Error {}
+    class Err extends customError {}
+    class Err2 extends customError {}
 
     test('to throw or not to throw', () => {
       jestExpect(() => {
-        throw new Error('apple');
+        throw new customError('apple');
       })[toThrow]();
       jestExpect(() => {}).not[toThrow]();
     });
 
     describe('strings', () => {
       it('passes', () => {
         jestExpect(() => {
-          throw new Error('apple');
+          throw new customError('apple');
         })[toThrow]('apple');
         jestExpect(() => {
-          throw new Error('banana');
+          throw new customError('banana');
         }).not[toThrow]('apple');
         jestExpect(() => {}).not[toThrow]('apple');
       });
@@ -54,7 +55,7 @@ class Error {
       test('threw, but message did not match', () => {
         expect(() => {
           jestExpect(() => {
-            throw new Error('apple');
+            throw new customError('apple');
           })[toThrow]('banana');
         }).toThrowErrorMatchingSnapshot();
       });
@@ -68,7 +69,7 @@ class Error {
       test('threw, but should not have', () => {
         expect(() => {
           jestExpect(() => {
-            throw new Error('apple');
+            throw new customError('apple');
           }).not[toThrow]('apple');
         }).toThrowErrorMatchingSnapshot();
       });
@@ -77,10 +78,10 @@ class Error {
     describe('regexp', () => {
       it('passes', () => {
         expect(() => {
-          throw new Error('apple');
+          throw new customError('apple');
         })[toThrow](/apple/);
         expect(() => {
-          throw new Error('banana');
+          throw new customError('banana');
         }).not[toThrow](/apple/);
         expect(() => {}).not[toThrow](/apple/);
       });
@@ -94,15 +95,15 @@ class Error {
       test('threw, but message did not match', () => {
         expect(() => {
           jestExpect(() => {
-            throw new Error('apple');
+            throw new customError('apple');
           })[toThrow](/banana/);
         }).toThrowErrorMatchingSnapshot();
       });
 
       test('threw, but should not have', () => {
         expect(() => {
           jestExpect(() => {
-            throw new Error('apple');
+            throw new customError('apple');
           }).not[toThrow](/apple/);
         }).toThrowErrorMatchingSnapshot();
       });
@@ -115,7 +116,7 @@ class Error {
         })[toThrow](Err);
         jestExpect(() => {
           throw new Err();
-        })[toThrow](Error);
+        })[toThrow](customError);
         jestExpect(() => {
           throw new Err();
         }).not[toThrow](Err2);
@@ -145,6 +146,89 @@ class Error {
       });
     });
 
+    describe('promise/async throws if Error-like object is returned', () => {
+      const asyncFn = async (shouldThrow?: boolean, resolve?: boolean) => {
+        let err;
+        if (shouldThrow) {
+          err = new Err('async apple');
+        }
+        if (resolve) {
+          return await Promise.resolve(err || 'apple');
+        } else {
+          return await Promise.reject(err || 'apple');
+        }
+      };
+
+      test('passes', async () => {
+        expect.assertions(24);
+        await jestExpect(Promise.reject(new Error())).rejects[toThrow]();
+
+        await jestExpect(asyncFn(true)).rejects[toThrow]();
+        await jestExpect(asyncFn(true)).rejects[toThrow](Err);
+        await jestExpect(asyncFn(true)).rejects[toThrow](Error);
+        await jestExpect(asyncFn(true)).rejects[toThrow]('apple');
+        await jestExpect(asyncFn(true)).rejects[toThrow](/app/);
+
+        await jestExpect(asyncFn(true)).rejects.not[toThrow](Err2);
+        await jestExpect(asyncFn(true)).rejects.not[toThrow]('banana');
+        await jestExpect(asyncFn(true)).rejects.not[toThrow](/banana/);
+
+        await jestExpect(asyncFn(true, true)).resolves[toThrow]();
+
+        await jestExpect(asyncFn(false, true)).resolves.not[toThrow]();
+        await jestExpect(asyncFn(false, true)).resolves.not[toThrow](Error);
+        await jestExpect(asyncFn(false, true)).resolves.not[toThrow]('apple');
+        await jestExpect(asyncFn(false, true)).resolves.not[toThrow](/apple/);
+        await jestExpect(asyncFn(false, true)).resolves.not[toThrow]('banana');
+        await jestExpect(asyncFn(false, true)).resolves.not[toThrow](/banana/);
+
+        await jestExpect(asyncFn()).rejects.not[toThrow]();
+        await jestExpect(asyncFn()).rejects.not[toThrow](Error);
+        await jestExpect(asyncFn()).rejects.not[toThrow]('apple');
+        await jestExpect(asyncFn()).rejects.not[toThrow](/apple/);
+        await jestExpect(asyncFn()).rejects.not[toThrow]('banana');
+        await jestExpect(asyncFn()).rejects.not[toThrow](/banana/);
+
+        // Works with nested functions inside promises
+        await jestExpect(
+          Promise.reject(() => {
+            throw new Error();
+          }),
+        ).rejects[toThrow]();
+        await jestExpect(Promise.reject(() => {})).rejects.not[toThrow]();
+      });
+
+      test('did not throw at all', async () => {
+        let err;
+        try {
+          await jestExpect(asyncFn()).rejects.toThrow();
+        } catch (error) {
+          err = error;
+        }
+        expect(err).toMatchSnapshot();
+      });
+
+      test('threw, but class did not match', async () => {
+        let err;
+        try {
+          await jestExpect(asyncFn(true)).rejects.toThrow(Err2);
+        } catch (error) {
+          err = error;
+        }
+        expect(err).toMatchSnapshot();
+      });
+
+      test('threw, but should not have', async () => {
+        let err;
+        try {
+          await jestExpect(asyncFn(true)).rejects.not.toThrow();
+        } catch (error) {
+          err = error;
+        }
+        expect(err).toMatchSnapshot();
+      });
+    });
+
     test('invalid arguments', () => {
       expect(() =>
         jestExpect(() => {})[toThrow](111),

diff --git a/packages/expect/src/index.js b/packages/expect/src/index.js
@@ -109,7 +109,7 @@ const expect = (actual: any, ...rest): ExpectationObject => {
     );
     expectation.rejects.not[name] = makeRejectMatcher(
       name,
-      matcher,
+      promiseMatcher,
       true,
       actual,
     );

diff --git a/packages/expect/src/to_throw_matchers.js b/packages/expect/src/to_throw_matchers.js
@@ -20,6 +20,7 @@ import {
   printWithType,
 } from 'jest-matcher-utils';
 import {equals} from './jasmine_utils';
+import {isError} from './utils';
 
 export const createMatcher = (matcherName: string, fromPromise?: boolean) => (
   actual: Function,
@@ -28,21 +29,24 @@ export const createMatcher = (matcherName: string, fromPromise?: boolean) => (
   const value = expected;
   let error;
 
-  if (fromPromise) {
+  if (fromPromise && isError(actual)) {
     error = actual;
   } else {
     if (typeof actual !== 'function') {
-      throw new Error(
-        matcherHint(matcherName, 'function', getType(value)) +
-          '\n\n' +
-          'Received value must be a function, but instead ' +
-          `"${getType(actual)}" was found`,
-      );
-    }
-    try {
-      actual();
-    } catch (e) {
-      error = e;
+      if (!fromPromise) {
+        throw new Error(
+          matcherHint(matcherName, 'function', getType(value)) +
+            '\n\n' +
+            'Received value must be a function, but instead ' +
+            `"${getType(actual)}" was found`,
+        );
+      }
+    } else {
+      try {
+        actual();
+      } catch (e) {
+        error = e;
+      }
     }
   }
 

diff --git a/packages/expect/src/utils.js b/packages/expect/src/utils.js
@@ -195,3 +195,17 @@ export const partition = <T>(
 
   return result;
 };
+
+// Copied from https://github.com/graingert/angular.js/blob/a43574052e9775cbc1d7dd8a086752c979b0f020/src/Angular.js#L685-L693
+export const isError = (value: any) => {
+  switch (Object.prototype.toString.call(value)) {
+    case '[object Error]':
+      return true;
+    case '[object Exception]':
+      return true;
+    case '[object DOMException]':
+      return true;
+    default:
+      return value instanceof Error;
+  }
+};
-Original file line number
+Diff line change
@@ Expand Up @@
         );
         expectation.rejects.not[name] = makeRejectMatcher(
           name,
-          matcher,
+          promiseMatcher,
           true,
           actual,
         );
@@ Expand Down @@