From f254d9abda0574b1a35b3c1c129bf667bdaebcbd Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 5 Dec 2017 15:18:50 -0800 Subject: [PATCH 01/17] Added toWarnInDev matcher and connected to 1 test --- .../__tests__/CSSPropertyOperations-test.js | 131 +++++++----------- scripts/jest/matchers/toWarnDev.js | 65 +++++++++ scripts/jest/setupTests.js | 4 + 3 files changed, 116 insertions(+), 84 deletions(-) create mode 100644 scripts/jest/matchers/toWarnDev.js diff --git a/packages/react-dom/src/__tests__/CSSPropertyOperations-test.js b/packages/react-dom/src/__tests__/CSSPropertyOperations-test.js index d1f97800dd993..29df17074ec7b 100644 --- a/packages/react-dom/src/__tests__/CSSPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/CSSPropertyOperations-test.js @@ -13,10 +13,6 @@ const React = require('react'); const ReactDOM = require('react-dom'); const ReactDOMServer = require('react-dom/server'); -function normalizeCodeLocInfo(str) { - return str && str.replace(/at .+?:\d+/g, 'at **'); -} - describe('CSSPropertyOperations', () => { it('should automatically append `px` to relevant styles', () => { const styles = { @@ -91,17 +87,13 @@ describe('CSSPropertyOperations', () => { } } - spyOnDev(console, 'error'); const root = document.createElement('div'); - ReactDOM.render(, root); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual( - 'Warning: Unsupported style property background-color. Did you mean backgroundColor?' + - '\n in div (at **)' + - '\n in Comp (at **)', - ); - } + + expect(() => ReactDOM.render(, root)).toWarnDev( + 'Warning: Unsupported style property background-color. Did you mean backgroundColor?' + + '\n in div (at **)' + + '\n in Comp (at **)', + ); }); it('should warn when updating hyphenated style names', () => { @@ -113,28 +105,21 @@ describe('CSSPropertyOperations', () => { } } - spyOnDev(console, 'error'); const styles = { '-ms-transform': 'translate3d(0, 0, 0)', '-webkit-transform': 'translate3d(0, 0, 0)', }; const root = document.createElement('div'); ReactDOM.render(, root); - ReactDOM.render(, root); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(2); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual( - 'Warning: Unsupported style property -ms-transform. Did you mean msTransform?' + - '\n in div (at **)' + - '\n in Comp (at **)', - ); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual( - 'Warning: Unsupported style property -webkit-transform. Did you mean WebkitTransform?' + - '\n in div (at **)' + - '\n in Comp (at **)', - ); - } + + expect(() => ReactDOM.render(, root)).toWarnDev([ + 'Warning: Unsupported style property -ms-transform. Did you mean msTransform?' + + '\n in div (at **)' + + '\n in Comp (at **)', + 'Warning: Unsupported style property -webkit-transform. Did you mean WebkitTransform?' + + '\n in div (at **)' + + '\n in Comp (at **)', + ]); }); it('warns when miscapitalizing vendored style names', () => { @@ -154,25 +139,19 @@ describe('CSSPropertyOperations', () => { } } - spyOnDev(console, 'error'); const root = document.createElement('div'); - ReactDOM.render(, root); - if (__DEV__) { + + expect(() => ReactDOM.render(, root)).toWarnDev([ // msTransform is correct already and shouldn't warn - expect(console.error.calls.count()).toBe(2); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual( - 'Warning: Unsupported vendor-prefixed style property oTransform. ' + - 'Did you mean OTransform?' + - '\n in div (at **)' + - '\n in Comp (at **)', - ); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual( - 'Warning: Unsupported vendor-prefixed style property webkitTransform. ' + - 'Did you mean WebkitTransform?' + - '\n in div (at **)' + - '\n in Comp (at **)', - ); - } + 'Warning: Unsupported vendor-prefixed style property oTransform. ' + + 'Did you mean OTransform?' + + '\n in div (at **)' + + '\n in Comp (at **)', + 'Warning: Unsupported vendor-prefixed style property webkitTransform. ' + + 'Did you mean WebkitTransform?' + + '\n in div (at **)' + + '\n in Comp (at **)', + ]); }); it('should warn about style having a trailing semicolon', () => { @@ -193,24 +172,18 @@ describe('CSSPropertyOperations', () => { } } - spyOnDev(console, 'error'); const root = document.createElement('div'); - ReactDOM.render(, root); - if (__DEV__) { - expect(console.error.calls.count()).toBe(2); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual( - "Warning: Style property values shouldn't contain a semicolon. " + - 'Try "backgroundColor: blue" instead.' + - '\n in div (at **)' + - '\n in Comp (at **)', - ); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual( - "Warning: Style property values shouldn't contain a semicolon. " + - 'Try "color: red" instead.' + - '\n in div (at **)' + - '\n in Comp (at **)', - ); - } + + expect(() => ReactDOM.render(, root)).toWarnDev([ + "Warning: Style property values shouldn't contain a semicolon. " + + 'Try "backgroundColor: blue" instead.' + + '\n in div (at **)' + + '\n in Comp (at **)', + "Warning: Style property values shouldn't contain a semicolon. " + + 'Try "color: red" instead.' + + '\n in div (at **)' + + '\n in Comp (at **)', + ]); }); it('should warn about style containing a NaN value', () => { @@ -222,18 +195,13 @@ describe('CSSPropertyOperations', () => { } } - spyOnDev(console, 'error'); const root = document.createElement('div'); - ReactDOM.render(, root); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual( - 'Warning: `NaN` is an invalid value for the `fontSize` css style property.' + - '\n in div (at **)' + - '\n in Comp (at **)', - ); - } + expect(() => ReactDOM.render(, root)).toWarnDev( + 'Warning: `NaN` is an invalid value for the `fontSize` css style property.' + + '\n in div (at **)' + + '\n in Comp (at **)', + ); }); it('should not warn when setting CSS custom properties', () => { @@ -256,18 +224,13 @@ describe('CSSPropertyOperations', () => { } } - spyOnDev(console, 'error'); const root = document.createElement('div'); - ReactDOM.render(, root); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual( - 'Warning: `Infinity` is an invalid value for the `fontSize` css style property.' + - '\n in div (at **)' + - '\n in Comp (at **)', - ); - } + expect(() => ReactDOM.render(, root)).toWarnDev( + 'Warning: `Infinity` is an invalid value for the `fontSize` css style property.' + + '\n in div (at **)' + + '\n in Comp (at **)', + ); }); it('should not add units to CSS custom properties', () => { diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js new file mode 100644 index 0000000000000..bc2a599729384 --- /dev/null +++ b/scripts/jest/matchers/toWarnDev.js @@ -0,0 +1,65 @@ +'use strict'; + +function normalizeCodeLocInfo(str) { + return str && str.replace(/at .+?:\d+/g, 'at **'); +} + +module.exports = function toWarnDev(callback, expectedWarnings) { + if (!console.error.hasOwnProperty('calls')) { + spyOnDev(console, 'error'); + } + + callback(); + + if (__DEV__) { + if (typeof expectedWarnings === 'string') { + expectedWarnings = [expectedWarnings]; + } + + if (Array.isArray(expectedWarnings)) { + if (console.error.calls.count() !== expectedWarnings.length) { + return { + message: () => + `Expected number of DEV warnings:\n ${this.utils.printExpected( + expectedWarnings.length + )}\n` + + `Actual number of DEV warnings:\n ${this.utils.printReceived( + console.error.calls.count() + )}`, + pass: false, + }; + } + + const actualWarnings = []; + for (let i = 0; i < console.error.calls.count(); i++) { + actualWarnings.push( + normalizeCodeLocInfo(console.error.calls.argsFor(i)[0]) + ); + } + + const failure = expectedWarnings.find(expectedWarning => { + return !actualWarnings.includes(expectedWarning) + ? expectedWarning + : null; + }); + + if (failure) { + return { + message: () => + `Expected DEV warning:\n${this.utils.printExpected(failure)}\n` + + `Actual DEV warnings:\n${this.utils.printReceived( + actualWarnings.join('\n') + )}`, + pass: false, + }; + } else { + return {pass: true}; + } + } else { + throw Error( + `toWarnDev() requires a parameter of type string or an array of strings ` + + `but was given ${typeof expectedWarnings}.` + ); + } + } +}; diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index 06fd90c9d9e28..62fdb26cbc5e8 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -39,6 +39,10 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { global.spyOnDevAndProd = spyOn; } + expect.extend({ + toWarnDev: require('./matchers/toWarnDev'), + }); + ['error', 'warn'].forEach(methodName => { var oldMethod = console[methodName]; var newMethod = function() { From 2cf33ebef6f1b2ff8cf6f49e4814ddac273922ca Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 6 Dec 2017 08:48:46 -0800 Subject: [PATCH 02/17] Added .toLowPriorityWarnDev() matcher --- .../__tests__/ReactElementValidator-test.js | 382 +++++++----------- scripts/jest/matchers/toWarnDev.js | 127 +++--- scripts/jest/setupTests.js | 2 +- 3 files changed, 236 insertions(+), 275 deletions(-) diff --git a/packages/react/src/__tests__/ReactElementValidator-test.js b/packages/react/src/__tests__/ReactElementValidator-test.js index 440ab3b5bfdfc..0ff366b94e9fa 100644 --- a/packages/react/src/__tests__/ReactElementValidator-test.js +++ b/packages/react/src/__tests__/ReactElementValidator-test.js @@ -18,10 +18,6 @@ let ReactDOM; let ReactTestUtils; describe('ReactElementValidator', () => { - function normalizeCodeLocInfo(str) { - return str && str.replace(/at .+?:\d+/g, 'at **'); - } - let ComponentClass; beforeEach(() => { @@ -39,21 +35,16 @@ describe('ReactElementValidator', () => { }); it('warns for keys for arrays of elements in rest args', () => { - spyOnDev(console, 'error'); const Component = React.createFactory(ComponentClass); - Component(null, [Component(), Component()]); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Each child in an array or iterator should have a unique "key" prop.', - ); - } + expect(() => { + Component(null, [Component(), Component()]); + }).toWarnDev( + 'Each child in an array or iterator should have a unique "key" prop.', + ); }); it('warns for keys for arrays of elements with owner info', () => { - spyOnDev(console, 'error'); const Component = React.createFactory(ComponentClass); class InnerClass extends React.Component { @@ -70,59 +61,46 @@ describe('ReactElementValidator', () => { } } - ReactTestUtils.renderIntoDocument(React.createElement(ComponentWrapper)); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Each child in an array or iterator should have a unique "key" prop.' + - '\n\nCheck the render method of `InnerClass`. ' + - 'It was passed a child from ComponentWrapper. ', - ); - } + expect(() => { + ReactTestUtils.renderIntoDocument(React.createElement(ComponentWrapper)); + }).toWarnDev( + 'Each child in an array or iterator should have a unique "key" prop.' + + '\n\nCheck the render method of `InnerClass`. ' + + 'It was passed a child from ComponentWrapper. ', + ); }); it('warns for keys for arrays with no owner or parent info', () => { - spyOnDev(console, 'error'); - function Anonymous() { return
; } Object.defineProperty(Anonymous, 'name', {value: undefined}); const divs = [
,
]; - ReactTestUtils.renderIntoDocument({divs}); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: Each child in an array or iterator should have a unique ' + - '"key" prop. See https://fb.me/react-warning-keys for more information.\n' + - ' in div (at **)', - ); - } + expect(() => { + ReactTestUtils.renderIntoDocument({divs}); + }).toWarnDev( + 'Warning: Each child in an array or iterator should have a unique ' + + '"key" prop. See https://fb.me/react-warning-keys for more information.\n' + + ' in div (at **)', + ); }); it('warns for keys for arrays of elements with no owner info', () => { - spyOnDev(console, 'error'); - const divs = [
,
]; - ReactTestUtils.renderIntoDocument(
{divs}
); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: Each child in an array or iterator should have a unique ' + - '"key" prop.\n\nCheck the top-level render call using
. See ' + - 'https://fb.me/react-warning-keys for more information.\n' + - ' in div (at **)', - ); - } + expect(() => { + ReactTestUtils.renderIntoDocument(
{divs}
); + }).toWarnDev( + 'Warning: Each child in an array or iterator should have a unique ' + + '"key" prop.\n\nCheck the top-level render call using
. See ' + + 'https://fb.me/react-warning-keys for more information.\n' + + ' in div (at **)', + ); }); it('warns for keys with component stack info', () => { - spyOnDev(console, 'error'); - function Component() { return
{[
,
]}
; } @@ -135,20 +113,15 @@ describe('ReactElementValidator', () => { return } />; } - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: Each child in an array or iterator should have a unique ' + - '"key" prop.\n\nCheck the render method of `Component`. See ' + - 'https://fb.me/react-warning-keys for more information.\n' + - ' in div (at **)\n' + - ' in Component (at **)\n' + - ' in Parent (at **)\n' + - ' in GrandParent (at **)', - ); - } + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Warning: Each child in an array or iterator should have a unique ' + + '"key" prop.\n\nCheck the render method of `Component`. See ' + + 'https://fb.me/react-warning-keys for more information.\n' + + ' in div (at **)\n' + + ' in Component (at **)\n' + + ' in Parent (at **)\n' + + ' in GrandParent (at **)', + ); }); it('does not warn for keys when passing children down', () => { @@ -170,7 +143,6 @@ describe('ReactElementValidator', () => { }); it('warns for keys for iterables of elements in rest args', () => { - spyOnDev(console, 'error'); const Component = React.createFactory(ComponentClass); const iterable = { @@ -185,14 +157,9 @@ describe('ReactElementValidator', () => { }, }; - Component(null, iterable); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Each child in an array or iterator should have a unique "key" prop.', - ); - } + expect(() => Component(null, iterable)).toWarnDev( + 'Each child in an array or iterator should have a unique "key" prop.', + ); }); it('does not warns for arrays of elements with keys', () => { @@ -238,7 +205,6 @@ describe('ReactElementValidator', () => { // In this test, we're making sure that if a proptype error is found in a // component, we give a small hint as to which parent instantiated that // component as per warnings about key usage in ReactElementValidator. - spyOnDev(console, 'error'); function MyComp(props) { return React.createElement('div', null, 'My color is ' + props.color); } @@ -248,61 +214,48 @@ describe('ReactElementValidator', () => { function ParentComp() { return React.createElement(MyComp, {color: 123}); } - ReactTestUtils.renderIntoDocument(React.createElement(ParentComp)); - if (__DEV__) { - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Failed prop type: ' + - 'Invalid prop `color` of type `number` supplied to `MyComp`, ' + - 'expected `string`.\n' + - ' in MyComp (created by ParentComp)\n' + - ' in ParentComp', - ); - } + expect(() => { + ReactTestUtils.renderIntoDocument(React.createElement(ParentComp)); + }).toWarnDev( + 'Warning: Failed prop type: ' + + 'Invalid prop `color` of type `number` supplied to `MyComp`, ' + + 'expected `string`.\n' + + ' in MyComp (created by ParentComp)\n' + + ' in ParentComp', + ); }); it('gives a helpful error when passing invalid types', () => { - spyOnDev(console, 'error'); - React.createElement(undefined); - React.createElement(null); - React.createElement(true); - React.createElement({x: 17}); - React.createElement({}); - if (__DEV__) { - expect(console.error.calls.count()).toBe(5); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: React.createElement: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: undefined. You likely forgot to export your ' + - "component from the file it's defined in, or you might have mixed up " + - 'default and named imports.', - ); - expect(console.error.calls.argsFor(1)[0]).toBe( - 'Warning: React.createElement: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: null.', - ); - expect(console.error.calls.argsFor(2)[0]).toBe( - 'Warning: React.createElement: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: boolean.', - ); - expect(console.error.calls.argsFor(3)[0]).toBe( - 'Warning: React.createElement: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: object.', - ); - expect(console.error.calls.argsFor(4)[0]).toBe( - 'Warning: React.createElement: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: object. You likely forgot to export your ' + - "component from the file it's defined in, or you might have mixed up " + - 'default and named imports.', - ); - } - React.createElement('div'); - if (__DEV__) { - expect(console.error.calls.count()).toBe(5); - } + expect(() => { + React.createElement(undefined); + React.createElement(null); + React.createElement(true); + React.createElement({x: 17}); + React.createElement({}); + }).toWarnDev([ + 'Warning: React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: undefined. You likely forgot to export your ' + + "component from the file it's defined in, or you might have mixed up " + + 'default and named imports.', + 'Warning: React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: null.', + 'Warning: React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: boolean.', + 'Warning: React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: object.', + 'Warning: React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: object. You likely forgot to export your ' + + "component from the file it's defined in, or you might have mixed up " + + 'default and named imports.', + ]); + + // Should not log any additional warnings + expect(() => React.createElement('div')).toWarnDev([]); }); it('includes the owner name when passing null, undefined, boolean, or number', () => { @@ -317,6 +270,8 @@ describe('ReactElementValidator', () => { 'or a class/function (for composite components) but got: null.' + (__DEV__ ? '\n\nCheck the render method of `ParentComp`.' : ''), ); + + // We can't use .toWarnDev() here because we can't chain it with .toThrowError() if (__DEV__) { expect(console.error.calls.count()).toBe(1); expect(console.error.calls.argsFor(0)[0]).toBe( @@ -329,8 +284,6 @@ describe('ReactElementValidator', () => { }); it('should check default prop values', () => { - spyOnDev(console, 'error'); - class Component extends React.Component { static propTypes = {prop: PropTypes.string.isRequired}; static defaultProps = {prop: null}; @@ -339,21 +292,16 @@ describe('ReactElementValidator', () => { } } - ReactTestUtils.renderIntoDocument(React.createElement(Component)); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Failed prop type: The prop `prop` is marked as required in ' + - '`Component`, but its value is `null`.\n' + - ' in Component', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument(React.createElement(Component)), + ).toWarnDev( + 'Warning: Failed prop type: The prop `prop` is marked as required in ' + + '`Component`, but its value is `null`.\n' + + ' in Component', + ); }); it('should not check the default for explicit null', () => { - spyOnDev(console, 'error'); - class Component extends React.Component { static propTypes = {prop: PropTypes.string.isRequired}; static defaultProps = {prop: 'text'}; @@ -362,23 +310,18 @@ describe('ReactElementValidator', () => { } } - ReactTestUtils.renderIntoDocument( - React.createElement(Component, {prop: null}), - ); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Failed prop type: The prop `prop` is marked as required in ' + - '`Component`, but its value is `null`.\n' + - ' in Component', + expect(() => { + ReactTestUtils.renderIntoDocument( + React.createElement(Component, {prop: null}), ); - } + }).toWarnDev( + 'Warning: Failed prop type: The prop `prop` is marked as required in ' + + '`Component`, but its value is `null`.\n' + + ' in Component', + ); }); it('should check declared prop types', () => { - spyOnDev(console, 'error'); - class Component extends React.Component { static propTypes = { prop: PropTypes.string.isRequired, @@ -388,36 +331,28 @@ describe('ReactElementValidator', () => { } } - ReactTestUtils.renderIntoDocument(React.createElement(Component)); - ReactTestUtils.renderIntoDocument( - React.createElement(Component, {prop: 42}), - ); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(2); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Failed prop type: ' + - 'The prop `prop` is marked as required in `Component`, but its value ' + - 'is `undefined`.\n' + - ' in Component', + expect(() => { + ReactTestUtils.renderIntoDocument(React.createElement(Component)); + ReactTestUtils.renderIntoDocument( + React.createElement(Component, {prop: 42}), ); - - expect(console.error.calls.argsFor(1)[0]).toBe( - 'Warning: Failed prop type: ' + - 'Invalid prop `prop` of type `number` supplied to ' + - '`Component`, expected `string`.\n' + - ' in Component', + }).toWarnDev([ + 'Warning: Failed prop type: ' + + 'The prop `prop` is marked as required in `Component`, but its value ' + + 'is `undefined`.\n' + + ' in Component', + 'Warning: Failed prop type: ' + + 'Invalid prop `prop` of type `number` supplied to ' + + '`Component`, expected `string`.\n' + + ' in Component', + ]); + + // Should not error for strings + expect(() => { + ReactTestUtils.renderIntoDocument( + React.createElement(Component, {prop: 'string'}), ); - } - - ReactTestUtils.renderIntoDocument( - React.createElement(Component, {prop: 'string'}), - ); - - if (__DEV__) { - // Should not error for strings - expect(console.error.calls.count()).toBe(2); - } + }).toWarnDev([]); }); it('should warn if a PropType creator is used as a PropType', () => { @@ -432,24 +367,20 @@ describe('ReactElementValidator', () => { } } - ReactTestUtils.renderIntoDocument( - React.createElement(Component, {myProp: {value: 'hi'}}), - ); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Component: type specification of prop `myProp` is invalid; ' + - 'the type checker function must return `null` or an `Error` but ' + - 'returned a function. You may have forgotten to pass an argument to ' + - 'the type checker creator (arrayOf, instanceOf, objectOf, oneOf, ' + - 'oneOfType, and shape all require an argument).', + expect(() => { + ReactTestUtils.renderIntoDocument( + React.createElement(Component, {myProp: {value: 'hi'}}), ); - } + }).toWarnDev( + 'Warning: Component: type specification of prop `myProp` is invalid; ' + + 'the type checker function must return `null` or an `Error` but ' + + 'returned a function. You may have forgotten to pass an argument to ' + + 'the type checker creator (arrayOf, instanceOf, objectOf, oneOf, ' + + 'oneOfType, and shape all require an argument).', + ); }); it('should warn if component declares PropTypes instead of propTypes', () => { - spyOnDevAndProd(console, 'error'); class MisspelledPropTypesComponent extends React.Component { static PropTypes = { prop: PropTypes.string, @@ -459,37 +390,33 @@ describe('ReactElementValidator', () => { } } - ReactTestUtils.renderIntoDocument( - React.createElement(MisspelledPropTypesComponent, {prop: 'Hi'}), - ); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Component MisspelledPropTypesComponent declared `PropTypes` ' + - 'instead of `propTypes`. Did you misspell the property assignment?', + expect(() => { + ReactTestUtils.renderIntoDocument( + React.createElement(MisspelledPropTypesComponent, {prop: 'Hi'}), ); - } + }).toWarnDev( + 'Warning: Component MisspelledPropTypesComponent declared `PropTypes` ' + + 'instead of `propTypes`. Did you misspell the property assignment?', + ); }); it('should warn when accessing .type on an element factory', () => { - spyOnDev(console, 'warn'); function TestComponent() { return
; } - const TestFactory = React.createFactory(TestComponent); - expect(TestFactory.type).toBe(TestComponent); - if (__DEV__) { - expect(console.warn.calls.count()).toBe(1); - expect(console.warn.calls.argsFor(0)[0]).toBe( - 'Warning: Factory.type is deprecated. Access the class directly before ' + - 'passing it to createFactory.', - ); - } + + let TestFactory = React.createFactory(TestComponent); + expect(() => + expect(TestFactory.type).toBe(TestComponent), + ).toLowPriorityWarnDev( + 'Warning: Factory.type is deprecated. Access the class directly before ' + + 'passing it to createFactory.', + ); + // Warn once, not again - expect(TestFactory.type).toBe(TestComponent); - if (__DEV__) { - expect(console.warn.calls.count()).toBe(1); - } + expect(() => + expect(TestFactory.type).toBe(TestComponent), + ).toLowPriorityWarnDev([]); }); it('does not warn when using DOM node as children', () => { @@ -544,18 +471,15 @@ describe('ReactElementValidator', () => { }); it('does not blow up on key warning with undefined type', () => { - spyOnDev(console, 'error'); const Foo = undefined; - void {[
]}; - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: React.createElement: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: undefined. You likely forgot to export your ' + - "component from the file it's defined in, or you might have mixed up " + - 'default and named imports.\n\nCheck your code at **.', - ); - } + expect(() => { + void {[
]}; + }).toWarnDev( + 'Warning: React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: undefined. You likely forgot to export your ' + + "component from the file it's defined in, or you might have mixed up " + + 'default and named imports.\n\nCheck your code at **.', + ); }); }); diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js index bc2a599729384..0f464d8349ea7 100644 --- a/scripts/jest/matchers/toWarnDev.js +++ b/scripts/jest/matchers/toWarnDev.js @@ -4,62 +4,99 @@ function normalizeCodeLocInfo(str) { return str && str.replace(/at .+?:\d+/g, 'at **'); } -module.exports = function toWarnDev(callback, expectedWarnings) { - if (!console.error.hasOwnProperty('calls')) { - spyOnDev(console, 'error'); - } - - callback(); - +function validator(consoleSpy, expectedWarnings) { if (__DEV__) { if (typeof expectedWarnings === 'string') { expectedWarnings = [expectedWarnings]; + } else if (!Array.isArray(expectedWarnings)) { + throw Error( + `toWarnDev() requires a parameter of type string or an array of strings ` + + `but was given ${typeof expectedWarnings}.` + ); } - if (Array.isArray(expectedWarnings)) { - if (console.error.calls.count() !== expectedWarnings.length) { - return { - message: () => - `Expected number of DEV warnings:\n ${this.utils.printExpected( - expectedWarnings.length - )}\n` + - `Actual number of DEV warnings:\n ${this.utils.printReceived( - console.error.calls.count() - )}`, - pass: false, - }; + if (consoleSpy.calls.count() !== expectedWarnings.length) { + return { + message: () => + `Expected number of DEV warnings:\n ${this.utils.printExpected( + expectedWarnings.length + )}\n` + + `Actual number of DEV warnings:\n ${this.utils.printReceived( + consoleSpy.calls.count() + )}`, + pass: false, + }; + } + + // Normalize warnings for easier comparison + const actualWarnings = []; + for (let i = 0; i < consoleSpy.calls.count(); i++) { + actualWarnings.push(normalizeCodeLocInfo(consoleSpy.calls.argsFor(i)[0])); + } + + let failedExpectation; + for (let i = 0; i < expectedWarnings.length; i++) { + const expectedWarning = expectedWarnings[i]; + let found = false; + + for (let x = 0; x < expectedWarnings.length; x++) { + const actualWarning = actualWarnings[x]; + + // Allow partial matching. + if ( + actualWarning === expectedWarning || + actualWarning.includes(expectedWarning) + ) { + found = true; + break; + } } - const actualWarnings = []; - for (let i = 0; i < console.error.calls.count(); i++) { - actualWarnings.push( - normalizeCodeLocInfo(console.error.calls.argsFor(i)[0]) - ); + if (!found) { + failedExpectation = expectedWarning; + break; } + } - const failure = expectedWarnings.find(expectedWarning => { - return !actualWarnings.includes(expectedWarning) - ? expectedWarning - : null; - }); + if (failedExpectation) { + return { + message: () => + `Expected DEV warning:\n${this.utils.printExpected( + failedExpectation + )}\n` + + `Actual DEV warnings:\n${this.utils.printReceived( + actualWarnings.join('\n') + )}`, + pass: false, + }; + } + } + + return {pass: true}; +} - if (failure) { - return { - message: () => - `Expected DEV warning:\n${this.utils.printExpected(failure)}\n` + - `Actual DEV warnings:\n${this.utils.printReceived( - actualWarnings.join('\n') - )}`, - pass: false, - }; +const createMatcherFor = consoleMethod => + function(callback, expectedWarnings) { + if (__DEV__) { + if (!console[consoleMethod].hasOwnProperty('calls')) { + spyOnDev(console, consoleMethod); } else { - return {pass: true}; + console[consoleMethod].calls.reset(); } - } else { - throw Error( - `toWarnDev() requires a parameter of type string or an array of strings ` + - `but was given ${typeof expectedWarnings}.` - ); } - } + + callback(); + + const response = validator.call( + this, + console[consoleMethod], + expectedWarnings + ); + + return response; + }; + +module.exports = { + toLowPriorityWarnDev: createMatcherFor('warn'), + toWarnDev: createMatcherFor('error'), }; diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index 62fdb26cbc5e8..2c91fcfa8a848 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -40,7 +40,7 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { } expect.extend({ - toWarnDev: require('./matchers/toWarnDev'), + ...require('./matchers/toWarnDev'), }); ['error', 'warn'].forEach(methodName => { From 472f5e34fdf771fd7f33642c2cee77110dd73632 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 18 Dec 2017 10:59:04 -0800 Subject: [PATCH 03/17] Nest toThrow and toWarn matchers in ReactElementValidator test --- .../__tests__/ReactElementValidator-test.js | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/packages/react/src/__tests__/ReactElementValidator-test.js b/packages/react/src/__tests__/ReactElementValidator-test.js index 0ff366b94e9fa..ce5690be3e204 100644 --- a/packages/react/src/__tests__/ReactElementValidator-test.js +++ b/packages/react/src/__tests__/ReactElementValidator-test.js @@ -259,28 +259,24 @@ describe('ReactElementValidator', () => { }); it('includes the owner name when passing null, undefined, boolean, or number', () => { - spyOnDev(console, 'error'); function ParentComp() { return React.createElement(null); } - expect(function() { - ReactTestUtils.renderIntoDocument(React.createElement(ParentComp)); - }).toThrowError( - 'Element type is invalid: expected a string (for built-in components) ' + - 'or a class/function (for composite components) but got: null.' + - (__DEV__ ? '\n\nCheck the render method of `ParentComp`.' : ''), - ); - // We can't use .toWarnDev() here because we can't chain it with .toThrowError() - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: React.createElement: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: null.' + - '\n\nCheck the render method of `ParentComp`.\n in ParentComp', + expect(() => { + expect(() => { + ReactTestUtils.renderIntoDocument(React.createElement(ParentComp)); + }).toThrowError( + 'Element type is invalid: expected a string (for built-in components) ' + + 'or a class/function (for composite components) but got: null.' + + (__DEV__ ? '\n\nCheck the render method of `ParentComp`.' : ''), ); - } + }).toWarnDev( + 'Warning: React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: null.' + + '\n\nCheck the render method of `ParentComp`.\n in ParentComp', + ); }); it('should check default prop values', () => { From d8d0f66385a66a0022aa645b4363be7019488bbc Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 18 Dec 2017 11:24:12 -0800 Subject: [PATCH 04/17] Reply Jest spy with custom spy. Unregister spy after toWarnDev() so unexpected console.error/warn calls will fail tests. --- .../__tests__/ReactElementValidator-test.js | 18 +++---- scripts/jest/matchers/toWarnDev.js | 48 ++++++++++++------- 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/packages/react/src/__tests__/ReactElementValidator-test.js b/packages/react/src/__tests__/ReactElementValidator-test.js index ce5690be3e204..61efff91e74b2 100644 --- a/packages/react/src/__tests__/ReactElementValidator-test.js +++ b/packages/react/src/__tests__/ReactElementValidator-test.js @@ -255,7 +255,7 @@ describe('ReactElementValidator', () => { ]); // Should not log any additional warnings - expect(() => React.createElement('div')).toWarnDev([]); + React.createElement('div'); }); it('includes the owner name when passing null, undefined, boolean, or number', () => { @@ -344,11 +344,9 @@ describe('ReactElementValidator', () => { ]); // Should not error for strings - expect(() => { - ReactTestUtils.renderIntoDocument( - React.createElement(Component, {prop: 'string'}), - ); - }).toWarnDev([]); + ReactTestUtils.renderIntoDocument( + React.createElement(Component, {prop: 'string'}), + ); }); it('should warn if a PropType creator is used as a PropType', () => { @@ -402,17 +400,13 @@ describe('ReactElementValidator', () => { } let TestFactory = React.createFactory(TestComponent); - expect(() => - expect(TestFactory.type).toBe(TestComponent), - ).toLowPriorityWarnDev( + expect(() => TestFactory.type).toLowPriorityWarnDev( 'Warning: Factory.type is deprecated. Access the class directly before ' + 'passing it to createFactory.', ); // Warn once, not again - expect(() => - expect(TestFactory.type).toBe(TestComponent), - ).toLowPriorityWarnDev([]); + expect(TestFactory.type).toBe(TestComponent); }); it('does not warn when using DOM node as children', () => { diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js index 0f464d8349ea7..71e64f617cc8f 100644 --- a/scripts/jest/matchers/toWarnDev.js +++ b/scripts/jest/matchers/toWarnDev.js @@ -1,5 +1,17 @@ 'use strict'; +function createSpy() { + let calls = []; + + function spy(...args) { + calls.push(args); + } + + spy.calls = calls; + + return spy; +} + function normalizeCodeLocInfo(str) { return str && str.replace(/at .+?:\d+/g, 'at **'); } @@ -15,14 +27,14 @@ function validator(consoleSpy, expectedWarnings) { ); } - if (consoleSpy.calls.count() !== expectedWarnings.length) { + if (consoleSpy.calls.length !== expectedWarnings.length) { return { message: () => `Expected number of DEV warnings:\n ${this.utils.printExpected( expectedWarnings.length )}\n` + `Actual number of DEV warnings:\n ${this.utils.printReceived( - consoleSpy.calls.count() + consoleSpy.calls.length )}`, pass: false, }; @@ -30,8 +42,8 @@ function validator(consoleSpy, expectedWarnings) { // Normalize warnings for easier comparison const actualWarnings = []; - for (let i = 0; i < consoleSpy.calls.count(); i++) { - actualWarnings.push(normalizeCodeLocInfo(consoleSpy.calls.argsFor(i)[0])); + for (let i = 0; i < consoleSpy.calls.length; i++) { + actualWarnings.push(normalizeCodeLocInfo(consoleSpy.calls[i][0])); } let failedExpectation; @@ -78,22 +90,24 @@ function validator(consoleSpy, expectedWarnings) { const createMatcherFor = consoleMethod => function(callback, expectedWarnings) { if (__DEV__) { - if (!console[consoleMethod].hasOwnProperty('calls')) { - spyOnDev(console, consoleMethod); - } else { - console[consoleMethod].calls.reset(); - } - } + let originalMethod = console[consoleMethod]; - callback(); + // Avoid using Jest's built-in spy since it can't be removed. + console[consoleMethod] = createSpy(); - const response = validator.call( - this, - console[consoleMethod], - expectedWarnings - ); + try { + callback(); - return response; + return validator.call(this, console[consoleMethod], expectedWarnings); + } finally { + // Restore the unspied method so that unexpected errors fail tests. + console[consoleMethod] = originalMethod; + } + } else { + callback(); + + return {pass: true}; + } }; module.exports = { From 1af3d1e1fbddfed3f3ed2a0e6abeaa0f571e2efb Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 18 Dec 2017 11:38:34 -0800 Subject: [PATCH 05/17] console warn/error throws immediately in tests by default (if not spied on) --- scripts/jest/setupTests.js | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index 6013a65da3225..46b5763e2284c 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -62,15 +62,17 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { ['error', 'warn'].forEach(methodName => { const oldMethod = console[methodName]; const newMethod = function() { - newMethod.__callCount++; - oldMethod.apply(this, arguments); + throw new Error( + `Expected test not to call console.${methodName}(). ` + + 'If the warning is expected, mock it out using ' + + `spyOnDev(console, '${methodName}') or spyOnProd(console, '${ + methodName + }'), ` + + 'and test that the warning occurs.' + ); }; - newMethod.__callCount = 0; - console[methodName] = newMethod; - env.beforeEach(() => { - newMethod.__callCount = 0; - }); + console[methodName] = newMethod; env.afterEach(() => { if (console[methodName] !== newMethod && !isSpy(console[methodName])) { @@ -78,16 +80,6 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { `Test did not tear down console.${methodName} mock properly.` ); } - if (console[methodName].__callCount !== 0) { - throw new Error( - `Expected test not to call console.${methodName}(). ` + - 'If the warning is expected, mock it out using ' + - `spyOnDev(console, '${methodName}') or spyOnProd(console, '${ - methodName - }'), ` + - 'and test that the warning occurs.' - ); - } }); }); From e6ff0da1f14c6c608fbace1856926c2913a0c48b Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 18 Dec 2017 12:36:05 -0800 Subject: [PATCH 06/17] Removed unused oldMethod --- scripts/jest/setupTests.js | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index 46b5763e2284c..6d612308e709f 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -60,7 +60,6 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { }); ['error', 'warn'].forEach(methodName => { - const oldMethod = console[methodName]; const newMethod = function() { throw new Error( `Expected test not to call console.${methodName}(). ` + From 0662fd0e1356ac260341127dfacc8210a40a121a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 18 Dec 2017 17:30:08 -0800 Subject: [PATCH 07/17] Refactored toWarn matchers to fail faster --- scripts/jest/matchers/toWarnDev.js | 128 ++++++++++------------------- 1 file changed, 44 insertions(+), 84 deletions(-) diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js index 71e64f617cc8f..f85aa0092b52a 100644 --- a/scripts/jest/matchers/toWarnDev.js +++ b/scripts/jest/matchers/toWarnDev.js @@ -1,109 +1,69 @@ 'use strict'; -function createSpy() { - let calls = []; - - function spy(...args) { - calls.push(args); - } - - spy.calls = calls; - - return spy; -} - function normalizeCodeLocInfo(str) { return str && str.replace(/at .+?:\d+/g, 'at **'); } -function validator(consoleSpy, expectedWarnings) { - if (__DEV__) { - if (typeof expectedWarnings === 'string') { - expectedWarnings = [expectedWarnings]; - } else if (!Array.isArray(expectedWarnings)) { - throw Error( - `toWarnDev() requires a parameter of type string or an array of strings ` + - `but was given ${typeof expectedWarnings}.` - ); - } +// TODO Consider the use-case of nested toWarn statements +// Throw immediately if detected? Best if possible. +// Unless we use this pattern and need it, in which case, maybe it would "just work" as is. - if (consoleSpy.calls.length !== expectedWarnings.length) { - return { - message: () => - `Expected number of DEV warnings:\n ${this.utils.printExpected( - expectedWarnings.length - )}\n` + - `Actual number of DEV warnings:\n ${this.utils.printReceived( - consoleSpy.calls.length - )}`, - pass: false, - }; - } - - // Normalize warnings for easier comparison - const actualWarnings = []; - for (let i = 0; i < consoleSpy.calls.length; i++) { - actualWarnings.push(normalizeCodeLocInfo(consoleSpy.calls[i][0])); - } - - let failedExpectation; - for (let i = 0; i < expectedWarnings.length; i++) { - const expectedWarning = expectedWarnings[i]; - let found = false; - - for (let x = 0; x < expectedWarnings.length; x++) { - const actualWarning = actualWarnings[x]; +const createMatcherFor = consoleMethod => + function matcher(callback, expectedMessages) { + if (__DEV__) { + // Warn about incorrect usage of matcher. + if (typeof expectedMessages === 'string') { + expectedMessages = [expectedMessages]; + } else if (!Array.isArray(expectedMessages)) { + throw Error( + `toWarnDev() requires a parameter of type string or an array of strings ` + + `but was given ${typeof expectedMessages}.` + ); + } - // Allow partial matching. - if ( - actualWarning === expectedWarning || - actualWarning.includes(expectedWarning) - ) { - found = true; - break; + function consoleSpy(message) { + const normalizedMessage = normalizeCodeLocInfo(message); + + for (let index = 0; index < expectedMessages.length; index++) { + const expectedMessage = expectedMessages[index]; + if ( + normalizedMessage === expectedMessage || + normalizedMessage.includes(expectedMessage) + ) { + expectedMessages.splice(index, 1); + return; + } } - } - if (!found) { - failedExpectation = expectedWarning; - break; + // Fail early for unexpected warnings to preserve the call stack. + throw Error(`Unexpected warning recorded: "${message}"`); } - } - - if (failedExpectation) { - return { - message: () => - `Expected DEV warning:\n${this.utils.printExpected( - failedExpectation - )}\n` + - `Actual DEV warnings:\n${this.utils.printReceived( - actualWarnings.join('\n') - )}`, - pass: false, - }; - } - } - - return {pass: true}; -} - -const createMatcherFor = consoleMethod => - function(callback, expectedWarnings) { - if (__DEV__) { - let originalMethod = console[consoleMethod]; // Avoid using Jest's built-in spy since it can't be removed. - console[consoleMethod] = createSpy(); + const originalMethod = console[consoleMethod]; + console[consoleMethod] = consoleSpy; try { callback(); - return validator.call(this, console[consoleMethod], expectedWarnings); + // Any remaining messages indicate a failed expectations. + if (expectedMessages.length > 0) { + return { + message: () => + `Expected warning was not recorded:\n ${this.utils.printReceived( + expectedMessages.join('\n') + )}`, + pass: false, + }; + } + + return {pass: true}; } finally { // Restore the unspied method so that unexpected errors fail tests. console[consoleMethod] = originalMethod; } } else { + // Any uncaught errors or warnings should fail tests in production mode. callback(); return {pass: true}; From 36c6550ee75c9dc52ebe9eb44bacfc76b38f4683 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 18 Dec 2017 19:11:23 -0800 Subject: [PATCH 08/17] Added TODO about Jest overriding error stacks --- scripts/jest/matchers/toWarnDev.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js index f85aa0092b52a..4067061e63117 100644 --- a/scripts/jest/matchers/toWarnDev.js +++ b/scripts/jest/matchers/toWarnDev.js @@ -21,7 +21,7 @@ const createMatcherFor = consoleMethod => ); } - function consoleSpy(message) { + const consoleSpy = message => { const normalizedMessage = normalizeCodeLocInfo(message); for (let index = 0; index < expectedMessages.length; index++) { @@ -37,7 +37,7 @@ const createMatcherFor = consoleMethod => // Fail early for unexpected warnings to preserve the call stack. throw Error(`Unexpected warning recorded: "${message}"`); - } + }; // Avoid using Jest's built-in spy since it can't be removed. const originalMethod = console[consoleMethod]; @@ -58,6 +58,10 @@ const createMatcherFor = consoleMethod => } return {pass: true}; + } catch (error) { + // TODO Flag this error so Jest doesn't override its stack + // See https://tinyurl.com/y9unakwb + throw error; } finally { // Restore the unspied method so that unexpected errors fail tests. console[consoleMethod] = originalMethod; From feac5596aedab3517f5ba615e16621f343a6b316 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 18 Dec 2017 19:43:47 -0800 Subject: [PATCH 09/17] Moved TODO --- scripts/jest/matchers/toWarnDev.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js index 4067061e63117..ebc070bf1dd91 100644 --- a/scripts/jest/matchers/toWarnDev.js +++ b/scripts/jest/matchers/toWarnDev.js @@ -4,10 +4,6 @@ function normalizeCodeLocInfo(str) { return str && str.replace(/at .+?:\d+/g, 'at **'); } -// TODO Consider the use-case of nested toWarn statements -// Throw immediately if detected? Best if possible. -// Unless we use this pattern and need it, in which case, maybe it would "just work" as is. - const createMatcherFor = consoleMethod => function matcher(callback, expectedMessages) { if (__DEV__) { @@ -39,8 +35,12 @@ const createMatcherFor = consoleMethod => throw Error(`Unexpected warning recorded: "${message}"`); }; - // Avoid using Jest's built-in spy since it can't be removed. + // TODO Decide whether we need to support nested toWarn* expectations. + // If we don't need id, add a check here to see if this is already our spy, + // And throw an error. const originalMethod = console[consoleMethod]; + + // Avoid using Jest's built-in spy since it can't be removed. console[consoleMethod] = consoleSpy; try { From 47fa899eefac0033de4ff1b68ce07ae4922ed01b Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 18 Dec 2017 20:24:27 -0800 Subject: [PATCH 10/17] Pass-thru console message before erroring to make it easier to identify --- scripts/jest/setupTests.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index 6d612308e709f..4e6f7373c6c17 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -60,7 +60,10 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { }); ['error', 'warn'].forEach(methodName => { - const newMethod = function() { + const oldMethod = console[methodName]; + const newMethod = function(...args) { + oldMethod(...args); + throw new Error( `Expected test not to call console.${methodName}(). ` + 'If the warning is expected, mock it out using ' + From cdb1b503661deb4f96b164fdddbb2759048dc5c1 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 18 Dec 2017 20:42:43 -0800 Subject: [PATCH 11/17] Updated a few more tests to use toWarnDev matcher --- .../react/src/__tests__/ReactChildren-test.js | 64 +--- .../ReactCoffeeScriptClass-test.coffee | 101 ++--- .../__tests__/ReactContextValidator-test.js | 145 +++---- .../react/src/__tests__/ReactES6Class-test.js | 112 ++---- .../react/src/__tests__/ReactElement-test.js | 60 +-- .../src/__tests__/ReactElementClone-test.js | 38 +- .../ReactJSXElementValidator-test.js | 353 +++++++----------- .../createReactClassIntegration-test.js | 279 ++++++-------- 8 files changed, 432 insertions(+), 720 deletions(-) diff --git a/packages/react/src/__tests__/ReactChildren-test.js b/packages/react/src/__tests__/ReactChildren-test.js index e9d4948f6c4e3..ce639254340cf 100644 --- a/packages/react/src/__tests__/ReactChildren-test.js +++ b/packages/react/src/__tests__/ReactChildren-test.js @@ -13,10 +13,6 @@ describe('ReactChildren', () => { let React; let ReactTestUtils; - function normalizeCodeLocInfo(str) { - return str && str.replace(/at .+?:\d+/g, 'at **'); - } - beforeEach(() => { jest.resetModules(); React = require('react'); @@ -352,7 +348,6 @@ describe('ReactChildren', () => { }); it('should be called for each child in an iterable without keys', () => { - spyOnDev(console, 'error'); const threeDivIterable = { '@@iterator': function() { let i = 0; @@ -374,7 +369,10 @@ describe('ReactChildren', () => { return kid; }); - const instance =
{threeDivIterable}
; + let instance; + expect(() => (instance =
{threeDivIterable}
)).toWarnDev( + 'Warning: Each child in an array or iterator should have a unique "key" prop.', + ); function assertCalls() { expect(callback.calls.count()).toBe(3); @@ -386,13 +384,6 @@ describe('ReactChildren', () => { React.Children.forEach(instance.props.children, callback, context); assertCalls(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Each child in an array or iterator should have a unique "key" prop.', - ); - console.error.calls.reset(); - } const mappedChildren = React.Children.map( instance.props.children, @@ -400,9 +391,6 @@ describe('ReactChildren', () => { context, ); assertCalls(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(0); - } expect(mappedChildren).toEqual([
,
, @@ -958,28 +946,23 @@ describe('ReactChildren', () => { describe('with fragments enabled', () => { it('warns for keys for arrays of elements in a fragment', () => { - spyOnDev(console, 'error'); class ComponentReturningArray extends React.Component { render() { return [
,
]; } } - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: ' + - 'Each child in an array or iterator should have a unique "key" prop.' + - ' See https://fb.me/react-warning-keys for more information.' + - '\n in ComponentReturningArray (at **)', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev( + 'Warning: ' + + 'Each child in an array or iterator should have a unique "key" prop.' + + ' See https://fb.me/react-warning-keys for more information.' + + '\n in ComponentReturningArray (at **)', + ); }); it('does not warn when there are keys on elements in a fragment', () => { - spyOnDev(console, 'error'); class ComponentReturningArray extends React.Component { render() { return [
,
]; @@ -987,25 +970,16 @@ describe('ReactChildren', () => { } ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(0); - } }); it('warns for keys for arrays at the top level', () => { - spyOnDev(console, 'error'); - - ReactTestUtils.renderIntoDocument([
,
]); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: ' + - 'Each child in an array or iterator should have a unique "key" prop.' + - ' See https://fb.me/react-warning-keys for more information.', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument([
,
]), + ).toWarnDev( + 'Warning: ' + + 'Each child in an array or iterator should have a unique "key" prop.' + + ' See https://fb.me/react-warning-keys for more information.', + ); }); }); }); diff --git a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee index 865487aaaa838..5312809363031 100644 --- a/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee +++ b/packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee @@ -46,14 +46,12 @@ describe 'ReactCoffeeScriptClass', -> expect(Foo.name).toBe 'Foo' it 'throws if no render function is defined', -> - spyOnDev console, 'error' class Foo extends React.Component expect(-> - ReactDOM.render React.createElement(Foo), container - ).toThrow() - if __DEV__ - expect(console.error.calls.count()).toBe(1) - expect(console.error.calls.argsFor(0)[0]).toContain('No `render` method found on the returned component instance') + expect(-> + ReactDOM.render React.createElement(Foo), container + ).toThrow() + ).toWarnDev('No `render` method found on the returned component instance') undefined it 'renders a simple stateless component with prop', -> @@ -150,8 +148,6 @@ describe 'ReactCoffeeScriptClass', -> undefined it 'should warn with non-object in the initial state property', -> - spyOnDev console, 'error' - [['an array'], 'a string', 1234].forEach (state) -> class Foo extends React.Component constructor: -> @@ -160,14 +156,9 @@ describe 'ReactCoffeeScriptClass', -> render: -> span() - test React.createElement(Foo), 'SPAN', '' - if __DEV__ - expect(console.error.calls.count()).toBe 1 - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Foo.state: must be set to an object or null' - ) - console.error.calls.reset() - + expect(-> + test React.createElement(Foo), 'SPAN', '' + ).toWarnDev('Foo.state: must be set to an object or null') undefined it 'should render with null in the initial state property', -> @@ -287,7 +278,6 @@ describe 'ReactCoffeeScriptClass', -> it 'warns when classic properties are defined on the instance, but does not invoke them.', -> - spyOnDev console, 'error' getInitialStateWasCalled = false getDefaultPropsWasCalled = false class Foo extends React.Component @@ -307,28 +297,20 @@ describe 'ReactCoffeeScriptClass', -> span className: 'foo' - test React.createElement(Foo), 'SPAN', 'foo' + expect(-> + test React.createElement(Foo), 'SPAN', 'foo' + ).toWarnDev([ + 'getInitialState was defined on Foo, a plain JavaScript class.', + 'getDefaultProps was defined on Foo, a plain JavaScript class.', + 'propTypes was defined as an instance property on Foo.', + 'contextTypes was defined as an instance property on Foo.', + ]) expect(getInitialStateWasCalled).toBe false expect(getDefaultPropsWasCalled).toBe false - if __DEV__ - expect(console.error.calls.count()).toBe 4 - expect(console.error.calls.argsFor(0)[0]).toContain( - 'getInitialState was defined on Foo, a plain JavaScript class.' - ) - expect(console.error.calls.argsFor(1)[0]).toContain( - 'getDefaultProps was defined on Foo, a plain JavaScript class.' - ) - expect(console.error.calls.argsFor(2)[0]).toContain( - 'propTypes was defined as an instance property on Foo.' - ) - expect(console.error.calls.argsFor(3)[0]).toContain( - 'contextTypes was defined as an instance property on Foo.' - ) undefined it 'does not warn about getInitialState() on class components if state is also defined.', -> - spyOnDev console, 'error' class Foo extends React.Component constructor: (props) -> super props @@ -342,12 +324,9 @@ describe 'ReactCoffeeScriptClass', -> className: 'foo' test React.createElement(Foo), 'SPAN', 'foo' - if __DEV__ - expect(console.error.calls.count()).toBe 0 undefined it 'should warn when misspelling shouldComponentUpdate', -> - spyOnDev console, 'error' class NamedComponent extends React.Component componentShouldUpdate: -> false @@ -356,18 +335,16 @@ describe 'ReactCoffeeScriptClass', -> span className: 'foo' - test React.createElement(NamedComponent), 'SPAN', 'foo' - if __DEV__ - expect(console.error.calls.count()).toBe 1 - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: NamedComponent has a method called componentShouldUpdate(). - Did you mean shouldComponentUpdate()? The name is phrased as a - question because the function is expected to return a value.' - ) + expect(-> + test React.createElement(NamedComponent), 'SPAN', 'foo' + ).toWarnDev( + 'Warning: NamedComponent has a method called componentShouldUpdate(). + Did you mean shouldComponentUpdate()? The name is phrased as a + question because the function is expected to return a value.' + ) undefined it 'should warn when misspelling componentWillReceiveProps', -> - spyOnDev console, 'error' class NamedComponent extends React.Component componentWillRecieveProps: -> false @@ -376,29 +353,27 @@ describe 'ReactCoffeeScriptClass', -> span className: 'foo' - test React.createElement(NamedComponent), 'SPAN', 'foo' - if __DEV__ - expect(console.error.calls.count()).toBe 1 - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: NamedComponent has a method called componentWillRecieveProps(). - Did you mean componentWillReceiveProps()?' - ) + expect(-> + test React.createElement(NamedComponent), 'SPAN', 'foo' + ).toWarnDev( + 'Warning: NamedComponent has a method called componentWillRecieveProps(). + Did you mean componentWillReceiveProps()?' + ) undefined it 'should throw AND warn when trying to access classic APIs', -> - spyOnDev console, 'warn' instance = test Inner(name: 'foo'), 'DIV', 'foo' - expect(-> instance.replaceState {}).toThrow() - expect(-> instance.isMounted()).toThrow() - if __DEV__ - expect(console.warn.calls.count()).toBe 2 - expect(console.warn.calls.argsFor(0)[0]).toContain( - 'replaceState(...) is deprecated in plain JavaScript React classes' - ) - expect(console.warn.calls.argsFor(1)[0]).toContain( - 'isMounted(...) is deprecated in plain JavaScript React classes' - ) + expect(-> + expect(-> instance.replaceState {}).toThrow() + ).toLowPriorityWarnDev( + 'replaceState(...) is deprecated in plain JavaScript React classes' + ) + expect(-> + expect(-> instance.isMounted()).toThrow() + ).toLowPriorityWarnDev( + 'isMounted(...) is deprecated in plain JavaScript React classes' + ) undefined it 'supports this.context passed via getChildContext', -> diff --git a/packages/react/src/__tests__/ReactContextValidator-test.js b/packages/react/src/__tests__/ReactContextValidator-test.js index b2ca14f7d130b..1f0be511f1282 100644 --- a/packages/react/src/__tests__/ReactContextValidator-test.js +++ b/packages/react/src/__tests__/ReactContextValidator-test.js @@ -21,10 +21,6 @@ let ReactDOM; let ReactTestUtils; describe('ReactContextValidator', () => { - function normalizeCodeLocInfo(str) { - return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); - } - beforeEach(() => { jest.resetModules(); @@ -161,8 +157,6 @@ describe('ReactContextValidator', () => { }); it('should check context types', () => { - spyOnDev(console, 'error'); - class Component extends React.Component { render() { return
; @@ -172,17 +166,12 @@ describe('ReactContextValidator', () => { foo: PropTypes.string.isRequired, }; - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: Failed context type: ' + - 'The context `foo` is marked as required in `Component`, but its value ' + - 'is `undefined`.\n' + - ' in Component (at **)', - ); - } + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Warning: Failed context type: ' + + 'The context `foo` is marked as required in `Component`, but its value ' + + 'is `undefined`.\n' + + ' in Component (at **)', + ); class ComponentInFooStringContext extends React.Component { getChildContext() { @@ -199,15 +188,11 @@ describe('ReactContextValidator', () => { foo: PropTypes.string, }; + // No additional errors expected ReactTestUtils.renderIntoDocument( , ); - // Previous call should not error - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - } - class ComponentInFooNumberContext extends React.Component { getChildContext() { return { @@ -223,25 +208,20 @@ describe('ReactContextValidator', () => { foo: PropTypes.number, }; - ReactTestUtils.renderIntoDocument( - , + expect(() => + ReactTestUtils.renderIntoDocument( + , + ), + ).toWarnDev( + 'Warning: Failed context type: ' + + 'Invalid context `foo` of type `number` supplied ' + + 'to `Component`, expected `string`.\n' + + ' in Component (at **)\n' + + ' in ComponentInFooNumberContext (at **)', ); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(2); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe( - 'Warning: Failed context type: ' + - 'Invalid context `foo` of type `number` supplied ' + - 'to `Component`, expected `string`.\n' + - ' in Component (at **)\n' + - ' in ComponentInFooNumberContext (at **)', - ); - } }); it('should check child context types', () => { - spyOnDev(console, 'error'); - class Component extends React.Component { getChildContext() { return this.props.testContext; @@ -256,46 +236,35 @@ describe('ReactContextValidator', () => { bar: PropTypes.number, }; - ReactTestUtils.renderIntoDocument(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: Failed child context type: ' + - 'The child context `foo` is marked as required in `Component`, but its ' + - 'value is `undefined`.\n' + - ' in Component (at **)', - ); - } - - ReactTestUtils.renderIntoDocument(); + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev( + 'Warning: Failed child context type: ' + + 'The child context `foo` is marked as required in `Component`, but its ' + + 'value is `undefined`.\n' + + ' in Component (at **)', + ); - if (__DEV__) { - expect(console.error.calls.count()).toBe(2); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe( - 'Warning: Failed child context type: ' + - 'Invalid child context `foo` of type `number` ' + - 'supplied to `Component`, expected `string`.\n' + - ' in Component (at **)', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev( + 'Warning: Failed child context type: ' + + 'Invalid child context `foo` of type `number` ' + + 'supplied to `Component`, expected `string`.\n' + + ' in Component (at **)', + ); + // No additional errors expected ReactTestUtils.renderIntoDocument( , ); ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - // Previous calls should not log errors - expect(console.error.calls.count()).toBe(2); - } }); // TODO (bvaughn) Remove this test and the associated behavior in the future. // It has only been added in Fiber to match the (unintentional) behavior in Stack. it('should warn (but not error) if getChildContext method is missing', () => { - spyOnDev(console, 'error'); - class ComponentA extends React.Component { static childContextTypes = { foo: PropTypes.string.isRequired, @@ -313,40 +282,28 @@ describe('ReactContextValidator', () => { } } - ReactTestUtils.renderIntoDocument(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: ComponentA.childContextTypes is specified but there is no ' + - 'getChildContext() method on the instance. You can either define ' + - 'getChildContext() on ComponentA or remove childContextTypes from it.', - ); - } + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Warning: ComponentA.childContextTypes is specified but there is no ' + + 'getChildContext() method on the instance. You can either define ' + + 'getChildContext() on ComponentA or remove childContextTypes from it.', + ); // Warnings should be deduped by component type ReactTestUtils.renderIntoDocument(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - } - ReactTestUtils.renderIntoDocument(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(2); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe( - 'Warning: ComponentB.childContextTypes is specified but there is no ' + - 'getChildContext() method on the instance. You can either define ' + - 'getChildContext() on ComponentB or remove childContextTypes from it.', - ); - } + + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Warning: ComponentB.childContextTypes is specified but there is no ' + + 'getChildContext() method on the instance. You can either define ' + + 'getChildContext() on ComponentB or remove childContextTypes from it.', + ); }); // TODO (bvaughn) Remove this test and the associated behavior in the future. // It has only been added in Fiber to match the (unintentional) behavior in Stack. it('should pass parent context if getChildContext method is missing', () => { - spyOnDev(console, 'error'); - class ParentContextProvider extends React.Component { static childContextTypes = { - foo: PropTypes.number, + foo: PropTypes.string, }; getChildContext() { return { @@ -379,7 +336,15 @@ describe('ReactContextValidator', () => { foo: PropTypes.string.isRequired, }; - ReactTestUtils.renderIntoDocument(); + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev([ + 'Warning: MiddleMissingContext.childContextTypes is specified but there is no getChildContext() method on the ' + + 'instance. You can either define getChildContext() on MiddleMissingContext or remove childContextTypes from ' + + 'it.', + 'Warning: Failed context type: The context `bar` is marked as required in `ChildContextConsumer`, but its ' + + 'value is `undefined`.', + ]); expect(childContext.bar).toBeUndefined(); expect(childContext.foo).toBe('FOO'); }); diff --git a/packages/react/src/__tests__/ReactES6Class-test.js b/packages/react/src/__tests__/ReactES6Class-test.js index cbf0d36dfb363..cbc065e3426ff 100644 --- a/packages/react/src/__tests__/ReactES6Class-test.js +++ b/packages/react/src/__tests__/ReactES6Class-test.js @@ -56,17 +56,13 @@ describe('ReactES6Class', () => { }); it('throws if no render function is defined', () => { - spyOnDev(console, 'error'); class Foo extends React.Component {} - expect(() => ReactDOM.render(, container)).toThrow(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Foo(...): No `render` method found on the returned component ' + - 'instance: you may have forgotten to define `render`.', - ); - } + expect(() => + expect(() => ReactDOM.render(, container)).toThrow(), + ).toWarnDev( + 'Warning: Foo(...): No `render` method found on the returned component ' + + 'instance: you may have forgotten to define `render`.', + ); }); it('renders a simple stateless component with prop', () => { @@ -164,7 +160,6 @@ describe('ReactES6Class', () => { }); it('should warn with non-object in the initial state property', () => { - spyOnDev(console, 'error'); [['an array'], 'a string', 1234].forEach(function(state) { class Foo extends React.Component { constructor() { @@ -175,14 +170,9 @@ describe('ReactES6Class', () => { return ; } } - test(, 'SPAN', ''); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Foo.state: must be set to an object or null', - ); - console.error.calls.reset(); - } + expect(() => test(, 'SPAN', '')).toWarnDev( + 'Foo.state: must be set to an object or null', + ); }); }); @@ -310,7 +300,6 @@ describe('ReactES6Class', () => { }); it('warns when classic properties are defined on the instance, but does not invoke them.', () => { - spyOnDev(console, 'error'); let getDefaultPropsWasCalled = false; let getInitialStateWasCalled = false; class Foo extends React.Component { @@ -331,28 +320,18 @@ describe('ReactES6Class', () => { return ; } } - test(, 'SPAN', 'foo'); + + expect(() => test(, 'SPAN', 'foo')).toWarnDev([ + 'getInitialState was defined on Foo, a plain JavaScript class.', + 'getDefaultProps was defined on Foo, a plain JavaScript class.', + 'propTypes was defined as an instance property on Foo.', + 'contextTypes was defined as an instance property on Foo.', + ]); expect(getInitialStateWasCalled).toBe(false); expect(getDefaultPropsWasCalled).toBe(false); - if (__DEV__) { - expect(console.error.calls.count()).toBe(4); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'getInitialState was defined on Foo, a plain JavaScript class.', - ); - expect(console.error.calls.argsFor(1)[0]).toContain( - 'getDefaultProps was defined on Foo, a plain JavaScript class.', - ); - expect(console.error.calls.argsFor(2)[0]).toContain( - 'propTypes was defined as an instance property on Foo.', - ); - expect(console.error.calls.argsFor(3)[0]).toContain( - 'contextTypes was defined as an instance property on Foo.', - ); - } }); it('does not warn about getInitialState() on class components if state is also defined.', () => { - spyOnDev(console, 'error'); class Foo extends React.Component { state = this.getInitialState(); getInitialState() { @@ -363,14 +342,9 @@ describe('ReactES6Class', () => { } } test(, 'SPAN', 'foo'); - if (__DEV__) { - expect(console.error.calls.count()).toBe(0); - } }); it('should warn when misspelling shouldComponentUpdate', () => { - spyOnDev(console, 'error'); - class NamedComponent extends React.Component { componentShouldUpdate() { return false; @@ -379,22 +353,16 @@ describe('ReactES6Class', () => { return ; } } - test(, 'SPAN', 'foo'); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: ' + - 'NamedComponent has a method called componentShouldUpdate(). Did you ' + - 'mean shouldComponentUpdate()? The name is phrased as a question ' + - 'because the function is expected to return a value.', - ); - } + expect(() => test(, 'SPAN', 'foo')).toWarnDev( + 'Warning: ' + + 'NamedComponent has a method called componentShouldUpdate(). Did you ' + + 'mean shouldComponentUpdate()? The name is phrased as a question ' + + 'because the function is expected to return a value.', + ); }); it('should warn when misspelling componentWillReceiveProps', () => { - spyOnDev(console, 'error'); - class NamedComponent extends React.Component { componentWillRecieveProps() { return false; @@ -403,32 +371,26 @@ describe('ReactES6Class', () => { return ; } } - test(, 'SPAN', 'foo'); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: ' + - 'NamedComponent has a method called componentWillRecieveProps(). Did ' + - 'you mean componentWillReceiveProps()?', - ); - } + expect(() => test(, 'SPAN', 'foo')).toWarnDev( + 'Warning: ' + + 'NamedComponent has a method called componentWillRecieveProps(). Did ' + + 'you mean componentWillReceiveProps()?', + ); }); it('should throw AND warn when trying to access classic APIs', () => { - spyOnDev(console, 'warn'); const instance = test(, 'DIV', 'foo'); - expect(() => instance.replaceState({})).toThrow(); - expect(() => instance.isMounted()).toThrow(); - if (__DEV__) { - expect(console.warn.calls.count()).toBe(2); - expect(console.warn.calls.argsFor(0)[0]).toContain( - 'replaceState(...) is deprecated in plain JavaScript React classes', - ); - expect(console.warn.calls.argsFor(1)[0]).toContain( - 'isMounted(...) is deprecated in plain JavaScript React classes', - ); - } + expect(() => + expect(() => instance.replaceState({})).toThrow(), + ).toLowPriorityWarnDev( + 'replaceState(...) is deprecated in plain JavaScript React classes', + ); + expect(() => + expect(() => instance.isMounted()).toThrow(), + ).toLowPriorityWarnDev( + 'isMounted(...) is deprecated in plain JavaScript React classes', + ); }); it('supports this.context passed via getChildContext', () => { diff --git a/packages/react/src/__tests__/ReactElement-test.js b/packages/react/src/__tests__/ReactElement-test.js index 1e0b41e0ba19a..5790767e5b5a3 100644 --- a/packages/react/src/__tests__/ReactElement-test.js +++ b/packages/react/src/__tests__/ReactElement-test.js @@ -58,7 +58,6 @@ describe('ReactElement', () => { }); it('should warn when `key` is being accessed on composite element', () => { - spyOnDev(console, 'error'); const container = document.createElement('div'); class Child extends React.Component { render() { @@ -76,41 +75,25 @@ describe('ReactElement', () => { ); } } - if (__DEV__) { - expect(console.error.calls.count()).toBe(0); - } - ReactDOM.render(, container); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Child: `key` is not a prop. Trying to access it will result ' + - 'in `undefined` being returned. If you need to access the same ' + - 'value within the child component, you should pass it as a different ' + - 'prop. (https://fb.me/react-special-props)', - ); - } + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Child: `key` is not a prop. Trying to access it will result ' + + 'in `undefined` being returned. If you need to access the same ' + + 'value within the child component, you should pass it as a different ' + + 'prop. (https://fb.me/react-special-props)', + ); }); it('should warn when `key` is being accessed on a host element', () => { - spyOnDev(console, 'error'); const element =
; - if (__DEV__) { - expect(console.error.calls.count()).toBe(0); - } - void element.props.key; - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'div: `key` is not a prop. Trying to access it will result ' + - 'in `undefined` being returned. If you need to access the same ' + - 'value within the child component, you should pass it as a different ' + - 'prop. (https://fb.me/react-special-props)', - ); - } + expect(() => void element.props.key).toWarnDev( + 'div: `key` is not a prop. Trying to access it will result ' + + 'in `undefined` being returned. If you need to access the same ' + + 'value within the child component, you should pass it as a different ' + + 'prop. (https://fb.me/react-special-props)', + ); }); it('should warn when `ref` is being accessed', () => { - spyOnDev(console, 'error'); const container = document.createElement('div'); class Child extends React.Component { render() { @@ -126,19 +109,12 @@ describe('ReactElement', () => { ); } } - if (__DEV__) { - expect(console.error.calls.count()).toBe(0); - } - ReactDOM.render(, container); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Child: `ref` is not a prop. Trying to access it will result ' + - 'in `undefined` being returned. If you need to access the same ' + - 'value within the child component, you should pass it as a different ' + - 'prop. (https://fb.me/react-special-props)', - ); - } + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Child: `ref` is not a prop. Trying to access it will result ' + + 'in `undefined` being returned. If you need to access the same ' + + 'value within the child component, you should pass it as a different ' + + 'prop. (https://fb.me/react-special-props)', + ); }); it('allows a string to be passed as the type', () => { diff --git a/packages/react/src/__tests__/ReactElementClone-test.js b/packages/react/src/__tests__/ReactElementClone-test.js index 592fba11b34d6..d5b08496ba747 100644 --- a/packages/react/src/__tests__/ReactElementClone-test.js +++ b/packages/react/src/__tests__/ReactElementClone-test.js @@ -257,16 +257,11 @@ describe('ReactElementClone', () => { }); it('warns for keys for arrays of elements in rest args', () => { - spyOnDev(console, 'error'); - - React.cloneElement(
, null, [
,
]); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Each child in an array or iterator should have a unique "key" prop.', - ); - } + expect(() => + React.cloneElement(
, null, [
,
]), + ).toWarnDev( + 'Each child in an array or iterator should have a unique "key" prop.', + ); }); it('does not warns for arrays of elements with keys', () => { @@ -282,7 +277,6 @@ describe('ReactElementClone', () => { }); it('should check declared prop types after clone', () => { - spyOnDev(console, 'error'); class Component extends React.Component { static propTypes = { color: PropTypes.string.isRequired, @@ -303,18 +297,16 @@ describe('ReactElementClone', () => { }); } } - ReactTestUtils.renderIntoDocument(React.createElement(GrandParent)); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Failed prop type: ' + - 'Invalid prop `color` of type `number` supplied to `Component`, ' + - 'expected `string`.\n' + - ' in Component (created by GrandParent)\n' + - ' in Parent (created by GrandParent)\n' + - ' in GrandParent', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument(React.createElement(GrandParent)), + ).toWarnDev( + 'Warning: Failed prop type: ' + + 'Invalid prop `color` of type `number` supplied to `Component`, ' + + 'expected `string`.\n' + + ' in Component (created by GrandParent)\n' + + ' in Parent (created by GrandParent)\n' + + ' in GrandParent', + ); }); it('should ignore key and ref warning getters', () => { diff --git a/packages/react/src/__tests__/ReactJSXElementValidator-test.js b/packages/react/src/__tests__/ReactJSXElementValidator-test.js index b533703600035..741d978de73c9 100644 --- a/packages/react/src/__tests__/ReactJSXElementValidator-test.js +++ b/packages/react/src/__tests__/ReactJSXElementValidator-test.js @@ -17,10 +17,6 @@ let ReactTestUtils; let PropTypes; describe('ReactJSXElementValidator', () => { - function normalizeCodeLocInfo(str) { - return str && str.replace(/at .+?:\d+/g, 'at **'); - } - let Component; let RequiredPropComponent; @@ -48,23 +44,16 @@ describe('ReactJSXElementValidator', () => { }); it('warns for keys for arrays of elements in children position', () => { - spyOnDev(console, 'error'); - - ReactTestUtils.renderIntoDocument( - {[, ]}, + expect(() => + ReactTestUtils.renderIntoDocument( + {[, ]}, + ), + ).toWarnDev( + 'Each child in an array or iterator should have a unique "key" prop.', ); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Each child in an array or iterator should have a unique "key" prop.', - ); - } }); it('warns for keys for arrays of elements with owner info', () => { - spyOnDev(console, 'error'); - class InnerComponent extends React.Component { render() { return {this.props.childSet}; @@ -77,21 +66,16 @@ describe('ReactJSXElementValidator', () => { } } - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Each child in an array or iterator should have a unique "key" prop.' + - '\n\nCheck the render method of `InnerComponent`. ' + - 'It was passed a child from ComponentWrapper. ', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev( + 'Each child in an array or iterator should have a unique "key" prop.' + + '\n\nCheck the render method of `InnerComponent`. ' + + 'It was passed a child from ComponentWrapper. ', + ); }); it('warns for keys for iterables of elements in rest args', () => { - spyOnDev(console, 'error'); - const iterable = { '@@iterator': function() { let i = 0; @@ -104,14 +88,11 @@ describe('ReactJSXElementValidator', () => { }, }; - ReactTestUtils.renderIntoDocument({iterable}); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Each child in an array or iterator should have a unique "key" prop.', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument({iterable}), + ).toWarnDev( + 'Each child in an array or iterator should have a unique "key" prop.', + ); }); it('does not warn for arrays of elements with keys', () => { @@ -173,7 +154,6 @@ describe('ReactJSXElementValidator', () => { // In this test, we're making sure that if a proptype error is found in a // component, we give a small hint as to which parent instantiated that // component as per warnings about key usage in ReactElementValidator. - spyOnDev(console, 'error'); class MyComp extends React.Component { render() { return
My color is {this.color}
; @@ -187,20 +167,16 @@ describe('ReactJSXElementValidator', () => { return ; } } - ReactTestUtils.renderIntoDocument(); - if (__DEV__) { - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: Failed prop type: ' + - 'Invalid prop `color` of type `number` supplied to `MyComp`, ' + - 'expected `string`.\n' + - ' in MyComp (at **)\n' + - ' in ParentComp (at **)', - ); - } + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Warning: Failed prop type: ' + + 'Invalid prop `color` of type `number` supplied to `MyComp`, ' + + 'expected `string`.\n' + + ' in MyComp (at **)\n' + + ' in ParentComp (at **)', + ); }); it('should update component stack after receiving next element', () => { - spyOnDev(console, 'error'); function MyComp() { return null; } @@ -221,21 +197,16 @@ describe('ReactJSXElementValidator', () => { const container = document.createElement('div'); ReactDOM.render(, container); - ReactDOM.render(, container); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - // The warning should have the full stack with line numbers. - // If it doesn't, it means we're using information from the old element. - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: Failed prop type: ' + - 'Invalid prop `color` of type `number` supplied to `MyComp`, ' + - 'expected `string`.\n' + - ' in MyComp (at **)\n' + - ' in MiddleComp (at **)\n' + - ' in ParentComp (at **)', - ); - } + expect(() => + ReactDOM.render(, container), + ).toWarnDev( + 'Warning: Failed prop type: ' + + 'Invalid prop `color` of type `number` supplied to `MyComp`, ' + + 'expected `string`.\n' + + ' in MyComp (at **)\n' + + ' in MiddleComp (at **)\n' + + ' in ParentComp (at **)', + ); }); it('gives a helpful error when passing null, undefined, or boolean', () => { @@ -243,100 +214,72 @@ describe('ReactJSXElementValidator', () => { const Null = null; const True = true; const Div = 'div'; - spyOnDev(console, 'error'); - void ; - void ; - void ; - if (__DEV__) { - expect(console.error.calls.count()).toBe(3); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: React.createElement: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: undefined. You likely forgot to export your ' + - "component from the file it's defined in, or you might have mixed up " + - 'default and named imports.' + - '\n\nCheck your code at **.', - ); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe( - 'Warning: React.createElement: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: null.' + - '\n\nCheck your code at **.', - ); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toBe( - 'Warning: React.createElement: type is invalid -- expected a string ' + - '(for built-in components) or a class/function (for composite ' + - 'components) but got: boolean.' + - '\n\nCheck your code at **.', - ); - } + expect(() => void ).toWarnDev( + 'Warning: React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: undefined. You likely forgot to export your ' + + "component from the file it's defined in, or you might have mixed up " + + 'default and named imports.' + + '\n\nCheck your code at **.', + ); + expect(() => void ).toWarnDev( + 'Warning: React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: null.' + + '\n\nCheck your code at **.', + ); + expect(() => void ).toWarnDev( + 'Warning: React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function (for composite ' + + 'components) but got: boolean.' + + '\n\nCheck your code at **.', + ); + // No error expected void
; - if (__DEV__) { - expect(console.error.calls.count()).toBe(3); - } }); it('should check default prop values', () => { - spyOnDev(console, 'error'); - RequiredPropComponent.defaultProps = {prop: null}; - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: Failed prop type: The prop `prop` is marked as required in ' + - '`RequiredPropComponent`, but its value is `null`.\n' + - ' in RequiredPropComponent (at **)', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev( + 'Warning: Failed prop type: The prop `prop` is marked as required in ' + + '`RequiredPropComponent`, but its value is `null`.\n' + + ' in RequiredPropComponent (at **)', + ); }); it('should not check the default for explicit null', () => { - spyOnDev(console, 'error'); - - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: Failed prop type: The prop `prop` is marked as required in ' + - '`RequiredPropComponent`, but its value is `null`.\n' + - ' in RequiredPropComponent (at **)', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev( + 'Warning: Failed prop type: The prop `prop` is marked as required in ' + + '`RequiredPropComponent`, but its value is `null`.\n' + + ' in RequiredPropComponent (at **)', + ); }); it('should check declared prop types', () => { - spyOnDev(console, 'error'); - - ReactTestUtils.renderIntoDocument(); - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(2); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe( - 'Warning: Failed prop type: ' + - 'The prop `prop` is marked as required in `RequiredPropComponent`, but ' + - 'its value is `undefined`.\n' + - ' in RequiredPropComponent (at **)', - ); - - expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe( - 'Warning: Failed prop type: ' + - 'Invalid prop `prop` of type `number` supplied to ' + - '`RequiredPropComponent`, expected `string`.\n' + - ' in RequiredPropComponent (at **)', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev( + 'Warning: Failed prop type: ' + + 'The prop `prop` is marked as required in `RequiredPropComponent`, but ' + + 'its value is `undefined`.\n' + + ' in RequiredPropComponent (at **)', + ); + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev( + 'Warning: Failed prop type: ' + + 'Invalid prop `prop` of type `number` supplied to ' + + '`RequiredPropComponent`, expected `string`.\n' + + ' in RequiredPropComponent (at **)', + ); + // Should not error for strings ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - // Should not error for strings - expect(console.error.calls.count()).toBe(2); - } }); it('should warn on invalid prop types', () => { @@ -344,7 +287,6 @@ describe('ReactJSXElementValidator', () => { // for us to issue a warning earlier than element creation when the error // actually occurs. Since this step is skipped in production, we should just // warn instead of throwing for this case. - spyOnDev(console, 'error'); class NullPropTypeComponent extends React.Component { render() { return {this.props.prop}; @@ -353,18 +295,15 @@ describe('ReactJSXElementValidator', () => { NullPropTypeComponent.propTypes = { prop: null, }; - ReactTestUtils.renderIntoDocument(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'NullPropTypeComponent: prop type `prop` is invalid; it must be a ' + - 'function, usually from the `prop-types` package,', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev( + 'NullPropTypeComponent: prop type `prop` is invalid; it must be a ' + + 'function, usually from the `prop-types` package,', + ); }); it('should warn on invalid context types', () => { - spyOnDev(console, 'error'); class NullContextTypeComponent extends React.Component { render() { return {this.props.prop}; @@ -373,18 +312,15 @@ describe('ReactJSXElementValidator', () => { NullContextTypeComponent.contextTypes = { prop: null, }; - ReactTestUtils.renderIntoDocument(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'NullContextTypeComponent: context type `prop` is invalid; it must ' + - 'be a function, usually from the `prop-types` package,', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev( + 'NullContextTypeComponent: context type `prop` is invalid; it must ' + + 'be a function, usually from the `prop-types` package,', + ); }); it('should warn if getDefaultProps is specificed on the class', () => { - spyOnDev(console, 'error'); class GetDefaultPropsComponent extends React.Component { render() { return {this.props.prop}; @@ -393,18 +329,15 @@ describe('ReactJSXElementValidator', () => { GetDefaultPropsComponent.getDefaultProps = () => ({ prop: 'foo', }); - ReactTestUtils.renderIntoDocument(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'getDefaultProps is only used on classic React.createClass definitions.' + - ' Use a static property named `defaultProps` instead.', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument(), + ).toWarnDev( + 'getDefaultProps is only used on classic React.createClass definitions.' + + ' Use a static property named `defaultProps` instead.', + ); }); it('should warn if component declares PropTypes instead of propTypes', () => { - spyOnDevAndProd(console, 'error'); class MisspelledPropTypesComponent extends React.Component { render() { return {this.props.prop}; @@ -413,46 +346,30 @@ describe('ReactJSXElementValidator', () => { MisspelledPropTypesComponent.PropTypes = { prop: PropTypes.string, }; - ReactTestUtils.renderIntoDocument( - , + expect(() => + ReactTestUtils.renderIntoDocument( + , + ), + ).toWarnDev( + 'Warning: Component MisspelledPropTypesComponent declared `PropTypes` ' + + 'instead of `propTypes`. Did you misspell the property assignment?', ); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Component MisspelledPropTypesComponent declared `PropTypes` ' + - 'instead of `propTypes`. Did you misspell the property assignment?', - ); - } }); it('warns for fragments with illegal attributes', () => { - spyOnDev(console, 'error'); - class Foo extends React.Component { render() { - return ( - - hello - - ); + return hello; } } - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain('Invalid prop `'); - expect(console.error.calls.argsFor(0)[0]).toContain( - '` supplied to `React.Fragment`. React.Fragment ' + - 'can only have `key` and `children` props.', - ); - } + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Invalid prop `a` supplied to `React.Fragment`. React.Fragment ' + + 'can only have `key` and `children` props.', + ); }); it('warns for fragments with refs', () => { - spyOnDev(console, 'error'); - class Foo extends React.Component { render() { return ( @@ -466,14 +383,9 @@ describe('ReactJSXElementValidator', () => { } } - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Invalid attribute `ref` supplied to `React.Fragment`.', - ); - } + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Invalid attribute `ref` supplied to `React.Fragment`.', + ); }); it('does not warn for fragments of multiple elements without keys', () => { @@ -486,21 +398,14 @@ describe('ReactJSXElementValidator', () => { }); it('warns for fragments of multiple elements with same key', () => { - spyOnDev(console, 'error'); - - ReactTestUtils.renderIntoDocument( - - 1 - 2 - 3 - , - ); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Encountered two children with the same key, `a`.', - ); - } + expect(() => + ReactTestUtils.renderIntoDocument( + + 1 + 2 + 3 + , + ), + ).toWarnDev('Encountered two children with the same key, `a`.'); }); }); diff --git a/packages/react/src/__tests__/createReactClassIntegration-test.js b/packages/react/src/__tests__/createReactClassIntegration-test.js index 22bf8e054c09e..50e20aeddb58c 100644 --- a/packages/react/src/__tests__/createReactClassIntegration-test.js +++ b/packages/react/src/__tests__/createReactClassIntegration-test.js @@ -52,121 +52,103 @@ describe('create-react-class-integration', () => { }); it('should warn on invalid prop types', () => { - spyOnDev(console, 'error'); - createReactClass({ - displayName: 'Component', - propTypes: { - prop: null, - }, - render: function() { - return {this.props.prop}; - }, - }); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Component: prop type `prop` is invalid; ' + - 'it must be a function, usually from React.PropTypes.', - ); - } + expect(() => + createReactClass({ + displayName: 'Component', + propTypes: { + prop: null, + }, + render: function() { + return {this.props.prop}; + }, + }), + ).toWarnDev( + 'Warning: Component: prop type `prop` is invalid; ' + + 'it must be a function, usually from React.PropTypes.', + ); }); it('should warn on invalid context types', () => { - spyOnDev(console, 'error'); - createReactClass({ - displayName: 'Component', - contextTypes: { - prop: null, - }, - render: function() { - return {this.props.prop}; - }, - }); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Component: context type `prop` is invalid; ' + - 'it must be a function, usually from React.PropTypes.', - ); - } + expect(() => + createReactClass({ + displayName: 'Component', + contextTypes: { + prop: null, + }, + render: function() { + return {this.props.prop}; + }, + }), + ).toWarnDev( + 'Warning: Component: context type `prop` is invalid; ' + + 'it must be a function, usually from React.PropTypes.', + ); }); it('should throw on invalid child context types', () => { - spyOnDev(console, 'error'); - createReactClass({ - displayName: 'Component', - childContextTypes: { - prop: null, - }, - render: function() { - return {this.props.prop}; - }, - }); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Component: child context type `prop` is invalid; ' + - 'it must be a function, usually from React.PropTypes.', - ); - } + expect(() => + createReactClass({ + displayName: 'Component', + childContextTypes: { + prop: null, + }, + render: function() { + return {this.props.prop}; + }, + }), + ).toWarnDev( + 'Warning: Component: child context type `prop` is invalid; ' + + 'it must be a function, usually from React.PropTypes.', + ); }); it('should warn when misspelling shouldComponentUpdate', () => { - spyOnDev(console, 'error'); - - createReactClass({ - componentShouldUpdate: function() { - return false; - }, - render: function() { - return
; - }, - }); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: A component has a method called componentShouldUpdate(). Did you ' + - 'mean shouldComponentUpdate()? The name is phrased as a question ' + - 'because the function is expected to return a value.', - ); - } + expect(() => + createReactClass({ + componentShouldUpdate: function() { + return false; + }, + render: function() { + return
; + }, + }), + ).toWarnDev( + 'Warning: A component has a method called componentShouldUpdate(). Did you ' + + 'mean shouldComponentUpdate()? The name is phrased as a question ' + + 'because the function is expected to return a value.', + ); - createReactClass({ - displayName: 'NamedComponent', - componentShouldUpdate: function() { - return false; - }, - render: function() { - return
; - }, - }); - if (__DEV__) { - expect(console.error.calls.count()).toBe(2); - expect(console.error.calls.argsFor(1)[0]).toBe( - 'Warning: NamedComponent has a method called componentShouldUpdate(). Did you ' + - 'mean shouldComponentUpdate()? The name is phrased as a question ' + - 'because the function is expected to return a value.', - ); - } + expect(() => + createReactClass({ + displayName: 'NamedComponent', + componentShouldUpdate: function() { + return false; + }, + render: function() { + return
; + }, + }), + ).toWarnDev( + 'Warning: NamedComponent has a method called componentShouldUpdate(). Did you ' + + 'mean shouldComponentUpdate()? The name is phrased as a question ' + + 'because the function is expected to return a value.', + ); }); it('should warn when misspelling componentWillReceiveProps', () => { - spyOnDev(console, 'error'); - createReactClass({ - componentWillRecieveProps: function() { - return false; - }, - render: function() { - return
; - }, - }); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: A component has a method called componentWillRecieveProps(). Did you ' + - 'mean componentWillReceiveProps()?', - ); - } + expect(() => + createReactClass({ + componentWillRecieveProps: function() { + return false; + }, + render: function() { + return
; + }, + }), + ).toWarnDev( + 'Warning: A component has a method called componentWillRecieveProps(). Did you ' + + 'mean componentWillReceiveProps()?', + ); }); it('should throw if a reserved property is in statics', () => { @@ -195,41 +177,32 @@ describe('create-react-class-integration', () => { // TODO: Consider actually moving these to statics or drop this unit test. xit('should warn when using deprecated non-static spec keys', () => { - spyOnDev(console, 'error'); - createReactClass({ - mixins: [{}], - propTypes: { - foo: PropTypes.string, - }, - contextTypes: { - foo: PropTypes.string, - }, - childContextTypes: { - foo: PropTypes.string, - }, - render: function() { - return
; - }, - }); - if (__DEV__) { - expect(console.error.calls.count()).toBe(4); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'createClass(...): `mixins` is now a static property and should ' + - 'be defined inside "statics".', - ); - expect(console.error.calls.argsFor(1)[0]).toBe( - 'createClass(...): `propTypes` is now a static property and should ' + - 'be defined inside "statics".', - ); - expect(console.error.calls.argsFor(2)[0]).toBe( - 'createClass(...): `contextTypes` is now a static property and ' + - 'should be defined inside "statics".', - ); - expect(console.error.calls.argsFor(3)[0]).toBe( - 'createClass(...): `childContextTypes` is now a static property and ' + - 'should be defined inside "statics".', - ); - } + expect(() => + createReactClass({ + mixins: [{}], + propTypes: { + foo: PropTypes.string, + }, + contextTypes: { + foo: PropTypes.string, + }, + childContextTypes: { + foo: PropTypes.string, + }, + render: function() { + return
; + }, + }), + ).toWarnDev([ + 'createClass(...): `mixins` is now a static property and should ' + + 'be defined inside "statics".', + 'createClass(...): `propTypes` is now a static property and should ' + + 'be defined inside "statics".', + 'createClass(...): `contextTypes` is now a static property and ' + + 'should be defined inside "statics".', + 'createClass(...): `childContextTypes` is now a static property and ' + + 'should be defined inside "statics".', + ]); }); it('should support statics', () => { @@ -342,21 +315,16 @@ describe('create-react-class-integration', () => { }); it('should throw when using legacy factories', () => { - spyOnDev(console, 'error'); const Component = createReactClass({ render() { return
; }, }); - expect(() => Component()).toThrow(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Something is calling a React component directly. Use a ' + - 'factory or JSX instead. See: https://fb.me/react-legacyfactory', - ); - } + expect(() => expect(() => Component()).toThrow()).toWarnDev( + 'Warning: Something is calling a React component directly. Use a ' + + 'factory or JSX instead. See: https://fb.me/react-legacyfactory', + ); }); it('replaceState and callback works', () => { @@ -379,8 +347,6 @@ describe('create-react-class-integration', () => { }); it('isMounted works', () => { - spyOnDev(console, 'error'); - const ops = []; let instance; const Component = createReactClass({ @@ -434,7 +400,13 @@ describe('create-react-class-integration', () => { }); const container = document.createElement('div'); - ReactDOM.render(, container); + + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Warning: MyComponent: isMounted is deprecated. Instead, make sure to ' + + 'clean up subscriptions and pending requests in componentWillUnmount ' + + 'to prevent memory leaks.', + ); + ReactDOM.render(, container); ReactDOM.unmountComponentAtNode(container); instance.log('after unmount'); @@ -454,14 +426,5 @@ describe('create-react-class-integration', () => { 'componentWillUnmount: true', 'after unmount: false', ]); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toEqual( - 'Warning: MyComponent: isMounted is deprecated. Instead, make sure to ' + - 'clean up subscriptions and pending requests in componentWillUnmount ' + - 'to prevent memory leaks.', - ); - } }); }); From 2541749e48f0c50da083347f84e6c978479df6d2 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 18 Dec 2017 22:01:18 -0800 Subject: [PATCH 12/17] Added TypeScript test and types, fixed equivalence test --- .../__tests__/ReactTypeScriptClass-test.ts | 276 ++++++++---------- .../spec-equivalence-reporter/setupTests.js | 4 + scripts/jest/typescript/jest.d.ts | 2 + 3 files changed, 123 insertions(+), 159 deletions(-) diff --git a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts index 76b3b34937613..3604af9da7be7 100644 --- a/packages/react/src/__tests__/ReactTypeScriptClass-test.ts +++ b/packages/react/src/__tests__/ReactTypeScriptClass-test.ts @@ -26,7 +26,7 @@ class Inner extends React.Component { render() { attachedListener = this.props.onClick; renderedName = this.props.name; - return React.createElement('div', { className: this.props.name }); + return React.createElement('div', {className: this.props.name}); } } @@ -45,11 +45,11 @@ function test(element, expectedTag, expectedClassName) { // it preserves the name of the class for use in error messages // it throws if no render function is defined -class Empty extends React.Component { } +class Empty extends React.Component {} // it renders a simple stateless component with prop class SimpleStateless extends React.Component { - props : any; + props: any; render() { return React.createElement(Inner, {name: this.props.bar}); } @@ -58,7 +58,7 @@ class SimpleStateless extends React.Component { // it renders based on state using initial values in this.props class InitialState extends React.Component { state = { - bar: this.props.initialValue + bar: this.props.initialValue, }; render() { return React.createElement('span', {className: this.state.bar}); @@ -69,10 +69,10 @@ class InitialState extends React.Component { class StateBasedOnProps extends React.Component { constructor(props) { super(props); - this.state = { bar: props.initialValue }; + this.state = {bar: props.initialValue}; } changeState() { - this.setState({ bar: 'bar' }); + this.setState({bar: 'bar'}); } render() { if (this.state.bar === 'foo') { @@ -86,11 +86,11 @@ class StateBasedOnProps extends React.Component { class StateBasedOnContext extends React.Component { static contextTypes = { tag: PropTypes.string, - className: PropTypes.string + className: PropTypes.string, }; state = { tag: this.context.tag, - className: this.context.className + className: this.context.className, }; render() { const Tag = this.state.tag; @@ -101,10 +101,10 @@ class StateBasedOnContext extends React.Component { class ProvideChildContextTypes extends React.Component { static childContextTypes = { tag: PropTypes.string, - className: PropTypes.string + className: PropTypes.string, }; getChildContext() { - return { tag: 'span', className: 'foo' }; + return {tag: 'span', className: 'foo'}; } render() { return React.createElement(StateBasedOnContext); @@ -115,10 +115,10 @@ class ProvideChildContextTypes extends React.Component { let renderCount = 0; class RenderOnce extends React.Component { state = { - bar: this.props.initialValue + bar: this.props.initialValue, }; componentWillMount() { - this.setState({ bar: 'bar' }); + this.setState({bar: 'bar'}); } render() { renderCount++; @@ -157,57 +157,54 @@ class NullState extends React.Component { // it setState through an event handler class BoundEventHandler extends React.Component { state = { - bar: this.props.initialValue + bar: this.props.initialValue, }; handleClick = () => { - this.setState({ bar: 'bar' }); + this.setState({bar: 'bar'}); }; render() { - return ( - React.createElement(Inner, { - name: this.state.bar, - onClick: this.handleClick - }) - ); + return React.createElement(Inner, { + name: this.state.bar, + onClick: this.handleClick, + }); } } // it should not implicitly bind event handlers class UnboundEventHandler extends React.Component { state = { - bar: this.props.initialValue + bar: this.props.initialValue, }; handleClick() { - this.setState({ bar: 'bar' }); + this.setState({bar: 'bar'}); } render() { - return React.createElement( - Inner, { name: this.state.bar, onClick: this.handleClick } - ); + return React.createElement(Inner, { + name: this.state.bar, + onClick: this.handleClick, + }); } } // it renders using forceUpdate even when there is no state class ForceUpdateWithNoState extends React.Component { - mutativeValue : string = this.props.initialValue; + mutativeValue: string = this.props.initialValue; handleClick() { this.mutativeValue = 'bar'; this.forceUpdate(); } render() { - return ( - React.createElement(Inner, { - name: this.mutativeValue, - onClick: this.handleClick.bind(this)} - ) - ); + return React.createElement(Inner, { + name: this.mutativeValue, + onClick: this.handleClick.bind(this), + }); } } // it will call all the normal life cycle methods let lifeCycles = []; class NormalLifeCycles extends React.Component { - props : any; + props: any; state = {}; componentWillMount() { lifeCycles.push('will-mount'); @@ -278,15 +275,15 @@ class MisspelledComponent2 extends React.Component { // it supports this.context passed via getChildContext class ReadContext extends React.Component { - static contextTypes = { bar: PropTypes.string }; + static contextTypes = {bar: PropTypes.string}; render() { - return React.createElement('div', { className: this.context.bar }); + return React.createElement('div', {className: this.context.bar}); } } class ProvideContext extends React.Component { - static childContextTypes = { bar: PropTypes.string }; + static childContextTypes = {bar: PropTypes.string}; getChildContext() { - return { bar: 'bar-through-context' }; + return {bar: 'bar-through-context'}; } render() { return React.createElement(ReadContext); @@ -303,7 +300,6 @@ class ClassicRefs extends React.Component { // Describe the actual test cases. describe('ReactTypeScriptClass', function() { - beforeEach(function() { container = document.createElement('div'); attachedListener = null; @@ -315,17 +311,14 @@ describe('ReactTypeScriptClass', function() { }); it('throws if no render function is defined', function() { - spyOnDev(console, 'error'); - - expect(() => ReactDOM.render(React.createElement(Empty), container)).toThrow(); - - if (__DEV__) { - expect((console.error).calls.count()).toBe(1); - expect((console.error).calls.argsFor(0)[0]).toBe( - 'Warning: Empty(...): No `render` method found on the returned ' + + expect(() => + expect(() => + ReactDOM.render(React.createElement(Empty), container) + ).toThrow() + ).toWarnDev( + 'Warning: Empty(...): No `render` method found on the returned ' + 'component instance: you may have forgotten to define `render`.' - ); - } + ); }); it('renders a simple stateless component with prop', function() { @@ -362,31 +355,15 @@ describe('ReactTypeScriptClass', function() { }); it('should warn with non-object in the initial state property', function() { - spyOnDev(console, 'error'); - test(React.createElement(ArrayState), 'SPAN', ''); - if (__DEV__) { - expect((console.error).calls.count()).toBe(1); - expect((console.error).calls.argsFor(0)[0]).toContain( - 'ArrayState.state: must be set to an object or null' - ); - (console.error).calls.reset() - } - test(React.createElement(StringState), 'SPAN', ''); - if (__DEV__) { - expect((console.error).calls.count()).toBe(1); - expect((console.error).calls.argsFor(0)[0]).toContain( - 'StringState.state: must be set to an object or null' - ); - (console.error).calls.reset() - } - test(React.createElement(NumberState), 'SPAN', ''); - if (__DEV__) { - expect((console.error).calls.count()).toBe(1); - expect((console.error).calls.argsFor(0)[0]).toContain( - 'NumberState.state: must be set to an object or null' - ); - (console.error).calls.reset() - } + expect(() => test(React.createElement(ArrayState), 'SPAN', '')).toWarnDev( + 'ArrayState.state: must be set to an object or null' + ); + expect(() => test(React.createElement(StringState), 'SPAN', '')).toWarnDev( + 'StringState.state: must be set to an object or null' + ); + expect(() => test(React.createElement(NumberState), 'SPAN', '')).toWarnDev( + 'NumberState.state: must be set to an object or null' + ); }); it('should render with null in the initial state property', function() { @@ -425,121 +402,103 @@ describe('ReactTypeScriptClass', function() { it('will call all the normal life cycle methods', function() { lifeCycles = []; test(React.createElement(NormalLifeCycles, {value: 'foo'}), 'SPAN', 'foo'); - expect(lifeCycles).toEqual([ - 'will-mount', - 'did-mount' - ]); + expect(lifeCycles).toEqual(['will-mount', 'did-mount']); lifeCycles = []; // reset test(React.createElement(NormalLifeCycles, {value: 'bar'}), 'SPAN', 'bar'); expect(lifeCycles).toEqual([ - 'receive-props', { value: 'bar' }, - 'should-update', { value: 'bar' }, {}, - 'will-update', { value: 'bar' }, {}, - 'did-update', { value: 'foo' }, {} + 'receive-props', + {value: 'bar'}, + 'should-update', + {value: 'bar'}, + {}, + 'will-update', + {value: 'bar'}, + {}, + 'did-update', + {value: 'foo'}, + {}, ]); lifeCycles = []; // reset ReactDOM.unmountComponentAtNode(container); - expect(lifeCycles).toEqual([ - 'will-unmount' - ]); + expect(lifeCycles).toEqual(['will-unmount']); }); - it('warns when classic properties are defined on the instance, ' + - 'but does not invoke them.', function() { - spyOnDev(console, 'error'); - getInitialStateWasCalled = false; - getDefaultPropsWasCalled = false; - test(React.createElement(ClassicProperties), 'SPAN', 'foo'); - expect(getInitialStateWasCalled).toBe(false); - expect(getDefaultPropsWasCalled).toBe(false); - - if (__DEV__) { - expect((console.error).calls.count()).toBe(4); - expect((console.error).calls.argsFor(0)[0]).toContain( + it( + 'warns when classic properties are defined on the instance, ' + + 'but does not invoke them.', + function() { + getInitialStateWasCalled = false; + getDefaultPropsWasCalled = false; + expect(() => + test(React.createElement(ClassicProperties), 'SPAN', 'foo') + ).toWarnDev([ 'getInitialState was defined on ClassicProperties, ' + - 'a plain JavaScript class.' - ); - expect((console.error).calls.argsFor(1)[0]).toContain( + 'a plain JavaScript class.', 'getDefaultProps was defined on ClassicProperties, ' + - 'a plain JavaScript class.' - ); - expect((console.error).calls.argsFor(2)[0]).toContain( - 'propTypes was defined as an instance property on ClassicProperties.' - ); - expect((console.error).calls.argsFor(3)[0]).toContain( - 'contextTypes was defined as an instance property on ClassicProperties.' - ); + 'a plain JavaScript class.', + 'propTypes was defined as an instance property on ClassicProperties.', + 'contextTypes was defined as an instance property on ClassicProperties.', + ]); + expect(getInitialStateWasCalled).toBe(false); + expect(getDefaultPropsWasCalled).toBe(false); } - }); - - it('does not warn about getInitialState() on class components ' + - 'if state is also defined.', () => { - spyOnDev(console, 'error'); - - class Example extends React.Component { - state = {}; - getInitialState() { - return {}; - } - render() { - return React.createElement('span', {className: 'foo'}); + ); + + it( + 'does not warn about getInitialState() on class components ' + + 'if state is also defined.', + () => { + class Example extends React.Component { + state = {}; + getInitialState() { + return {}; + } + render() { + return React.createElement('span', {className: 'foo'}); + } } - } - test(React.createElement(Example), 'SPAN', 'foo'); - if (__DEV__) { - expect((console.error).calls.count()).toBe(0); + test(React.createElement(Example), 'SPAN', 'foo'); } - }); + ); it('should warn when misspelling shouldComponentUpdate', function() { - spyOnDev(console, 'error'); - - test(React.createElement(MisspelledComponent1), 'SPAN', 'foo'); - - if (__DEV__) { - expect((console.error).calls.count()).toBe(1); - expect((console.error).calls.argsFor(0)[0]).toBe( - 'Warning: ' + + expect(() => + test(React.createElement(MisspelledComponent1), 'SPAN', 'foo') + ).toWarnDev( + 'Warning: ' + 'MisspelledComponent1 has a method called componentShouldUpdate(). Did ' + 'you mean shouldComponentUpdate()? The name is phrased as a question ' + 'because the function is expected to return a value.' - ); - } + ); }); it('should warn when misspelling componentWillReceiveProps', function() { - spyOnDev(console, 'error'); - - test(React.createElement(MisspelledComponent2), 'SPAN', 'foo'); - - if (__DEV__) { - expect((console.error).calls.count()).toBe(1); - expect((console.error).calls.argsFor(0)[0]).toBe( - 'Warning: ' + + expect(() => + test(React.createElement(MisspelledComponent2), 'SPAN', 'foo') + ).toWarnDev( + 'Warning: ' + 'MisspelledComponent2 has a method called componentWillRecieveProps(). ' + 'Did you mean componentWillReceiveProps()?' - ); - } + ); }); it('should throw AND warn when trying to access classic APIs', function() { - spyOnDev(console, 'warn'); const instance = test( React.createElement(Inner, {name: 'foo'}), - 'DIV','foo' + 'DIV', + 'foo' + ); + expect(() => + expect(() => instance.replaceState({})).toThrow() + ).toLowPriorityWarnDev( + 'replaceState(...) is deprecated in plain JavaScript React classes' + ); + expect(() => + expect(() => instance.isMounted()).toThrow() + ).toLowPriorityWarnDev( + 'isMounted(...) is deprecated in plain JavaScript React classes' ); - expect(() => instance.replaceState({})).toThrow(); - expect(() => instance.isMounted()).toThrow(); - if (__DEV__) { - expect((console.warn).calls.count()).toBe(2); - expect((console.warn).calls.argsFor(0)[0]).toContain( - 'replaceState(...) is deprecated in plain JavaScript React classes' - ); - expect((console.warn).calls.argsFor(1)[0]).toContain( - 'isMounted(...) is deprecated in plain JavaScript React classes' - ); - } }); it('supports this.context passed via getChildContext', function() { @@ -560,5 +519,4 @@ describe('ReactTypeScriptClass', function() { const node = ReactDOM.findDOMNode(instance); expect(node).toBe(container.firstChild); }); - }); diff --git a/scripts/jest/spec-equivalence-reporter/setupTests.js b/scripts/jest/spec-equivalence-reporter/setupTests.js index 753eea70e40cc..e91d34b1b11bf 100644 --- a/scripts/jest/spec-equivalence-reporter/setupTests.js +++ b/scripts/jest/spec-equivalence-reporter/setupTests.js @@ -45,6 +45,10 @@ global.spyOnProd = function(...args) { } }; +expect.extend({ + ...require('../matchers/toWarnDev'), +}); + beforeEach(() => (numExpectations = 0)); jasmine.currentEnv_.addReporter({ diff --git a/scripts/jest/typescript/jest.d.ts b/scripts/jest/typescript/jest.d.ts index 28096ec177014..2426374c4d2fc 100644 --- a/scripts/jest/typescript/jest.d.ts +++ b/scripts/jest/typescript/jest.d.ts @@ -21,6 +21,8 @@ interface Expect { not: Expect toThrow(message?: string): void toThrowError(message?: string): void + toWarnDev(message?: string | Array): void + toLowPriorityWarnDev(message?: string | Array): void toBe(value: any): void toEqual(value: any): void toBeFalsy(): void From cdba8b4ad4a4771d278d2f43ff7e60caa74e85c5 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 19 Dec 2017 12:58:56 -0800 Subject: [PATCH 13/17] Upgraded a few additional tests --- .../react-art/src/__tests__/ReactART-test.js | 77 +++++--------- .../__tests__/DOMPropertyOperations-test.js | 17 ++- .../src/__tests__/EventPluginHub-test.js | 20 ++-- .../__tests__/ReactChildReconciler-test.js | 100 ++++++------------ .../src/__tests__/ReactPureComponent-test.js | 37 +++---- 5 files changed, 90 insertions(+), 161 deletions(-) diff --git a/packages/react-art/src/__tests__/ReactART-test.js b/packages/react-art/src/__tests__/ReactART-test.js index 5aed8ca6b2261..22d4892b314d2 100644 --- a/packages/react-art/src/__tests__/ReactART-test.js +++ b/packages/react-art/src/__tests__/ReactART-test.js @@ -336,10 +336,6 @@ describe('ReactART', () => { }); describe('ReactARTComponents', () => { - function normalizeCodeLocInfo(str) { - return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); - } - it('should generate a with props for drawing the Circle', () => { const circle = renderer.create( , @@ -348,16 +344,13 @@ describe('ReactARTComponents', () => { }); it('should warn if radius is missing on a Circle component', () => { - spyOnDev(console, 'error'); - renderer.create(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual( - 'Warning: Failed prop type: The prop `radius` is marked as required in `Circle`, ' + - 'but its value is `undefined`.' + - '\n in Circle (at **)', - ); - } + expect(() => + renderer.create(), + ).toWarnDev( + 'Warning: Failed prop type: The prop `radius` is marked as required in `Circle`, ' + + 'but its value is `undefined`.' + + '\n in Circle (at **)', + ); }); it('should generate a with props for drawing the Rectangle', () => { @@ -368,21 +361,16 @@ describe('ReactARTComponents', () => { }); it('should warn if width/height is missing on a Rectangle component', () => { - spyOnDev(console, 'error'); - renderer.create(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(2); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual( - 'Warning: Failed prop type: The prop `width` is marked as required in `Rectangle`, ' + - 'but its value is `undefined`.' + - '\n in Rectangle (at **)', - ); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual( - 'Warning: Failed prop type: The prop `height` is marked as required in `Rectangle`, ' + - 'but its value is `undefined`.' + - '\n in Rectangle (at **)', - ); - } + expect(() => + renderer.create(), + ).toWarnDev([ + 'Warning: Failed prop type: The prop `width` is marked as required in `Rectangle`, ' + + 'but its value is `undefined`.' + + '\n in Rectangle (at **)', + 'Warning: Failed prop type: The prop `height` is marked as required in `Rectangle`, ' + + 'but its value is `undefined`.' + + '\n in Rectangle (at **)', + ]); }); it('should generate a with props for drawing the Wedge', () => { @@ -400,25 +388,16 @@ describe('ReactARTComponents', () => { }); it('should warn if outerRadius/startAngle/endAngle is missing on a Wedge component', () => { - spyOnDev(console, 'error'); - renderer.create(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(3); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toEqual( - 'Warning: Failed prop type: The prop `outerRadius` is marked as required in `Wedge`, ' + - 'but its value is `undefined`.' + - '\n in Wedge (at **)', - ); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toEqual( - 'Warning: Failed prop type: The prop `startAngle` is marked as required in `Wedge`, ' + - 'but its value is `undefined`.' + - '\n in Wedge (at **)', - ); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(2)[0])).toEqual( - 'Warning: Failed prop type: The prop `endAngle` is marked as required in `Wedge`, ' + - 'but its value is `undefined`.' + - '\n in Wedge (at **)', - ); - } + expect(() => renderer.create()).toWarnDev([ + 'Warning: Failed prop type: The prop `outerRadius` is marked as required in `Wedge`, ' + + 'but its value is `undefined`.' + + '\n in Wedge (at **)', + 'Warning: Failed prop type: The prop `startAngle` is marked as required in `Wedge`, ' + + 'but its value is `undefined`.' + + '\n in Wedge (at **)', + 'Warning: Failed prop type: The prop `endAngle` is marked as required in `Wedge`, ' + + 'but its value is `undefined`.' + + '\n in Wedge (at **)', + ]); }); }); diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 8cdf53f1c4ba8..bb7ce8ca544d9 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -151,25 +151,22 @@ describe('DOMPropertyOperations', () => { it('should not remove attributes for special properties', () => { const container = document.createElement('div'); - spyOnDev(console, 'error'); ReactDOM.render( , container, ); expect(container.firstChild.getAttribute('value')).toBe('foo'); expect(container.firstChild.value).toBe('foo'); - ReactDOM.render( - , - container, + expect(() => + ReactDOM.render( + , + container, + ), + ).toWarnDev( + 'A component is changing a controlled input of type text to be uncontrolled', ); expect(container.firstChild.getAttribute('value')).toBe('foo'); expect(container.firstChild.value).toBe('foo'); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'A component is changing a controlled input of type text to be uncontrolled', - ); - } }); }); }); diff --git a/packages/react-dom/src/__tests__/EventPluginHub-test.js b/packages/react-dom/src/__tests__/EventPluginHub-test.js index 3fb1aa72c4501..65a3e9246dd04 100644 --- a/packages/react-dom/src/__tests__/EventPluginHub-test.js +++ b/packages/react-dom/src/__tests__/EventPluginHub-test.js @@ -22,21 +22,17 @@ describe('EventPluginHub', () => { }); it('should prevent non-function listeners, at dispatch', () => { - spyOnDev(console, 'error'); - const node = ReactTestUtils.renderIntoDocument( -
, + let node; + expect(() => { + node = ReactTestUtils.renderIntoDocument( +
, + ); + }).toWarnDev( + 'Expected `onClick` listener to be a function, instead got a value of `string` type.', ); - expect(function() { - ReactTestUtils.SimulateNative.click(node); - }).toThrowError( + expect(() => ReactTestUtils.SimulateNative.click(node)).toThrowError( 'Expected `onClick` listener to be a function, instead got a value of `string` type.', ); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Expected `onClick` listener to be a function, instead got a value of `string` type.', - ); - } }); it('should not prevent null listeners, at dispatch', () => { diff --git a/packages/react-dom/src/__tests__/ReactChildReconciler-test.js b/packages/react-dom/src/__tests__/ReactChildReconciler-test.js index ebc03fa0ff99a..20c7e246313a0 100644 --- a/packages/react-dom/src/__tests__/ReactChildReconciler-test.js +++ b/packages/react-dom/src/__tests__/ReactChildReconciler-test.js @@ -16,10 +16,6 @@ let React; let ReactTestUtils; describe('ReactChildReconciler', () => { - function normalizeCodeLocInfo(str) { - return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); - } - beforeEach(() => { jest.resetModules(); @@ -46,30 +42,21 @@ describe('ReactChildReconciler', () => { } it('warns for duplicated array keys', () => { - spyOnDev(console, 'error'); - class Component extends React.Component { render() { return
{[
,
]}
; } } - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Keys should be unique so that components maintain their identity ' + - 'across updates. Non-unique keys may cause children to be ' + - 'duplicated and/or omitted — the behavior is unsupported and ' + - 'could change in a future version.', - ); - } + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Keys should be unique so that components maintain their identity ' + + 'across updates. Non-unique keys may cause children to be ' + + 'duplicated and/or omitted — the behavior is unsupported and ' + + 'could change in a future version.', + ); }); it('warns for duplicated array keys with component stack info', () => { - spyOnDev(console, 'error'); - class Component extends React.Component { render() { return
{[
,
]}
; @@ -88,49 +75,35 @@ describe('ReactChildReconciler', () => { } } - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toContain( - 'Encountered two children with the same key, `1`. ' + - 'Keys should be unique so that components maintain their identity ' + - 'across updates. Non-unique keys may cause children to be ' + - 'duplicated and/or omitted — the behavior is unsupported and ' + - 'could change in a future version.', - ' in div (at **)\n' + - ' in Component (at **)\n' + - ' in Parent (at **)\n' + - ' in GrandParent (at **)', - ); - } + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Encountered two children with the same key, `1`. ' + + 'Keys should be unique so that components maintain their identity ' + + 'across updates. Non-unique keys may cause children to be ' + + 'duplicated and/or omitted — the behavior is unsupported and ' + + 'could change in a future version.', + ' in div (at **)\n' + + ' in Component (at **)\n' + + ' in Parent (at **)\n' + + ' in GrandParent (at **)', + ); }); it('warns for duplicated iterable keys', () => { - spyOnDev(console, 'error'); - class Component extends React.Component { render() { return
{createIterable([
,
])}
; } } - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Keys should be unique so that components maintain their identity ' + - 'across updates. Non-unique keys may cause children to be ' + - 'duplicated and/or omitted — the behavior is unsupported and ' + - 'could change in a future version.', - ); - } + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Keys should be unique so that components maintain their identity ' + + 'across updates. Non-unique keys may cause children to be ' + + 'duplicated and/or omitted — the behavior is unsupported and ' + + 'could change in a future version.', + ); }); it('warns for duplicated iterable keys with component stack info', () => { - spyOnDev(console, 'error'); - class Component extends React.Component { render() { return
{createIterable([
,
])}
; @@ -149,21 +122,16 @@ describe('ReactChildReconciler', () => { } } - ReactTestUtils.renderIntoDocument(); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toContain( - 'Encountered two children with the same key, `1`. ' + - 'Keys should be unique so that components maintain their identity ' + - 'across updates. Non-unique keys may cause children to be ' + - 'duplicated and/or omitted — the behavior is unsupported and ' + - 'could change in a future version.', - ' in div (at **)\n' + - ' in Component (at **)\n' + - ' in Parent (at **)\n' + - ' in GrandParent (at **)', - ); - } + expect(() => ReactTestUtils.renderIntoDocument()).toWarnDev( + 'Encountered two children with the same key, `1`. ' + + 'Keys should be unique so that components maintain their identity ' + + 'across updates. Non-unique keys may cause children to be ' + + 'duplicated and/or omitted — the behavior is unsupported and ' + + 'could change in a future version.', + ' in div (at **)\n' + + ' in Component (at **)\n' + + ' in Parent (at **)\n' + + ' in GrandParent (at **)', + ); }); }); diff --git a/packages/react/src/__tests__/ReactPureComponent-test.js b/packages/react/src/__tests__/ReactPureComponent-test.js index cfa7325619dca..776f7a2cc1fd3 100644 --- a/packages/react/src/__tests__/ReactPureComponent-test.js +++ b/packages/react/src/__tests__/ReactPureComponent-test.js @@ -62,7 +62,6 @@ describe('ReactPureComponent', () => { }); it('can override shouldComponentUpdate', () => { - spyOnDev(console, 'error'); let renders = 0; class Component extends React.PureComponent { render() { @@ -73,18 +72,15 @@ describe('ReactPureComponent', () => { return true; } } + const container = document.createElement('div'); + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Warning: ' + + 'Component has a method called shouldComponentUpdate(). ' + + 'shouldComponentUpdate should not be used when extending React.PureComponent. ' + + 'Please extend React.Component if shouldComponentUpdate is used.', + ); ReactDOM.render(, container); - ReactDOM.render(, container); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: ' + - 'Component has a method called shouldComponentUpdate(). ' + - 'shouldComponentUpdate should not be used when extending React.PureComponent. ' + - 'Please extend React.Component if shouldComponentUpdate is used.', - ); - } expect(renders).toBe(2); }); @@ -103,8 +99,6 @@ describe('ReactPureComponent', () => { }); it('should warn when shouldComponentUpdate is defined on React.PureComponent', () => { - spyOnDev(console, 'error'); - class PureComponent extends React.PureComponent { shouldComponentUpdate() { return true; @@ -114,16 +108,11 @@ describe('ReactPureComponent', () => { } } const container = document.createElement('div'); - ReactDOM.render(, container); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: ' + - 'PureComponent has a method called shouldComponentUpdate(). ' + - 'shouldComponentUpdate should not be used when extending React.PureComponent. ' + - 'Please extend React.Component if shouldComponentUpdate is used.', - ); - } + expect(() => ReactDOM.render(, container)).toWarnDev( + 'Warning: ' + + 'PureComponent has a method called shouldComponentUpdate(). ' + + 'shouldComponentUpdate should not be used when extending React.PureComponent. ' + + 'Please extend React.Component if shouldComponentUpdate is used.', + ); }); }); From 68fc971eef7e0cb5d5075730a70f34669046fd69 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 19 Dec 2017 15:12:56 -0800 Subject: [PATCH 14/17] More robustly handle unexpected warnings within try/catch --- scripts/jest/setupTests.js | 55 +++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index 4e6f7373c6c17..c02f818a0fcc9 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -1,5 +1,7 @@ 'use strict'; +const chalk = require('chalk'); + if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { // Inside the class equivalence tester, we have a custom environment, let's // require that instead. @@ -60,28 +62,57 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { }); ['error', 'warn'].forEach(methodName => { - const oldMethod = console[methodName]; - const newMethod = function(...args) { - oldMethod(...args); - - throw new Error( - `Expected test not to call console.${methodName}(). ` + - 'If the warning is expected, mock it out using ' + - `spyOnDev(console, '${methodName}') or spyOnProd(console, '${ - methodName - }'), ` + - 'and test that the warning occurs.' - ); + const unexpectedConsoleCallStacks = []; + const newMethod = function(message) { + // Capture the call stack now so we can warn about it later. + // The call stack has helpful information for the test author. + // Don't throw yet though b'c it might be accidentally caught and suppressed. + const stack = new Error().stack; + unexpectedConsoleCallStacks.push([ + stack.substr(stack.indexOf('\n') + 1), + message, + ]); }; console[methodName] = newMethod; + env.beforeEach(() => { + unexpectedConsoleCallStacks.splice(0); + }); + env.afterEach(() => { if (console[methodName] !== newMethod && !isSpy(console[methodName])) { throw new Error( `Test did not tear down console.${methodName} mock properly.` ); } + + if (unexpectedConsoleCallStacks.length > 0) { + const messages = unexpectedConsoleCallStacks.map( + ([stack, message]) => + `${chalk.red(message)}\n` + + `${stack + .split('\n') + .map(line => chalk.gray(line)) + .join('\n')}` + ); + + const message = + `Expected test not to call ${chalk.bold( + `console.${methodName}()` + )}.\n\n` + + 'If the warning is expected, test for it explicitly by:\n' + + `1. Using the ${chalk.bold('.toWarnDev()')} / ${chalk.bold( + '.toLowPriorityWarnDev()' + )} matchers, or...\n` + + `2. Mock it out using ${chalk.bold('spyOnDev')}(console, '${ + methodName + }') or ${chalk.bold('spyOnProd')}(console, '${ + methodName + }'), and test that the warning occurs.`; + + throw new Error(`${message}\n\n${messages.join('\n\n')}`); + } }); }); From 62c4b4fd6d1ec2e2ebb861d62dfebabf5a805c77 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 2 Jan 2018 10:42:29 -0800 Subject: [PATCH 15/17] Replaced .splice(0) with .length=0 --- scripts/jest/setupTests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index c02f818a0fcc9..40ccf4c8f5685 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -77,7 +77,7 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { console[methodName] = newMethod; env.beforeEach(() => { - unexpectedConsoleCallStacks.splice(0); + unexpectedConsoleCallStacks.length = 0; }); env.afterEach(() => { From b76696456a6480e75f2a460e6869b8c4ecb1fc9c Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 2 Jan 2018 10:51:01 -0800 Subject: [PATCH 16/17] Error message includes remaining expected warnings in addition to unexpected warning --- scripts/jest/matchers/toWarnDev.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js index ebc070bf1dd91..8329df461d531 100644 --- a/scripts/jest/matchers/toWarnDev.js +++ b/scripts/jest/matchers/toWarnDev.js @@ -32,7 +32,13 @@ const createMatcherFor = consoleMethod => } // Fail early for unexpected warnings to preserve the call stack. - throw Error(`Unexpected warning recorded: "${message}"`); + throw Error( + `Unexpected warning recorded:\n "${this.utils.printReceived( + message + )}"\n\nThe following expected warnings were not yet seen:\n ${this.utils.printExpected( + expectedMessages.join('\n') + )}` + ); }; // TODO Decide whether we need to support nested toWarn* expectations. From eeeb0e38f7430899b69e0bc7fcaa48bcba5f4a49 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 2 Jan 2018 10:58:00 -0800 Subject: [PATCH 17/17] Removed unnecessary "" around unexpected error message --- scripts/jest/matchers/toWarnDev.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js index 8329df461d531..5f4369bf18a4b 100644 --- a/scripts/jest/matchers/toWarnDev.js +++ b/scripts/jest/matchers/toWarnDev.js @@ -33,9 +33,9 @@ const createMatcherFor = consoleMethod => // Fail early for unexpected warnings to preserve the call stack. throw Error( - `Unexpected warning recorded:\n "${this.utils.printReceived( + `Unexpected warning recorded:\n ${this.utils.printReceived( message - )}"\n\nThe following expected warnings were not yet seen:\n ${this.utils.printExpected( + )}\n\nThe following expected warnings were not yet seen:\n ${this.utils.printExpected( expectedMessages.join('\n') )}` );