From 27a1dc659dd3f2c91efb106961a5d0c640a1085b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Dani=C5=A1?= Date: Fri, 2 Mar 2018 12:38:30 +0100 Subject: [PATCH] [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 --- CHANGELOG.md | 6 + .../to_throw_matchers.test.js.snap | 52 +++++++++ .../expect/src/__tests__/is_error.test.js | 45 +++++++ .../src/__tests__/to_throw_matchers.test.js | 110 +++++++++++++++--- packages/expect/src/index.js | 2 +- packages/expect/src/to_throw_matchers.js | 28 +++-- packages/expect/src/utils.js | 14 +++ 7 files changed, 231 insertions(+), 26 deletions(-) create mode 100644 packages/expect/src/__tests__/is_error.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index d5121964bfc2..cd0076cb3b8f 100644 --- 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 index 3c7fd01c7f58..0e03ed6d61b8 100644 --- 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: \\"111\\"" `; +exports[`.toThrow() promise/async throws if Error-like object is returned did not throw at all 1`] = ` +[Error: expect(function).toThrow(undefined) + +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: expect(function).toThrow(type) + +Expected the function to throw an error of type: + "Err2" +Instead, it threw: + Error  + at jestExpect (packages/expect/src/__tests__/toThrowMatchers-test.js:24:74)] +`; + +exports[`.toThrow() promise/async throws if Error-like object is returned threw, but should not have 1`] = ` +[Error: expect(function).not.toThrow() + +Expected the function not to throw an error. +Instead, it threw: + Error  + at jestExpect (packages/expect/src/__tests__/toThrowMatchers-test.js:24:74)] +`; + exports[`.toThrow() regexp did not throw at all 1`] = ` "expect(function).toThrow(regexp) @@ -142,6 +168,32 @@ Got: string: \\"111\\"" `; +exports[`.toThrowError() promise/async throws if Error-like object is returned did not throw at all 1`] = ` +[Error: expect(function).toThrow(undefined) + +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: expect(function).toThrow(type) + +Expected the function to throw an error of type: + "Err2" +Instead, it threw: + Error  + at jestExpect (packages/expect/src/__tests__/toThrowMatchers-test.js:24:74)] +`; + +exports[`.toThrowError() promise/async throws if Error-like object is returned threw, but should not have 1`] = ` +[Error: expect(function).not.toThrow() + +Expected the function not to throw an error. +Instead, it threw: + Error  + at jestExpect (packages/expect/src/__tests__/toThrowMatchers-test.js:24:74)] +`; + exports[`.toThrowError() regexp did not throw at all 1`] = ` "expect(function).toThrowError(regexp) diff --git a/packages/expect/src/__tests__/is_error.test.js b/packages/expect/src/__tests__/is_error.test.js new file mode 100644 index 000000000000..b08969c920a0 --- /dev/null +++ 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 index a8a37079bcf5..3bbe89b6b933 100644 --- 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,12 +25,12 @@ 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](); }); @@ -37,10 +38,10 @@ class Error { 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,7 +95,7 @@ class Error { test('threw, but message did not match', () => { expect(() => { jestExpect(() => { - throw new Error('apple'); + throw new customError('apple'); })[toThrow](/banana/); }).toThrowErrorMatchingSnapshot(); }); @@ -102,7 +103,7 @@ class Error { 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 index 50a495f50332..51517855b673 100644 --- 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 index 9bab72a182c5..f86bc4835dc7 100644 --- 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 index 4018e5b09633..35031a2264a2 100644 --- a/packages/expect/src/utils.js +++ b/packages/expect/src/utils.js @@ -195,3 +195,17 @@ export const partition = ( 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; + } +};