From 9f848f8ebec30b3aaa4844ecaef83b014359c5e3 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 3 Jan 2018 13:55:37 -0800 Subject: [PATCH] Update additional tests to use .toWarnDev() matcher (#11957) * Migrated several additional tests to use new .toWarnDev() matcher * Migrated ReactDOMComponent-test to use .toWarnDev() matcher Note this test previous had some hacky logic to verify errors were reported against unique line numbers. Since the new matcher doesn't suppor this, I replaced this check with an equivalent (I think) comparison of unique DOM elements (eg div -> span) * Updated several additional tests to use the new .toWarnDev() matcher * Updated many more tests to use .toWarnDev() * Updated several additional tests to use .toWarnDev() matcher * Updated ReactElementValidator to distinguish between Array and Object in its warning. Also updated its test to use .toWarnDev() matcher. * Updated a couple of additional tests * Removed unused normalizeCodeLocInfo() methods --- .../ReactCompositeComponentState-test.js | 2 - .../__tests__/ReactErrorBoundaries-test.js | 14 +- .../src/__tests__/ReactMultiChildText-test.js | 188 ++++++++-------- .../src/__tests__/ReactRenderDocument-test.js | 99 +++------ .../__tests__/ReactServerRendering-test.js | 101 ++++----- .../ReactServerRenderingHydration.js | 61 ++--- .../__tests__/ReactStatelessComponent-test.js | 210 +++++++----------- .../src/__tests__/ReactUpdates-test.js | 84 +++---- .../__tests__/TapEventPlugin-test.internal.js | 20 +- .../src/__tests__/validateDOMNesting-test.js | 21 +- .../events/__tests__/SyntheticEvent-test.js | 147 ++++++------ .../src/__tests__/ReactFragment-test.js | 32 +-- .../ReactIncrementalErrorHandling-test.js | 39 +--- .../ReactIncrementalSideEffects-test.js | 25 +-- .../__tests__/ReactIncrementalUpdates-test.js | 23 +- .../__tests__/ReactShallowRenderer-test.js | 83 +++---- .../src/__tests__/ReactTestRenderer-test.js | 21 +- packages/react/src/ReactElementValidator.js | 11 +- .../__tests__/ReactElementValidator-test.js | 2 - .../__tests__/ReactDOMFrameScheduling-test.js | 11 +- 20 files changed, 463 insertions(+), 731 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js index c7135984d2760..87a09f944875f 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js @@ -423,8 +423,6 @@ describe('ReactCompositeComponent-state', () => { }); it('should treat assigning to this.state inside cWM as a replaceState, with a warning', () => { - spyOnDev(console, 'error'); - let ops = []; class Test extends React.Component { state = {step: 1, extra: true}; diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js index 04cfc5525e6a1..13ce4422ded61 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js @@ -1909,8 +1909,6 @@ describe('ReactErrorBoundaries', () => { }); it('discards a bad root if the root component fails', () => { - spyOnDev(console, 'error'); - const X = null; const Y = undefined; let err1; @@ -1918,13 +1916,21 @@ describe('ReactErrorBoundaries', () => { try { let container = document.createElement('div'); - ReactDOM.render(, container); + expect(() => ReactDOM.render(, container)).toWarnDev( + 'React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function ' + + '(for composite components) but got: null.', + ); } catch (err) { err1 = err; } try { let container = document.createElement('div'); - ReactDOM.render(, container); + expect(() => ReactDOM.render(, container)).toWarnDev( + 'React.createElement: type is invalid -- expected a string ' + + '(for built-in components) or a class/function ' + + '(for composite components) but got: undefined.', + ); } catch (err) { err2 = err; } diff --git a/packages/react-dom/src/__tests__/ReactMultiChildText-test.js b/packages/react-dom/src/__tests__/ReactMultiChildText-test.js index 1900cda4e4992..d89a7e8dda230 100644 --- a/packages/react-dom/src/__tests__/ReactMultiChildText-test.js +++ b/packages/react-dom/src/__tests__/ReactMultiChildText-test.js @@ -73,104 +73,98 @@ const expectChildren = function(container, children) { */ describe('ReactMultiChildText', () => { it('should correctly handle all possible children for render and update', () => { - spyOnDev(console, 'error'); - // prettier-ignore - testAllPermutations([ - // basic values - undefined, [], - null, [], - false, [], - true, [], - 0, '0', - 1.2, '1.2', - '', '', - 'foo', 'foo', - - [], [], - [undefined], [], - [null], [], - [false], [], - [true], [], - [0], ['0'], - [1.2], ['1.2'], - [''], [''], - ['foo'], ['foo'], - [
], [
], - - // two adjacent values - [true, 0], ['0'], - [0, 0], ['0', '0'], - [1.2, 0], ['1.2', '0'], - [0, ''], ['0', ''], - ['foo', 0], ['foo', '0'], - [0,
], ['0',
], - - [true, 1.2], ['1.2'], - [1.2, 0], ['1.2', '0'], - [1.2, 1.2], ['1.2', '1.2'], - [1.2, ''], ['1.2', ''], - ['foo', 1.2], ['foo', '1.2'], - [1.2,
], ['1.2',
], - - [true, ''], [''], - ['', 0], ['', '0'], - [1.2, ''], ['1.2', ''], - ['', ''], ['', ''], - ['foo', ''], ['foo', ''], - ['',
], ['',
], - - [true, 'foo'], ['foo'], - ['foo', 0], ['foo', '0'], - [1.2, 'foo'], ['1.2', 'foo'], - ['foo', ''], ['foo', ''], - ['foo', 'foo'], ['foo', 'foo'], - ['foo',
], ['foo',
], - - // values separated by an element - [true,
, true], [
], - [1.2,
, 1.2], ['1.2',
, '1.2'], - ['',
, ''], ['',
, ''], - ['foo',
, 'foo'], ['foo',
, 'foo'], - - [true, 1.2,
, '', 'foo'], ['1.2',
, '', 'foo'], - [1.2, '',
, 'foo', true], ['1.2', '',
, 'foo'], - ['', 'foo',
, true, 1.2], ['', 'foo',
, '1.2'], - - [true, 1.2, '',
, 'foo', true, 1.2], ['1.2', '',
, 'foo', '1.2'], - ['', 'foo', true,
, 1.2, '', 'foo'], ['', 'foo',
, '1.2', '', 'foo'], - - // values inside arrays - [[true], [true]], [], - [[1.2], [1.2]], ['1.2', '1.2'], - [[''], ['']], ['', ''], - [['foo'], ['foo']], ['foo', 'foo'], - [[
], [
]], [
,
], - - [[true, 1.2,
], '', 'foo'], ['1.2',
, '', 'foo'], - [1.2, '', [
, 'foo', true]], ['1.2', '',
, 'foo'], - ['', ['foo',
, true], 1.2], ['', 'foo',
, '1.2'], - - [true, [1.2, '',
, 'foo'], true, 1.2], ['1.2', '',
, 'foo', '1.2'], - ['', 'foo', [true,
, 1.2, ''], 'foo'], ['', 'foo',
, '1.2', '', 'foo'], - - // values inside elements - [
{true}{1.2}{
}
, '', 'foo'], [
, '', 'foo'], - [1.2, '',
{
}{'foo'}{true}
], ['1.2', '',
], - ['',
{'foo'}{
}{true}
, 1.2], ['',
, '1.2'], - - [true,
{1.2}{''}{
}{'foo'}
, true, 1.2], [
, '1.2'], - ['', 'foo',
{true}{
}{1.2}{''}
, 'foo'], ['', 'foo',
, 'foo'], + expect(() => { + // prettier-ignore + testAllPermutations([ + // basic values + undefined, [], + null, [], + false, [], + true, [], + 0, '0', + 1.2, '1.2', + '', '', + 'foo', 'foo', + + [], [], + [undefined], [], + [null], [], + [false], [], + [true], [], + [0], ['0'], + [1.2], ['1.2'], + [''], [''], + ['foo'], ['foo'], + [
], [
], + + // two adjacent values + [true, 0], ['0'], + [0, 0], ['0', '0'], + [1.2, 0], ['1.2', '0'], + [0, ''], ['0', ''], + ['foo', 0], ['foo', '0'], + [0,
], ['0',
], + + [true, 1.2], ['1.2'], + [1.2, 0], ['1.2', '0'], + [1.2, 1.2], ['1.2', '1.2'], + [1.2, ''], ['1.2', ''], + ['foo', 1.2], ['foo', '1.2'], + [1.2,
], ['1.2',
], + + [true, ''], [''], + ['', 0], ['', '0'], + [1.2, ''], ['1.2', ''], + ['', ''], ['', ''], + ['foo', ''], ['foo', ''], + ['',
], ['',
], + + [true, 'foo'], ['foo'], + ['foo', 0], ['foo', '0'], + [1.2, 'foo'], ['1.2', 'foo'], + ['foo', ''], ['foo', ''], + ['foo', 'foo'], ['foo', 'foo'], + ['foo',
], ['foo',
], + + // values separated by an element + [true,
, true], [
], + [1.2,
, 1.2], ['1.2',
, '1.2'], + ['',
, ''], ['',
, ''], + ['foo',
, 'foo'], ['foo',
, 'foo'], + + [true, 1.2,
, '', 'foo'], ['1.2',
, '', 'foo'], + [1.2, '',
, 'foo', true], ['1.2', '',
, 'foo'], + ['', 'foo',
, true, 1.2], ['', 'foo',
, '1.2'], + + [true, 1.2, '',
, 'foo', true, 1.2], ['1.2', '',
, 'foo', '1.2'], + ['', 'foo', true,
, 1.2, '', 'foo'], ['', 'foo',
, '1.2', '', 'foo'], + + // values inside arrays + [[true], [true]], [], + [[1.2], [1.2]], ['1.2', '1.2'], + [[''], ['']], ['', ''], + [['foo'], ['foo']], ['foo', 'foo'], + [[
], [
]], [
,
], + + [[true, 1.2,
], '', 'foo'], ['1.2',
, '', 'foo'], + [1.2, '', [
, 'foo', true]], ['1.2', '',
, 'foo'], + ['', ['foo',
, true], 1.2], ['', 'foo',
, '1.2'], + + [true, [1.2, '',
, 'foo'], true, 1.2], ['1.2', '',
, 'foo', '1.2'], + ['', 'foo', [true,
, 1.2, ''], 'foo'], ['', 'foo',
, '1.2', '', 'foo'], + + // values inside elements + [
{true}{1.2}{
}
, '', 'foo'], [
, '', 'foo'], + [1.2, '',
{
}{'foo'}{true}
], ['1.2', '',
], + ['',
{'foo'}{
}{true}
, 1.2], ['',
, '1.2'], + + [true,
{1.2}{''}{
}{'foo'}
, true, 1.2], [
, '1.2'], + ['', 'foo',
{true}{
}{1.2}{''}
, 'foo'], ['', 'foo',
, 'foo'], + ]); + }).toWarnDev([ + 'Warning: Each child in an array or iterator should have a unique "key" prop.', + 'Warning: Each child in an array or iterator should have a unique "key" prop.', ]); - - if (__DEV__) { - expect(console.error.calls.count()).toBe(2); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Each child in an array or iterator should have a unique "key" prop.', - ); - expect(console.error.calls.argsFor(1)[0]).toContain( - 'Warning: Each child in an array or iterator should have a unique "key" prop.', - ); - } }); it('should throw if rendering both HTML and children', () => { diff --git a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js index 3ff4c0d7b2894..c9b8cb89f3c53 100644 --- a/packages/react-dom/src/__tests__/ReactRenderDocument-test.js +++ b/packages/react-dom/src/__tests__/ReactRenderDocument-test.js @@ -34,19 +34,15 @@ describe('rendering React components at document', () => { }); describe('with old implicit hydration API', () => { - function expectDeprecationWarningWithFiber() { - if (__DEV__) { - expect(console.warn.calls.count()).toBe(1); - expect(console.warn.calls.argsFor(0)[0]).toContain( - 'render(): Calling ReactDOM.render() to hydrate server-rendered markup ' + - 'will stop working in React v17. Replace the ReactDOM.render() call ' + - 'with ReactDOM.hydrate() if you want React to attach to the server HTML.', - ); - } + function expectDeprecationWarningWithFiber(callback) { + expect(callback).toLowPriorityWarnDev( + 'render(): Calling ReactDOM.render() to hydrate server-rendered markup ' + + 'will stop working in React v17. Replace the ReactDOM.render() call ' + + 'with ReactDOM.hydrate() if you want React to attach to the server HTML.', + ); } it('should be able to adopt server markup', () => { - spyOnDev(console, 'warn'); class Root extends React.Component { render() { return ( @@ -64,18 +60,18 @@ describe('rendering React components at document', () => { const testDocument = getTestDocument(markup); const body = testDocument.body; - ReactDOM.render(, testDocument); + expectDeprecationWarningWithFiber(() => + ReactDOM.render(, testDocument), + ); expect(testDocument.body.innerHTML).toBe('Hello world'); ReactDOM.render(, testDocument); expect(testDocument.body.innerHTML).toBe('Hello moon'); expect(body === testDocument.body).toBe(true); - expectDeprecationWarningWithFiber(); }); it('should not be able to unmount component from document node', () => { - spyOnDev(console, 'warn'); class Root extends React.Component { render() { return ( @@ -91,18 +87,17 @@ describe('rendering React components at document', () => { const markup = ReactDOMServer.renderToString(); const testDocument = getTestDocument(markup); - ReactDOM.render(, testDocument); + expectDeprecationWarningWithFiber(() => + ReactDOM.render(, testDocument), + ); expect(testDocument.body.innerHTML).toBe('Hello world'); // In Fiber this actually works. It might not be a good idea though. ReactDOM.unmountComponentAtNode(testDocument); expect(testDocument.firstChild).toBe(null); - - expectDeprecationWarningWithFiber(); }); it('should not be able to switch root constructors', () => { - spyOnDev(console, 'warn'); class Component extends React.Component { render() { return ( @@ -132,18 +127,18 @@ describe('rendering React components at document', () => { const markup = ReactDOMServer.renderToString(); const testDocument = getTestDocument(markup); - ReactDOM.render(, testDocument); + expectDeprecationWarningWithFiber(() => + ReactDOM.render(, testDocument), + ); expect(testDocument.body.innerHTML).toBe('Hello world'); // This works but is probably a bad idea. ReactDOM.render(, testDocument); expect(testDocument.body.innerHTML).toBe('Goodbye world'); - expectDeprecationWarningWithFiber(); }); it('should be able to mount into document', () => { - spyOnDev(console, 'warn'); class Component extends React.Component { render() { return ( @@ -162,10 +157,11 @@ describe('rendering React components at document', () => { ); const testDocument = getTestDocument(markup); - ReactDOM.render(, testDocument); + expectDeprecationWarningWithFiber(() => + ReactDOM.render(, testDocument), + ); expect(testDocument.body.innerHTML).toBe('Hello world'); - expectDeprecationWarningWithFiber(); }); it('renders over an existing text child without throwing', () => { @@ -197,22 +193,15 @@ describe('rendering React components at document', () => { ); const testDocument = getTestDocument(markup); - spyOnDev(console, 'warn'); - spyOnDev(console, 'error'); - ReactDOM.render(, testDocument); - expect(testDocument.body.innerHTML).toBe('Hello world'); - if (__DEV__) { - expect(console.warn.calls.count()).toBe(1); - expect(console.warn.calls.argsFor(0)[0]).toContain( + expect(() => { + expect(() => + ReactDOM.render(, testDocument), + ).toLowPriorityWarnDev( 'render(): Calling ReactDOM.render() to hydrate server-rendered markup ' + 'will stop working in React v17. Replace the ReactDOM.render() call ' + 'with ReactDOM.hydrate() if you want React to attach to the server HTML.', ); - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Text content did not match.', - ); - } + }).toWarnDev('Warning: Text content did not match.'); }); it('should throw on full document render w/ no markup', () => { @@ -239,7 +228,6 @@ describe('rendering React components at document', () => { }); it('supports findDOMNode on full-page components', () => { - spyOnDev(console, 'warn'); const tree = ( @@ -251,10 +239,12 @@ describe('rendering React components at document', () => { const markup = ReactDOMServer.renderToString(tree); const testDocument = getTestDocument(markup); - const component = ReactDOM.render(tree, testDocument); + let component; + expectDeprecationWarningWithFiber(() => { + component = ReactDOM.render(tree, testDocument); + }); expect(testDocument.body.innerHTML).toBe('Hello world'); expect(ReactDOM.findDOMNode(component).tagName).toBe('HTML'); - expectDeprecationWarningWithFiber(); }); }); @@ -375,17 +365,12 @@ describe('rendering React components at document', () => { }); it('renders over an existing text child without throwing', () => { - spyOnDev(console, 'error'); const container = document.createElement('div'); container.textContent = 'potato'; - ReactDOM.hydrate(
parsnip
, container); + expect(() => ReactDOM.hydrate(
parsnip
, container)).toWarnDev( + 'Expected server HTML to contain a matching
in
.', + ); expect(container.textContent).toBe('parsnip'); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Expected server HTML to contain a matching
in
.', - ); - } }); it('should give helpful errors on state desync', () => { @@ -407,19 +392,13 @@ describe('rendering React components at document', () => { ); const testDocument = getTestDocument(markup); - spyOnDev(console, 'error'); - ReactDOM.hydrate(, testDocument); + expect(() => + ReactDOM.hydrate(, testDocument), + ).toWarnDev('Warning: Text content did not match.'); expect(testDocument.body.innerHTML).toBe('Hello world'); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - 'Warning: Text content did not match.', - ); - } }); it('should render w/ no markup to full document', () => { - spyOnDev(console, 'error'); const testDocument = getTestDocument(); class Component extends React.Component { @@ -435,15 +414,11 @@ describe('rendering React components at document', () => { } } - ReactDOM.hydrate(, testDocument); + // getTestDocument() has an extra that we didn't render. + expect(() => + ReactDOM.hydrate(, testDocument), + ).toWarnDev('Did not expect server HTML to contain a in .'); expect(testDocument.body.innerHTML).toBe('Hello world'); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toContain( - // getTestDocument() has an extra that we didn't render. - 'Did not expect server HTML to contain a in .', - ); - } }); it('supports findDOMNode on full-page components', () => { diff --git a/packages/react-dom/src/__tests__/ReactServerRendering-test.js b/packages/react-dom/src/__tests__/ReactServerRendering-test.js index 642d950f229b6..cfba21e0523ef 100644 --- a/packages/react-dom/src/__tests__/ReactServerRendering-test.js +++ b/packages/react-dom/src/__tests__/ReactServerRendering-test.js @@ -431,24 +431,17 @@ describe('ReactDOMServer', () => { } } - spyOnDev(console, 'error'); ReactDOMServer.renderToString(); - jest.runOnlyPendingTimers(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.mostRecent().args[0]).toBe( - 'Warning: setState(...): Can only update a mounting component.' + - ' This usually means you called setState() outside componentWillMount() on the server.' + - ' This is a no-op.\n\nPlease check the code for the Foo component.', - ); - } + expect(() => jest.runOnlyPendingTimers()).toWarnDev( + 'Warning: setState(...): Can only update a mounting component.' + + ' This usually means you called setState() outside componentWillMount() on the server.' + + ' This is a no-op.\n\nPlease check the code for the Foo component.', + ); const markup = ReactDOMServer.renderToStaticMarkup(); expect(markup).toBe('
hello
'); + // No additional warnings are expected jest.runOnlyPendingTimers(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - } }); it('warns with a no-op when an async forceUpdate is triggered', () => { @@ -465,23 +458,17 @@ describe('ReactDOMServer', () => { } } - spyOnDev(console, 'error'); ReactDOMServer.renderToString(); - jest.runOnlyPendingTimers(); - if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.mostRecent().args[0]).toBe( - 'Warning: forceUpdate(...): Can only update a mounting component. ' + - 'This usually means you called forceUpdate() outside componentWillMount() on the server. ' + - 'This is a no-op.\n\nPlease check the code for the Baz component.', - ); - } + expect(() => jest.runOnlyPendingTimers()).toWarnDev( + 'Warning: forceUpdate(...): Can only update a mounting component. ' + + 'This usually means you called forceUpdate() outside componentWillMount() on the server. ' + + 'This is a no-op.\n\nPlease check the code for the Baz component.', + ); const markup = ReactDOMServer.renderToStaticMarkup(); expect(markup).toBe('
'); }); it('should throw (in dev) when children are mutated during render', () => { - spyOnDev(console, 'error'); function Wrapper(props) { props.children[1] =

; // Mutation is illegal return

{props.children}
; @@ -510,51 +497,43 @@ describe('ReactDOMServer', () => { }); it('warns about lowercase html but not in svg tags', () => { - spyOnDev(console, 'error'); function CompositeG(props) { // Make sure namespace passes through composites return {props.children}; } - ReactDOMServer.renderToStaticMarkup( -
- - - - - - {/* back to HTML */} -