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] Fix broken tests in react@next #20472

Merged
merged 12 commits into from Apr 8, 2020
6 changes: 6 additions & 0 deletions .circleci/config.yml
Expand Up @@ -35,6 +35,7 @@ commands:
node scripts/use-react-dist-tag
# log a patch for maintainers who want to check out this change
git diff HEAD
exit 0
- restore_cache:
keys:
- v2-yarn-sha-{{ checksum "yarn.lock" }}
Expand Down Expand Up @@ -153,6 +154,7 @@ jobs:
node scripts/use-typescript-dist-tag
# log a patch for maintainers who want to check out this change
git diff HEAD
exit 0
- install_js
- run:
name: Tests TypeScript definitions
Expand Down Expand Up @@ -210,6 +212,8 @@ workflows:
- test_unit:
requires:
- checkout
# REVERT_ME_OR_JUST_KIDDING_IT_WILL_BE_FINE
react-dist-tag: next
- test_static:
requires:
- checkout
Expand All @@ -219,6 +223,8 @@ workflows:
- test_browser:
requires:
- checkout
# REVERT_ME_OR_JUST_KIDDING_IT_WILL_BE_FINE
react-dist-tag: next
- test_regressions:
requires:
- test_unit
Expand Down
Expand Up @@ -159,11 +159,13 @@ describe('StylesProvider', () => {
<Test />
</StylesProvider>,
);
assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.strictEqual(consoleErrorMock.callCount(), 2);
assert.include(
consoleErrorMock.messages()[0],
'you cannot use the jss and injectFirst props at the same time',
);
// one error per render (twice rendered in StrictMode)
assert.strictEqual(consoleErrorMock.messages()[0], consoleErrorMock.messages()[1]);
});
});
});
Expand Up @@ -67,7 +67,9 @@ describe('ThemeProvider', () => {
const text = () => ref.current.textContent;
function Test() {
const theme = useTheme();
themes.push(theme);
React.useEffect(() => {
themes.push(theme);
});

return (
<span ref={ref}>
Expand Down
10 changes: 7 additions & 3 deletions packages/material-ui/src/FormControl/FormControl.test.js
Expand Up @@ -3,7 +3,7 @@ import { expect } from 'chai';
import { spy } from 'sinon';
import { createMount, getClasses } from '@material-ui/core/test-utils';
import describeConformance from '../test-utils/describeConformance';
import { createClientRender } from 'test/utils/createClientRender';
import { act, createClientRender } from 'test/utils/createClientRender';
import Input from '../Input';
import Select from '../Select';
import FormControl from './FormControl';
Expand All @@ -16,7 +16,9 @@ describe('<FormControl />', () => {

function TestComponent(props) {
const context = useFormControl();
props.contextCallback(context);
React.useEffect(() => {
props.contextCallback(context);
});
return null;
}

Expand Down Expand Up @@ -101,7 +103,9 @@ describe('<FormControl />', () => {
);
expect(readContext.args[0][0]).to.have.property('focused', false);

container.querySelector('input').focus();
act(() => {
container.querySelector('input').focus();
});
expect(readContext.args[1][0]).to.have.property('focused', true);

setProps({ disabled: true });
Expand Down
4 changes: 3 additions & 1 deletion packages/material-ui/src/GridList/GridList.test.js
Expand Up @@ -189,11 +189,13 @@ describe('<GridList />', () => {
</GridList>,
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.strictEqual(consoleErrorMock.callCount(), 2);
assert.include(
consoleErrorMock.messages()[0],
"Material-UI: the GridList component doesn't accept a Fragment as a child.",
);
// one error per render (twice rendered in StrictMode)
assert.strictEqual(consoleErrorMock.messages()[0], consoleErrorMock.messages()[1]);
});
});
});
4 changes: 3 additions & 1 deletion packages/material-ui/src/Hidden/HiddenCss.test.js
Expand Up @@ -160,11 +160,13 @@ describe('<HiddenCss />', () => {
</HiddenCss>,
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.strictEqual(consoleErrorMock.callCount(), 2);
assert.include(
consoleErrorMock.messages()[0],
'Material-UI: unsupported props received by `<Hidden implementation="css" />`: xxlUp.',
);
// one error per render (twice rendered in StrictMode)
assert.strictEqual(consoleErrorMock.messages()[0], consoleErrorMock.messages()[1]);
});
});
});
Expand Up @@ -150,11 +150,13 @@ describe('<InputAdornment />', () => {
/>
</FormControl>,
);
expect(consoleErrorMock.callCount()).to.equal(1);
expect(consoleErrorMock.callCount()).to.equal(2);
expect(consoleErrorMock.messages()[0]).to.equal(
'Material-UI: The `InputAdornment` variant infers the variant ' +
'prop you do not have to provide one.',
);
// one error per render (twice rendered in StrictMode)
expect(consoleErrorMock.messages()[0]).to.equal(consoleErrorMock.messages()[1]);
});
});
});
Expand Down
19 changes: 12 additions & 7 deletions packages/material-ui/src/Paper/Paper.js
@@ -1,6 +1,7 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { chainPropTypes } from '@material-ui/utils';
import withStyles from '../styles/withStyles';

