Skip to content

Commit

Permalink
[test] Use .checkPropTypes instead of render + propTypes (#20451)
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon committed Apr 8, 2020
1 parent c776d34 commit 69f7046
Show file tree
Hide file tree
Showing 21 changed files with 137 additions and 93 deletions.
5 changes: 4 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ module.exports = {
// test to `beforeEach`.
// `beforeEach`+`afterEach` also means that the `beforeEach`
// is cleaned up in `afterEach` if the test causes a crash
'mocha/no-hooks-for-single-case': 'off'
'mocha/no-hooks-for-single-case': 'off',

// They are accessed to test custom validator implementation with PropTypes.checkPropTypes
'react/forbid-foreign-prop-types': 'off',
},
},
{
Expand Down
12 changes: 7 additions & 5 deletions packages/material-ui-styles/src/styled/styled.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,19 +93,21 @@ describe('styled', () => {
describe('warnings', () => {
beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

it('warns if it cant detect the secondary action properly', () => {
mount(
<StyledButton clone component="div">
<div>Styled Components</div>
</StyledButton>,
PropTypes.checkPropTypes(
StyledButton.propTypes,
{ clone: true, component: 'div' },
'prop',
'StyledButton',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
consoleErrorMock.messages()[0],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,11 @@ describe('withStyles', () => {
// describe('innerRef', () => {
// beforeEach(() => {
// consoleErrorMock.spy();
// PropTypes.resetWarningCache();
// });

// afterEach(() => {
// consoleErrorMock.reset();
// PropTypes.resetWarningCache();
// });

// it('is deprecated', () => {
Expand Down
10 changes: 7 additions & 3 deletions packages/material-ui-styles/src/withTheme/withTheme.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,21 @@ describe('withTheme', () => {
describe('innerRef', () => {
beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

it('is deprecated', () => {
const ThemedDiv = withTheme('div');

mount(<ThemedDiv innerRef={React.createRef()} />);
PropTypes.checkPropTypes(
ThemedDiv.propTypes,
{ innerRef: React.createRef() },
'prop',
'ThemedDiv',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
Expand Down
1 change: 1 addition & 0 deletions packages/material-ui-utils/src/chainPropTypes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ describe('chainPropTypes', () => {

beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui-utils/src/elementAcceptingRef.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ describe('elementAcceptingRef', () => {

beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

describe('acceptance when not required', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ describe('elementTypeAcceptingRef', () => {

beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

describe('acceptance', () => {
Expand Down
13 changes: 9 additions & 4 deletions packages/material-ui/src/ButtonBase/ButtonBase.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -908,11 +908,11 @@ describe('<ButtonBase />', () => {
describe('warnings', () => {
beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

it('warns on invalid `component` prop: ref forward', () => {
Expand All @@ -929,11 +929,16 @@ describe('<ButtonBase />', () => {
return <button type="button" {...props} />;
}

// cant match the error message here because flakiness with mocha watchmode
render(<ButtonBase component={Component} />);
PropTypes.checkPropTypes(
// @ts-ignore `Naked` is internal
ButtonBase.Naked.propTypes,
{ classes: {}, component: Component },
'prop',
'MockedName',
);

expect(consoleErrorMock.messages()[0]).to.include(
'Invalid prop `component` supplied to `ForwardRef(ButtonBase)`. Expected an element type that can hold a ref',
'Invalid prop `component` supplied to `MockedName`. Expected an element type that can hold a ref',
);
});

Expand Down
4 changes: 2 additions & 2 deletions packages/material-ui/src/CardMedia/CardMedia.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@ describe('<CardMedia />', () => {
describe('warnings', () => {
before(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

after(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

it('warns when neither `children`, nor `image`, nor `src`, nor `component` are provided', () => {
mount(<CardMedia />);
PropTypes.checkPropTypes(CardMedia.Naked.propTypes, { classes: {} }, 'prop', 'MockedName');

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
Expand Down
28 changes: 15 additions & 13 deletions packages/material-ui/src/ExpansionPanel/ExpansionPanel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,31 +137,33 @@ describe('<ExpansionPanel />', () => {
describe('first child', () => {
beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

it('requires at least one child', () => {
if (!/jsdom/.test(window.navigator.userAgent)) {
// errors during mount are not caught by try-catch in the browser
// can't use skip since this causes multiple errors related to cleanup of the console mock
return;
}

assert.throws(() => mount(<ExpansionPanel>{[]}</ExpansionPanel>));
assert.strictEqual(consoleErrorMock.callCount(), 3);
PropTypes.checkPropTypes(
ExpansionPanel.Naked.propTypes,
{ classes: {}, children: [] },
'prop',
'MockedName',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(consoleErrorMock.messages()[0], 'Material-UI: expected the first child');
});

it('needs a valid element as the first child', () => {
mount(
<ExpansionPanel>
<React.Fragment />
</ExpansionPanel>,
PropTypes.checkPropTypes(
ExpansionPanel.Naked.propTypes,
{ classes: {}, children: <React.Fragment /> },
'prop',
'MockedName',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
consoleErrorMock.messages()[0],
Expand Down
2 changes: 0 additions & 2 deletions packages/material-ui/src/GridList/GridList.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { createMount, createShallow, getClasses } from '@material-ui/core/test-u
import describeConformance from '../test-utils/describeConformance';
import GridList from './GridList';
import consoleErrorMock from 'test/utils/consoleErrorMock';
import PropTypes from 'prop-types';

const tilesData = [
{
Expand Down Expand Up @@ -181,7 +180,6 @@ describe('<GridList />', () => {

after(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

it('warns a Fragment is passed as a child', () => {
Expand Down
12 changes: 7 additions & 5 deletions packages/material-ui/src/IconButton/IconButton.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,21 @@ describe('<IconButton />', () => {
describe('Firefox onClick', () => {
beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

it('should raise a warning', () => {
render(
<IconButton>
<svg onClick={() => {}} />
</IconButton>,
PropTypes.checkPropTypes(
IconButton.Naked.propTypes,
{ classes: {}, children: <svg onClick={() => {}} /> },
'prop',
'MockedName',
);

expect(consoleErrorMock.callCount()).to.equal(1);
expect(consoleErrorMock.messages()[0]).to.include(
'you are providing an onClick event listener',
Expand Down
18 changes: 12 additions & 6 deletions packages/material-ui/src/ListItem/ListItem.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,25 @@ describe('<ListItem />', () => {
describe('warnings', () => {
beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

it('warns if it cant detect the secondary action properly', () => {
render(
<ListItem>
<ListItemSecondaryAction>I should have come last :(</ListItemSecondaryAction>
<ListItemText>My position doesn not matter.</ListItemText>
</ListItem>,
PropTypes.checkPropTypes(
ListItem.Naked.propTypes,
{
classes: {},
children: [
<ListItemSecondaryAction>I should have come last :(</ListItemSecondaryAction>,
<ListItemText>My position doesn not matter.</ListItemText>,
],
},
'prop',
'MockedName',
);

expect(consoleErrorMock.callCount()).to.equal(1);
Expand Down
2 changes: 0 additions & 2 deletions packages/material-ui/src/Menu/Menu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import Popover from '../Popover';
import Menu from './Menu';
import MenuList from '../MenuList';
import consoleErrorMock from 'test/utils/consoleErrorMock';
import PropTypes from 'prop-types';

const MENU_LIST_HEIGHT = 100;

Expand Down Expand Up @@ -233,7 +232,6 @@ describe('<Menu />', () => {

after(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

it('warns a Fragment is passed as a child', () => {
Expand Down
2 changes: 0 additions & 2 deletions packages/material-ui/src/Popover/Popover.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as React from 'react';
import { assert, expect } from 'chai';
import { spy, stub, useFakeTimers } from 'sinon';
import PropTypes from 'prop-types';
import { createMount, findOutermostIntrinsic, getClasses } from '@material-ui/core/test-utils';
import describeConformance from '../test-utils/describeConformance';
import consoleErrorMock from 'test/utils/consoleErrorMock';
Expand Down Expand Up @@ -461,7 +460,6 @@ describe('<Popover />', () => {
describe('warnings', () => {
beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
Expand Down
18 changes: 11 additions & 7 deletions packages/material-ui/src/Popper/Popper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,19 @@ describe('<Popper />', () => {
});

it('should warn if anchorEl is not valid', () => {
mount(<Popper {...defaultProps} open anchorEl={null} />);
PropTypes.checkPropTypes(
Popper.propTypes,
{
...defaultProps,
open: true,
anchorEl: null,
},
'prop',
'MockedPopper',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(consoleErrorMock.messages()[0], 'It should be an HTML Element instance');
});

// it('should warn if anchorEl is not visible', () => {
// mount(<Popper {...defaultProps} open anchorEl={document.createElement('div')} />);
// assert.strictEqual(consoleErrorMock.callCount(), 1);
// assert.include(consoleErrorMock.messages()[0], 'The node element should be visible');
// });
});
});
20 changes: 17 additions & 3 deletions packages/material-ui/src/Slider/Slider.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -489,22 +489,36 @@ describe('<Slider />', () => {
describe('warnings', () => {
beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
consoleErrorMock.reset();
PropTypes.resetWarningCache();
});

it('should warn if aria-valuetext is provided', () => {
render(<Slider value={[20, 50]} aria-valuetext="hot" />);
PropTypes.checkPropTypes(
Slider.Naked.propTypes,
{ classes: {}, value: [20, 50], 'aria-valuetext': 'hot' },
'prop',
'MockedSlider',
);

expect(consoleErrorMock.callCount()).to.equal(1);
expect(consoleErrorMock.messages()[0]).to.include(
'you need to use the `getAriaValueText` prop instead of',
);
});

it('should warn if aria-label is provided', () => {
render(<Slider value={[20, 50]} aria-label="hot" />);
PropTypes.checkPropTypes(
Slider.Naked.propTypes,
{ classes: {}, value: [20, 50], 'aria-label': 'hot' },
'prop',
'MockedSlider',
);

expect(consoleErrorMock.callCount()).to.equal(1);
expect(consoleErrorMock.messages()[0]).to.include(
'you need to use the `getAriaLabel` prop instead of',
);
Expand Down
Loading

0 comments on commit 69f7046

Please sign in to comment.