Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[test] Use .checkPropTypes instead of render + propTypes #20451

Merged
merged 2 commits into from
Apr 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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