Skip to content

Commit

Permalink
[test] Improve coverage (#18385)
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon authored and oliviertassinari committed Nov 16, 2019
1 parent b7cb1c5 commit 9922713
Show file tree
Hide file tree
Showing 20 changed files with 168 additions and 25 deletions.
3 changes: 2 additions & 1 deletion packages/material-ui-lab/src/SpeedDial/SpeedDial.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { duration, withStyles } from '@material-ui/core/styles';
Expand Down Expand Up @@ -280,7 +281,7 @@ const SpeedDial = React.forwardRef(function SpeedDial(props, ref) {

const allItems = React.Children.toArray(childrenProp).filter(child => {
if (process.env.NODE_ENV !== 'production') {
if (child.type === React.Fragment) {
if (isFragment(child)) {
console.error(
[
"Material-UI: the SpeedDial component doesn't accept a Fragment as a child.",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import isValueSelected from './isValueSelected';
Expand Down Expand Up @@ -82,7 +83,7 @@ const ToggleButtonGroup = React.forwardRef(function ToggleButton(props, ref) {
}

if (process.env.NODE_ENV !== 'production') {
if (child.type === React.Fragment) {
if (isFragment(child)) {
console.error(
[
"Material-UI: the ToggleButtonGroup component doesn't accept a Fragment as a child.",
Expand Down
1 change: 1 addition & 0 deletions packages/material-ui/scripts/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const commonjsOptions = {
],
'../../node_modules/react-is/index.js': [
'ForwardRef',
'isFragment',
'isLazy',
'isMemo',
'isValidElementType',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import withStyles from '../styles/withStyles';
Expand Down Expand Up @@ -33,7 +34,7 @@ const BottomNavigation = React.forwardRef(function BottomNavigation(props, ref)
}

if (process.env.NODE_ENV !== 'production') {
if (child.type === React.Fragment) {
if (isFragment(child)) {
console.error(
[
"Material-UI: the BottomNavigation component doesn't accept a Fragment as a child.",
Expand Down
3 changes: 2 additions & 1 deletion packages/material-ui/src/Breadcrumbs/Breadcrumbs.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import withStyles from '../styles/withStyles';
Expand Down Expand Up @@ -86,7 +87,7 @@ const Breadcrumbs = React.forwardRef(function Breadcrumbs(props, ref) {
const allItems = React.Children.toArray(children)
.filter(child => {
if (process.env.NODE_ENV !== 'production') {
if (child.type === React.Fragment) {
if (isFragment(child)) {
console.error(
[
"Material-UI: the Breadcrumbs component doesn't accept a Fragment as a child.",
Expand Down
3 changes: 2 additions & 1 deletion packages/material-ui/src/ButtonGroup/ButtonGroup.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import capitalize from '../utils/capitalize';
Expand Down Expand Up @@ -148,7 +149,7 @@ const ButtonGroup = React.forwardRef(function ButtonGroup(props, ref) {
}

if (process.env.NODE_ENV !== 'production') {
if (child.type === React.Fragment) {
if (isFragment(child)) {
console.error(
[
"Material-UI: the ButtonGroup component doesn't accept a Fragment as a child.",
Expand Down
3 changes: 2 additions & 1 deletion packages/material-ui/src/ExpansionPanel/ExpansionPanel.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { chainPropTypes } from '@material-ui/utils';
Expand Down Expand Up @@ -169,7 +170,7 @@ ExpansionPanel.propTypes = {
*/
children: chainPropTypes(PropTypes.node.isRequired, props => {
const summary = React.Children.toArray(props.children)[0];
if (summary.type === React.Fragment) {
if (isFragment(summary)) {
return new Error(
"Material-UI: the ExpansionPanel doesn't accept a Fragment as a child. " +
'Consider providing an array instead.',
Expand Down
12 changes: 8 additions & 4 deletions packages/material-ui/src/ExpansionPanel/ExpansionPanel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,17 @@ describe('<ExpansionPanel />', () => {
PropTypes.resetWarningCache();
});

/* works locally but doesn't catch the errors in test:karma
it('requires at least one child', () => {
assert.throws(() => mount(<ExpansionPanel>[]</ExpansionPanel>));
// 2 other errors are from accesing prop of undefined and react component stack
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);
assert.include(consoleErrorMock.args()[0][0], 'Material-UI: expected the first child');
}); */
});

it('needs a valid element as the first child', () => {
mount(
Expand Down
3 changes: 2 additions & 1 deletion packages/material-ui/src/GridList/GridList.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import withStyles from '../styles/withStyles';
Expand Down Expand Up @@ -41,7 +42,7 @@ const GridList = React.forwardRef(function GridList(props, ref) {
}

if (process.env.NODE_ENV !== 'production') {
if (child.type === React.Fragment) {
if (isFragment(child)) {
console.error(
[
"Material-UI: the GridList component doesn't accept a Fragment as a child.",
Expand Down
27 changes: 27 additions & 0 deletions packages/material-ui/src/GridList/GridList.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { assert } from 'chai';
import { createMount, createShallow, getClasses } from '@material-ui/core/test-utils';
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 @@ -186,4 +188,29 @@ describe('<GridList />', () => {
);
});
});

describe('warnings', () => {
before(() => {
consoleErrorMock.spy();
});

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

it('warns a Fragment is passed as a child', () => {
mount(
<GridList>
<React.Fragment />
</GridList>,
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
consoleErrorMock.args()[0][0],
"Material-UI: the GridList component doesn't accept a Fragment as a child.",
);
});
});
});
3 changes: 2 additions & 1 deletion packages/material-ui/src/Menu/Menu.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import withStyles from '../styles/withStyles';
Expand Down Expand Up @@ -95,7 +96,7 @@ const Menu = React.forwardRef(function Menu(props, ref) {
}

if (process.env.NODE_ENV !== 'production') {
if (child.type === React.Fragment) {
if (isFragment(child)) {
console.error(
[
"Material-UI: the Menu component doesn't accept a Fragment as a child.",
Expand Down
42 changes: 42 additions & 0 deletions packages/material-ui/src/Menu/Menu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import describeConformance from '../test-utils/describeConformance';
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 @@ -212,4 +214,44 @@ describe('<Menu />', () => {
assert.strictEqual(onCloseSpy.callCount, 1);
assert.strictEqual(onCloseSpy.args[0][1], 'tabKeyDown');
});

it('ignores invalid children', () => {
const wrapper = mount(
<Menu {...defaultProps} open>
{null}
<span role="menuitem">hello</span>
{/* testing conditional rendering */}
{false && <span role="menuitem">hello</span>}
{undefined}
foo
</Menu>,
);

assert.lengthOf(wrapper.find('span[role="menuitem"]'), 1);
});

describe('warnings', () => {
before(() => {
consoleErrorMock.spy();
});

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

it('warns a Fragment is passed as a child', () => {
mount(
<Menu anchorEl={document.createElement('div')} open>
<React.Fragment />
</Menu>,
);

assert.strictEqual(consoleErrorMock.callCount(), 2);
assert.include(
consoleErrorMock.args()[0][0],
"Material-UI: the Menu component doesn't accept a Fragment as a child.",
);
});
});
});
6 changes: 2 additions & 4 deletions packages/material-ui/src/MenuList/MenuList.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import ReactDOM from 'react-dom';
import ownerDocument from '../utils/ownerDocument';
Expand Down Expand Up @@ -35,9 +36,6 @@ function textCriteriaMatches(nextFocus, textCriteria) {
// jsdom doesn't support innerText
text = nextFocus.textContent;
}
if (text === undefined) {
return false;
}
text = text.trim().toLowerCase();
if (text.length === 0) {
return false;
Expand Down Expand Up @@ -209,7 +207,7 @@ const MenuList = React.forwardRef(function MenuList(props, ref) {
}

if (process.env.NODE_ENV !== 'production') {
if (child.type === React.Fragment) {
if (isFragment(child)) {
console.error(
[
"Material-UI: the Menu component doesn't accept a Fragment as a child.",
Expand Down
5 changes: 1 addition & 4 deletions packages/material-ui/src/Paper/Paper.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import withStyles from '../styles/withStyles';
import useTheme from '../styles/useTheme';

export const styles = theme => {
const elevations = {};
Expand Down Expand Up @@ -38,9 +37,7 @@ const Paper = React.forwardRef(function Paper(props, ref) {
} = props;

if (process.env.NODE_ENV !== 'production') {
// eslint-disable-next-line react-hooks/rules-of-hooks
const theme = useTheme();
if (!theme.shadows[elevation]) {
if (classes[`elevation${elevation}`] === undefined) {
console.error(`Material-UI: this elevation \`${elevation}\` is not implemented.`);
}
}
Expand Down
32 changes: 32 additions & 0 deletions packages/material-ui/src/Paper/Paper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,22 @@ import { assert } from 'chai';
import { createMount, createShallow, getClasses } from '@material-ui/core/test-utils';
import describeConformance from '../test-utils/describeConformance';
import Paper from './Paper';
import { createMuiTheme, ThemeProvider } from '../styles';
import consoleErrorMock from 'test/utils/consoleErrorMock';

describe('<Paper />', () => {
let mount;
let shallow;
let classes;

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

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

before(() => {
mount = createMount({ strict: true });
shallow = createShallow({ dive: true });
Expand Down Expand Up @@ -59,4 +69,26 @@ describe('<Paper />', () => {
'should have the 2 elevation class',
);
});

it('warns if the given `elevation` is not implemented in the theme', () => {
mount(<Paper elevation={25} />);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
consoleErrorMock.args()[0][0],
'Material-UI: this elevation `25` is not implemented.',
);
});

it('allows custom elevations via theme.shadows', () => {
const theme = createMuiTheme();
theme.shadows.push('20px 20px');
const wrapper = mount(
<ThemeProvider theme={theme}>
<Paper data-testid="paper" classes={{ elevation25: 'custom-elevation' }} elevation={25} />
</ThemeProvider>,
);

assert.strictEqual(wrapper.find('div[data-testid="paper"]').hasClass('custom-elevation'), true);
});
});
3 changes: 2 additions & 1 deletion packages/material-ui/src/Select/SelectInput.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import capitalize from '../utils/capitalize';
Expand Down Expand Up @@ -214,7 +215,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
}

if (process.env.NODE_ENV !== 'production') {
if (child.type === React.Fragment) {
if (isFragment(child)) {
console.error(
[
"Material-UI: the Select component doesn't accept a Fragment as a child.",
Expand Down
3 changes: 2 additions & 1 deletion packages/material-ui/src/Step/Step.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import withStyles from '../styles/withStyles';
Expand Down Expand Up @@ -69,7 +70,7 @@ const Step = React.forwardRef(function Step(props, ref) {
}

if (process.env.NODE_ENV !== 'production') {
if (child.type === React.Fragment) {
if (isFragment(child)) {
console.error(
[
"Material-UI: the Step component doesn't accept a Fragment as a child.",
Expand Down
3 changes: 2 additions & 1 deletion packages/material-ui/src/Tabs/Tabs.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { isFragment } from 'react-is';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { refType } from '@material-ui/utils';
Expand Down Expand Up @@ -383,7 +384,7 @@ const Tabs = React.forwardRef(function Tabs(props, ref) {
}

if (process.env.NODE_ENV !== 'production') {
if (child.type === React.Fragment) {
if (isFragment(child)) {
console.error(
[
"Material-UI: the Tabs component doesn't accept a Fragment as a child.",
Expand Down
4 changes: 2 additions & 2 deletions packages/material-ui/src/styles/createTypography.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ export default function createTypography(palette, typography) {

if (process.env.NODE_ENV !== 'production') {
if (typeof fontSize !== 'number') {
console.error(`Material-UI: 'fontSize' is required to be a number.`);
console.error('Material-UI: `fontSize` is required to be a number.');
}

if (typeof htmlFontSize !== 'number') {
console.error(`Material-UI: 'htmlFontSize' is required to be a number.`);
console.error('Material-UI: `htmlFontSize` is required to be a number.');
}
}

Expand Down
Loading

0 comments on commit 9922713

Please sign in to comment.