Skip to content

Commit

Permalink
Update additional tests to use .toWarnDev() matcher (facebook#11952)
Browse files Browse the repository at this point in the history 
* 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()
  • Loading branch information
@bvaughn @yenshih
bvaughn authored and yenshih committed Jan 6, 2018
1 parent 8296477 commit 0fbd864
Show file tree
Hide file tree
Showing 19 changed files with 1,410 additions and 2,243 deletions.
4 changes: 0 additions & 4 deletions packages/react-dom/src/__tests__/ReactBrowserEventEmitter-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,11 @@ describe('ReactBrowserEventEmitter', () => {
putListener(CHILD, ON_CLICK_KEY, recordIDAndReturnFalse.bind(null, CHILD));
putListener(PARENT, ON_CLICK_KEY, recordID.bind(null, PARENT));
putListener(GRANDPARENT, ON_CLICK_KEY, recordID.bind(null, GRANDPARENT));
spyOnDev(console, 'error');
ReactTestUtils.Simulate.click(CHILD);
expect(idCallOrder.length).toBe(3);
expect(idCallOrder[0]).toBe(CHILD);
expect(idCallOrder[1]).toBe(PARENT);
expect(idCallOrder[2]).toBe(GRANDPARENT);
if (__DEV__) {
expect(console.error.calls.count()).toEqual(0);
}
});

/**
Expand Down
129 changes: 54 additions & 75 deletions packages/react-dom/src/__tests__/ReactComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ describe('ReactComponent', () => {
});

it('should throw (in dev) when children are mutated during render', () => {
spyOnDev(console, 'error');
function Wrapper(props) {
props.children[1] = <p key={1} />; // Mutation is illegal
return <div>{props.children}</div>;
Expand All @@ -73,8 +72,6 @@ describe('ReactComponent', () => {
});

it('should throw (in dev) when children are mutated during update', () => {
spyOnDev(console, 'error');

class Wrapper extends React.Component {
componentDidMount() {
this.props.children[1] = <p key={1} />; // Mutation is illegal
Expand Down Expand Up @@ -355,10 +352,13 @@ describe('ReactComponent', () => {
});

it('throws usefully when rendering badly-typed elements', () => {
spyOnDev(console, 'error');

const X = undefined;
expect(() => ReactTestUtils.renderIntoDocument(<X />)).toThrowError(
expect(() => {
expect(() => ReactTestUtils.renderIntoDocument(<X />)).toWarnDev(
'React.createElement: type is invalid -- expected a string (for built-in components) ' +
'or a class/function (for composite components) but got: undefined.',
);
}).toThrowError(
'Element type is invalid: expected a string (for built-in components) ' +
'or a class/function (for composite components) but got: undefined.' +
(__DEV__
Expand All @@ -368,20 +368,18 @@ describe('ReactComponent', () => {
);

const Y = null;
expect(() => ReactTestUtils.renderIntoDocument(<Y />)).toThrowError(
expect(() => {
expect(() => ReactTestUtils.renderIntoDocument(<Y />)).toWarnDev(
'React.createElement: type is invalid -- expected a string (for built-in components) ' +
'or a class/function (for composite components) but got: null.',
);
}).toThrowError(
'Element type is invalid: expected a string (for built-in components) ' +
'or a class/function (for composite components) but got: null.',
);

if (__DEV__) {
// One warning for each element creation
expect(console.error.calls.count()).toBe(2);
}
});

it('includes owner name in the error about badly-typed elements', () => {
spyOnDev(console, 'error');

const X = undefined;

function Indirection(props) {
Expand All @@ -400,7 +398,12 @@ describe('ReactComponent', () => {
return <Bar />;
}

expect(() => ReactTestUtils.renderIntoDocument(<Foo />)).toThrowError(
expect(() => {
expect(() => ReactTestUtils.renderIntoDocument(<Foo />)).toWarnDev(
'React.createElement: type is invalid -- expected a string (for built-in components) ' +
'or a class/function (for composite components) but got: undefined.',
);
}).toThrowError(
'Element type is invalid: expected a string (for built-in components) ' +
'or a class/function (for composite components) but got: undefined.' +
(__DEV__
Expand All @@ -409,11 +412,6 @@ describe('ReactComponent', () => {
'\n\nCheck the render method of `Bar`.'
: ''),
);

if (__DEV__) {
// One warning for each element creation
expect(console.error.calls.count()).toBe(1);
}
});

it('throws if a plain object is used as a child', () => {
Expand Down Expand Up @@ -530,18 +528,13 @@ describe('ReactComponent', () => {
function Foo() {
return Foo;
}
spyOnDev(console, 'error');
const container = document.createElement('div');
ReactDOM.render(<Foo />, container);
if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in Foo (at **)',
);
}
expect(() => ReactDOM.render(<Foo />, container)).toWarnDev(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in Foo (at **)',
);
});

it('warns on function as a return value from a class', () => {
Expand All @@ -550,18 +543,13 @@ describe('ReactComponent', () => {
return Foo;
}
}
spyOnDev(console, 'error');
const container = document.createElement('div');
ReactDOM.render(<Foo />, container);
if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in Foo (at **)',
);
}
expect(() => ReactDOM.render(<Foo />, container)).toWarnDev(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in Foo (at **)',
);
});

it('warns on function as a child to host component', () => {
Expand All @@ -572,20 +560,15 @@ describe('ReactComponent', () => {
</div>
);
}
spyOnDev(console, 'error');
const container = document.createElement('div');
ReactDOM.render(<Foo />, container);
if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in span (at **)\n' +
' in div (at **)\n' +
' in Foo (at **)',
);
}
expect(() => ReactDOM.render(<Foo />, container)).toWarnDev(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in span (at **)\n' +
' in div (at **)\n' +
' in Foo (at **)',
);
});

it('does not warn for function-as-a-child that gets resolved', () => {
Expand All @@ -601,7 +584,6 @@ describe('ReactComponent', () => {
});

it('deduplicates function type warnings based on component type', () => {
spyOnDev(console, 'error');
class Foo extends React.PureComponent {
constructor() {
super();
Expand All @@ -621,26 +603,23 @@ describe('ReactComponent', () => {
}
}
const container = document.createElement('div');
const component = ReactDOM.render(<Foo />, container);
let component;
expect(() => {
component = ReactDOM.render(<Foo />, container);
}).toWarnDev([
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in div (at **)\n' +
' in Foo (at **)',
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in span (at **)\n' +
' in div (at **)\n' +
' in Foo (at **)',
]);
component.setState({type: 'portobello mushrooms'});
if (__DEV__) {
expect(console.error.calls.count()).toBe(2);
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in div (at **)\n' +
' in Foo (at **)',
);
expect(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' in span (at **)\n' +
' in div (at **)\n' +
' in Foo (at **)',
);
}
});
});
});
81 changes: 27 additions & 54 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,6 @@ describe('ReactComponentLifeCycle', () => {
});

it('should not allow update state inside of getInitialState', () => {
spyOnDev(console, 'error');

class StatefulComponent extends React.Component {
constructor(props, context) {
super(props, context);
Expand All @@ -213,26 +211,20 @@ describe('ReactComponentLifeCycle', () => {
}
}

ReactTestUtils.renderIntoDocument(<StatefulComponent />);
if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toBe(
'Warning: setState(...): Can only update a mounted or ' +
'mounting component. This usually means you called setState() on an ' +
'unmounted component. This is a no-op.\n\nPlease check the code for the ' +
'StatefulComponent component.',
);
}
expect(() => {
ReactTestUtils.renderIntoDocument(<StatefulComponent />);
}).toWarnDev(
'Warning: setState(...): Can only update a mounted or ' +
'mounting component. This usually means you called setState() on an ' +
'unmounted component. This is a no-op.\n\nPlease check the code for the ' +
'StatefulComponent component.',
);

// Check deduplication
// Check deduplication; (no extra warnings should be logged).
ReactTestUtils.renderIntoDocument(<StatefulComponent />);
if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
}
});

it('should correctly determine if a component is mounted', () => {
spyOnDev(console, 'error');
class Component extends React.Component {
_isMounted() {
// No longer a public API, but we can test that it works internally by
Expand All @@ -253,19 +245,13 @@ describe('ReactComponentLifeCycle', () => {

const element = <Component />;

const instance = ReactTestUtils.renderIntoDocument(element);
expect(instance._isMounted()).toBeTruthy();

if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Component is accessing isMounted inside its render()',
);
}
expect(() => {
const instance = ReactTestUtils.renderIntoDocument(element);
expect(instance._isMounted()).toBeTruthy();
}).toWarnDev('Component is accessing isMounted inside its render()');
});

it('should correctly determine if a null component is mounted', () => {
spyOnDev(console, 'error');
class Component extends React.Component {
_isMounted() {
// No longer a public API, but we can test that it works internally by
Expand All @@ -286,15 +272,10 @@ describe('ReactComponentLifeCycle', () => {

const element = <Component />;

const instance = ReactTestUtils.renderIntoDocument(element);
expect(instance._isMounted()).toBeTruthy();

if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Component is accessing isMounted inside its render()',
);
}
expect(() => {
const instance = ReactTestUtils.renderIntoDocument(element);
expect(instance._isMounted()).toBeTruthy();
}).toWarnDev('Component is accessing isMounted inside its render()');
});

it('isMounted should return false when unmounted', () => {
Expand All @@ -317,7 +298,6 @@ describe('ReactComponentLifeCycle', () => {
});

it('warns if findDOMNode is used inside render', () => {
spyOnDev(console, 'error');
class Component extends React.Component {
state = {isMounted: false};
componentDidMount() {
Expand All @@ -331,18 +311,12 @@ describe('ReactComponentLifeCycle', () => {
}
}

ReactTestUtils.renderIntoDocument(<Component />);
if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Component is accessing findDOMNode inside its render()',
);
}
expect(() => {
ReactTestUtils.renderIntoDocument(<Component />);
}).toWarnDev('Component is accessing findDOMNode inside its render()');
});

it('should carry through each of the phases of setup', () => {
spyOnDev(console, 'error');

class LifeCycleComponent extends React.Component {
constructor(props, context) {
super(props, context);
Expand Down Expand Up @@ -399,7 +373,13 @@ describe('ReactComponentLifeCycle', () => {
// yet initialized, or rendered.
//
const container = document.createElement('div');
const instance = ReactDOM.render(<LifeCycleComponent />, container);

let instance;
expect(() => {
instance = ReactDOM.render(<LifeCycleComponent />, container);
}).toWarnDev(
'LifeCycleComponent is accessing isMounted inside its render() function',
);

// getInitialState
expect(instance._testJournal.returnedFromGetInitialState).toEqual(
Expand Down Expand Up @@ -449,13 +429,6 @@ describe('ReactComponentLifeCycle', () => {
// But the current lifecycle of the component is unmounted.
expect(getLifeCycleState(instance)).toBe('UNMOUNTED');
expect(instance.state).toEqual(POST_WILL_UNMOUNT_STATE);

if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'LifeCycleComponent is accessing isMounted inside its render() function',
);
}
});

it('should not throw when updating an auxiliary component', () => {
Expand Down
Loading

0 comments on commit 0fbd864

Please sign in to comment.