export const styles = (theme) => {
Expand Down Expand Up @@ -41,12 +42,6 @@ const Paper = React.forwardRef(function Paper(props, ref) {
...other
} = props;

if (process.env.NODE_ENV !== 'production') {
if (classes[`elevation${elevation}`] === undefined) {
console.error(`Material-UI: this elevation \`${elevation}\` is not implemented.`);
}
}

return (
<Component
className={clsx(
Expand Down Expand Up @@ -91,7 +86,17 @@ Paper.propTypes = {
* Shadow depth, corresponds to `dp` in the spec.
* It accepts values between 0 and 24 inclusive.
*/
elevation: PropTypes.number,
elevation: chainPropTypes(PropTypes.number, (props) => {
const { classes, elevation } = props;
// in case `withStyles` fails to inject we don't need this warning
if (classes === undefined) {
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
return null;
}
if (elevation != null && classes[`elevation${elevation}`] === undefined) {
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
return new Error(`Material-UI: this elevation \`${elevation}\` is not implemented.`);
}
return null;
}),
/**
* If `true`, rounded corners are disabled.
*/
Expand Down
45 changes: 27 additions & 18 deletions packages/material-ui/src/Paper/Paper.test.js
@@ -1,6 +1,7 @@
import * as React from 'react';
import { assert } from 'chai';
import { createMount, createShallow, getClasses } from '@material-ui/core/test-utils';
import * as PropTypes from 'prop-types';
import describeConformance from '../test-utils/describeConformance';
import Paper from './Paper';
import { createMuiTheme, ThemeProvider } from '../styles';
Expand All @@ -11,14 +12,6 @@ describe('<Paper />', () => {
let shallow;
let classes;

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

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

before(() => {
mount = createMount({ strict: true });
shallow = createShallow({ dive: true });
Expand Down Expand Up @@ -77,16 +70,6 @@ describe('<Paper />', () => {
);
});

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

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

it('allows custom elevations via theme.shadows', () => {
const theme = createMuiTheme();
theme.shadows.push('20px 20px');
Expand All @@ -98,4 +81,30 @@ describe('<Paper />', () => {

assert.strictEqual(wrapper.find('div[data-testid="paper"]').hasClass('custom-elevation'), true);
});

describe('warnings', () => {
beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

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

it('warns if the given `elevation` is not implemented in the theme', () => {
PropTypes.checkPropTypes(
Paper.Naked.propTypes,
{ classes: { elevation24: 'elevation-24', elevation26: 'elevation-26' }, elevation: 25 },
'prop',
'MockedPaper',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
consoleErrorMock.messages()[0],
'Material-UI: this elevation `25` is not implemented.',
);
});
});
});
21 changes: 17 additions & 4 deletions packages/material-ui/src/Popover/Popover.test.js
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import { assert, expect } from 'chai';
import { spy, stub, useFakeTimers } from 'sinon';
import { createMount, findOutermostIntrinsic, getClasses } from '@material-ui/core/test-utils';
import * as PropTypes from 'prop-types';
import describeConformance from '../test-utils/describeConformance';
import consoleErrorMock from 'test/utils/consoleErrorMock';
import Grow from '../Grow';
Expand Down Expand Up @@ -460,25 +461,37 @@ describe('<Popover />', () => {
describe('warnings', () => {
beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

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

it('should warn if anchorEl is not valid', () => {
const otherWrapper = mount(<Popover open />);
assert.strictEqual(otherWrapper.find(Modal).props().container, undefined);
PropTypes.checkPropTypes(
Popover.Naked.propTypes,
{ classes: {}, open: true },
'prop',
'MockedPopover',
);

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

it('warns if a component for the Paper is used that cant hold a ref', () => {
mount(<Popover {...defaultProps} PaperProps={{ component: () => <div />, elevation: 4 }} />);
PropTypes.checkPropTypes(
Popover.Naked.propTypes,
{ ...defaultProps, classes: {}, PaperProps: { component: () => <div />, elevation: 4 } },
'prop',
'MockedPopover',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
consoleErrorMock.messages()[0],
'Warning: Failed prop type: Invalid prop `PaperProps.component` supplied to `ForwardRef(Popover)`. Expected an element type that can hold a ref.',
'Warning: Failed prop type: Invalid prop `PaperProps.component` supplied to `MockedPopover`. Expected an element type that can hold a ref.',
);
});

Expand Down
6 changes: 4 additions & 2 deletions scripts/use-react-dist-tag.js
Expand Up @@ -9,7 +9,6 @@
*/
const childProcess = require('child_process');
const fs = require('fs');
const os = require('os');
const path = require('path');
const { promisify } = require('util');

Expand Down Expand Up @@ -45,8 +44,11 @@ async function main(distTag) {
packageJson.resolutions[reactPackageName] = version;
});

// https://github.com/enzymejs/enzyme/issues/2358
packageJson.devDependencies['enzyme-adapter-react-16'] = 'npm:@eps1lon/enzyme-adapter-react-next';

// CircleCI seemingly times out if it has a newline diff at the end
fs.writeFileSync(packageJsonPath, `${JSON.stringify(packageJson, null, 2)}${os.EOL}`);
fs.writeFileSync(packageJsonPath, JSON.stringify(packageJson, null, 2));
}

main(process.env.REACT_DIST_TAG).catch((error) => {
Expand Down
4 changes: 1 addition & 3 deletions scripts/use-typescript-dist-tag.js
Expand Up @@ -11,7 +11,6 @@
*/
const childProcess = require('child_process');
const fs = require('fs');
const os = require('os');
const path = require('path');
const { promisify } = require('util');

Expand Down Expand Up @@ -44,8 +43,7 @@ async function main(distTag) {
packageJson.devDependencies.typescript = version;
packageJson.resolutions['**/dtslint/typescript'] = version;

// CircleCI seemingly times out if it has a newline diff at the end
fs.writeFileSync(packageJsonPath, `${JSON.stringify(packageJson, null, 2)}${os.EOL}`);
fs.writeFileSync(packageJsonPath, JSON.stringify(packageJson, null, 2));
}

main(process.env.TYPESCRIPT_DIST_TAG).catch((error) => {
Expand Down