From 7f26d466f0b03bec5f0524b852879486f6ba1806 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 21 Jan 2020 12:01:03 +0100 Subject: [PATCH 1/5] [Step] Use testing-library in test --- packages/material-ui/src/Step/Step.test.js | 146 ++++++++++++++------- 1 file changed, 95 insertions(+), 51 deletions(-) diff --git a/packages/material-ui/src/Step/Step.test.js b/packages/material-ui/src/Step/Step.test.js index a8809af796b630..b1ffcddca3c7e5 100644 --- a/packages/material-ui/src/Step/Step.test.js +++ b/packages/material-ui/src/Step/Step.test.js @@ -1,107 +1,151 @@ import React from 'react'; -import { assert } from 'chai'; -import { createShallow, createMount, getClasses } from '@material-ui/core/test-utils'; +import * as PropTypes from 'prop-types'; +import { expect } from 'chai'; +import { createMount, getClasses } from '@material-ui/core/test-utils'; import describeConformance from '../test-utils/describeConformance'; +import { createClientRender, within } from 'test/utils/createClientRender'; import Step from './Step'; +/** + * Exposes props stringified in the dataset of the `[data-testid="props"]` + */ +function PropsAsDataset(props) { + const elementRef = React.useRef(); + React.useEffect(() => { + const { current: element } = elementRef; + + Object.keys(props).forEach(key => { + // converted to strings internally. writing it out for readability + element.dataset[key] = String(props[key]); + }); + }); + + return ; +} + +/** + * A component that can be used as a child of `Step`. + * It passes through all unrelated props to the underlying `div` + * The props passed from `Step` are intercepted + */ +function StepChildDiv(props) { + const { + active, + alternativeLabel, + completed, + disabled, + expanded, + icon, + last, + orientation, + ...other + } = props; + + return
; +} +StepChildDiv.propTypes = { + active: PropTypes.bool, + alternativeLabel: PropTypes.bool, + completed: PropTypes.bool, + disabled: PropTypes.bool, + expanded: PropTypes.bool, + icon: PropTypes.node, + last: PropTypes.bool, + orientation: PropTypes.oneOf(['horizontal', 'vertical']), +}; + describe('', () => { let classes; - let shallow; let mount; + const render = createClientRender(); + before(() => { classes = getClasses(); - shallow = createShallow({ dive: true }); mount = createMount({ strict: true }); }); - after(() => { - mount.cleanUp(); - }); - describeConformance(, () => ({ classes, inheritComponent: 'div', mount, refInstanceof: window.HTMLDivElement, skip: ['componentProp'], + after: () => mount.cleanUp(), })); it('merges styles and other props into the root node', () => { - const wrapper = shallow( + const { getByTestId } = render( , ); - const props = wrapper.props(); - assert.strictEqual(props.style.paddingRight, 200); - assert.strictEqual(props.style.color, 'purple'); - assert.strictEqual(props.style.border, '1px solid tomato'); - assert.strictEqual(props['data-role'], 'Menuitem'); + + const rootNode = getByTestId('root'); + expect(rootNode.style).to.have.property('paddingRight', '200px'); + expect(rootNode.style).to.have.property('color', 'purple'); + expect(rootNode.style).to.have.property('border', '1px solid tomato'); }); describe('rendering children', () => { it('renders children', () => { - const children =

Hello World

; - const wrapper = shallow( - - {children} + const { getByTestId } = render( + + Hello World , ); - assert.strictEqual(wrapper.find('.hello-world').length, 1); + + expect(within(getByTestId('root')).getByTestId('child')).to.be.ok; }); it('renders children with all props passed through', () => { - const children = [ -

- Hello World -

, -

- How are you? -

, - ]; - const wrapper = shallow( + const { getAllByTestId } = render( - {children} + + , ); - const child1 = wrapper.find('.hello-world'); - const child2 = wrapper.find('.hay'); - [child1, child2].forEach(child => { - assert.strictEqual(child.length, 1); - assert.strictEqual(child.props().active, false); - assert.strictEqual(child.props().completed, true); - assert.strictEqual(child.props().disabled, true); - assert.strictEqual(child.props().icon, 1); + getAllByTestId('props').forEach(child => { + // HTMLElement.dataset is a DOMStringMap which fails deep.equal + const datasetAsObject = { ...child.dataset }; + expect(datasetAsObject).to.deep.equal({ + // props passed from Step + active: 'false', + alternativeLabel: 'undefined', + completed: 'true', + disabled: 'true', + expanded: 'false', + last: 'undefined', + icon: '1', + orientation: 'horizontal', + // test impl details + testid: 'props', + }); }); }); it('honours children overriding props passed through', () => { - const children = ( -

- Hello World -

- ); - const wrapper = shallow( + const { getByTestId } = render( - {children} + , ); - const childWrapper = wrapper.find('.hello-world'); - assert.strictEqual(childWrapper.props().active, false); + + expect(getByTestId('props').dataset).to.have.property('active', 'false'); }); - it('should handle invalid children', () => { - const wrapper = shallow( + it('should handle null children', () => { + const { getByTestId } = render( -

Hello World

+ Hello World {null} , ); - assert.strictEqual(wrapper.find('.hello-world').length, 1); + + expect(getByTestId('child')).to.be.ok; }); }); }); From 5cae3953e05627284d0b41d2a9d1623099dcdba4 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 21 Jan 2020 12:37:18 +0100 Subject: [PATCH 2/5] [StepButton] test using testing-library --- .../src/StepButton/StepButton.test.js | 169 +++++++++--------- 1 file changed, 82 insertions(+), 87 deletions(-) diff --git a/packages/material-ui/src/StepButton/StepButton.test.js b/packages/material-ui/src/StepButton/StepButton.test.js index 254c05aebcf84c..c5d0a2c05f9a77 100644 --- a/packages/material-ui/src/StepButton/StepButton.test.js +++ b/packages/material-ui/src/StepButton/StepButton.test.js @@ -1,122 +1,117 @@ import React from 'react'; -import { assert } from 'chai'; +import { expect } from 'chai'; import { spy } from 'sinon'; -import { createShallow, createMount, getClasses } from '@material-ui/core/test-utils'; +import { createMount, getClasses } from '@material-ui/core/test-utils'; import describeConformance from '../test-utils/describeConformance'; +import { createClientRender } from 'test/utils/createClientRender'; import StepButton from './StepButton'; import StepLabel from '../StepLabel'; import ButtonBase from '../ButtonBase'; +import { fireEvent } from '@testing-library/dom'; describe('', () => { let classes; - let shallow; - let mount; - const defaultProps = { orientation: 'horizontal' }; + const render = createClientRender(); before(() => { classes = getClasses(); - shallow = createShallow({ dive: true }); - mount = createMount({ strict: true }); }); - after(() => { - mount.cleanUp(); - }); + describe('internals', () => { + let mount; - describeConformance(, () => ({ - classes, - inheritComponent: ButtonBase, - mount, - refInstanceof: window.HTMLButtonElement, - skip: ['componentProp'], - })); - - it('passes active, completed, disabled to StepLabel', () => { - const wrapper = shallow( - - Step One - , - ); - const stepLabel = wrapper.find(StepLabel); - assert.strictEqual(stepLabel.props().active, true); - assert.strictEqual(stepLabel.props().completed, true); - assert.strictEqual(stepLabel.props().disabled, true); - assert.strictEqual(stepLabel.props().children, 'Step One'); - }); + before(() => { + mount = createMount({ strict: true }); + }); - it('should pass props to a provided StepLabel', () => { - const wrapper = shallow( - - Step One - , - ); - const stepLabel = wrapper.find(StepLabel); - assert.strictEqual(stepLabel.props().active, true); - assert.strictEqual(stepLabel.props().completed, true); - assert.strictEqual(stepLabel.props().disabled, true); - }); + after(() => { + mount.cleanUp(); + }); - it('should pass disabled prop to the ButtonBase', () => { - const wrapper = shallow( - - Step One - , - ); - const stepLabel = wrapper.find(ButtonBase); - assert.strictEqual(stepLabel.props().disabled, true); - }); + describeConformance(, () => ({ + classes, + inheritComponent: ButtonBase, + mount, + refInstanceof: window.HTMLButtonElement, + skip: ['componentProp'], + })); - describe('event handlers', () => { - describe('handleMouseEnter/Leave', () => { - const handleMouseEnter = spy(); - const handleMouseLeave = spy(); + it('passes active, completed, disabled to StepLabel', () => { + const wrapper = mount( + + Step One + , + ); + + const stepLabel = wrapper.find(StepLabel); + expect(stepLabel.props()).to.have.property('active', true); + expect(stepLabel.props()).to.have.property('completed', true); + expect(stepLabel.props()).to.have.property('disabled', true); + expect(stepLabel.props()).to.have.property('children', 'Step One'); + }); + + it('should pass props to a provided StepLabel', () => { + const wrapper = mount( + + Step One + , + ); - it('should fire event callbacks', () => { - const wrapper = shallow( - - Step One - , - ); - wrapper.simulate('mouseEnter'); - assert.strictEqual(handleMouseEnter.callCount, 1); - wrapper.simulate('mouseLeave'); - assert.strictEqual(handleMouseEnter.callCount, 1); - assert.strictEqual(handleMouseLeave.callCount, 1); - }); + const stepLabel = wrapper.find(StepLabel); + expect(stepLabel.props()).to.have.property('active', true); + expect(stepLabel.props()).to.have.property('completed', true); + expect(stepLabel.props()).to.have.property('disabled', true); + expect(stepLabel.props()).to.have.property('children', 'Step One'); }); + }); + + it('should disable the button', () => { + const { getByRole } = render(Step One); - it('should bubble callbacks used internally', () => { + expect(getByRole('button')).to.have.property('disabled', true); + }); + + describe('event handlers', () => { + it('should forward mouseenter, mouseleave and touchstart', () => { const handleMouseEnter = spy(); const handleMouseLeave = spy(); const handleTouchStart = spy(); - const wrapper = shallow( + const { getByRole } = render( Step One , ); - wrapper.simulate('mouseEnter'); - assert.strictEqual(handleMouseEnter.callCount, 1); - wrapper.simulate('mouseLeave'); - assert.strictEqual(handleMouseEnter.callCount, 1); - assert.strictEqual(handleMouseLeave.callCount, 1); - wrapper.simulate('touchStart'); - assert.strictEqual(handleMouseEnter.callCount, 1); - assert.strictEqual(handleMouseLeave.callCount, 1); - assert.strictEqual(handleTouchStart.callCount, 1); - wrapper.simulate('mouseEnter'); - wrapper.simulate('touchStart'); - assert.strictEqual(handleMouseEnter.callCount, 2); - assert.strictEqual(handleMouseLeave.callCount, 1); - assert.strictEqual(handleTouchStart.callCount, 2); + const button = getByRole('button'); + + fireEvent.mouseOver(button); + + expect(handleMouseEnter).to.have.property('callCount', 1); + expect(handleMouseLeave).to.have.property('callCount', 0); + expect(handleTouchStart).to.have.property('callCount', 0); + + fireEvent.mouseOut(button); + + expect(handleMouseEnter).to.have.property('callCount', 1); + expect(handleMouseLeave).to.have.property('callCount', 1); + expect(handleTouchStart).to.have.property('callCount', 0); + + // fake touch + fireEvent.touchStart(button, { touches: [{}] }); + + expect(handleMouseEnter).to.have.property('callCount', 1); + expect(handleMouseLeave).to.have.property('callCount', 1); + expect(handleTouchStart).to.have.property('callCount', 1); + + fireEvent.mouseOver(button); + fireEvent.touchStart(button, { touches: [{}] }); + + expect(handleMouseEnter).to.have.property('callCount', 2); + expect(handleMouseLeave).to.have.property('callCount', 1); + expect(handleTouchStart).to.have.property('callCount', 2); }); }); }); From 1554f81755b08ea1252b13d3f84a7e175fca7a86 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 21 Jan 2020 12:41:10 +0100 Subject: [PATCH 3/5] [test] Add failing test for StepButton --- .../material-ui/src/StepButton/StepButton.test.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/material-ui/src/StepButton/StepButton.test.js b/packages/material-ui/src/StepButton/StepButton.test.js index c5d0a2c05f9a77..3e4856cc142054 100644 --- a/packages/material-ui/src/StepButton/StepButton.test.js +++ b/packages/material-ui/src/StepButton/StepButton.test.js @@ -5,6 +5,7 @@ import { createMount, getClasses } from '@material-ui/core/test-utils'; import describeConformance from '../test-utils/describeConformance'; import { createClientRender } from 'test/utils/createClientRender'; import StepButton from './StepButton'; +import Step from '../Step'; import StepLabel from '../StepLabel'; import ButtonBase from '../ButtonBase'; import { fireEvent } from '@testing-library/dom'; @@ -114,4 +115,17 @@ describe('', () => { expect(handleTouchStart).to.have.property('callCount', 2); }); }); + + it('can be used as a child of `Step`', () => { + // a simple smoke test to check that these two + // integrate without any errors/warnings + // TODO: move into integration test for Stepper component + const { getByRole } = render( + + Next + , + ); + + expect(getByRole('button')).to.be.ok; + }); }); From 8ce5c9d211ec09a5b39c51f0d928e6120729446f Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 21 Jan 2020 12:41:36 +0100 Subject: [PATCH 4/5] [StepButton] Fix prop-types warning regarding `expanded` --- packages/material-ui/src/StepButton/StepButton.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/material-ui/src/StepButton/StepButton.js b/packages/material-ui/src/StepButton/StepButton.js index 0e948ac13b9dfe..6f17ae78389a21 100644 --- a/packages/material-ui/src/StepButton/StepButton.js +++ b/packages/material-ui/src/StepButton/StepButton.js @@ -37,6 +37,7 @@ const StepButton = React.forwardRef(function StepButton(props, ref) { className, completed, disabled, + expanded, icon, last, optional, @@ -106,6 +107,11 @@ StepButton.propTypes = { * Disables the button and sets disabled styling. Is passed to StepLabel. */ disabled: PropTypes.bool, + /** + * @ignore + * potentially passed from parent `Step` + */ + expanded: PropTypes.bool, /** * The icon displayed by the step label. */ From 6b0fbb5fc9d51cbd22c9b012faf07658bc2ec23c Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Tue, 21 Jan 2020 19:27:28 +0100 Subject: [PATCH 5/5] fix: invalid touch event --- .../material-ui/src/StepButton/StepButton.test.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/material-ui/src/StepButton/StepButton.test.js b/packages/material-ui/src/StepButton/StepButton.test.js index 3e4856cc142054..c4e4a1dc802c2f 100644 --- a/packages/material-ui/src/StepButton/StepButton.test.js +++ b/packages/material-ui/src/StepButton/StepButton.test.js @@ -73,7 +73,11 @@ describe('', () => { }); describe('event handlers', () => { - it('should forward mouseenter, mouseleave and touchstart', () => { + it('should forward mouseenter, mouseleave and touchstart', function touchTests() { + if (typeof Touch === 'undefined') { + this.skip(); + } + const handleMouseEnter = spy(); const handleMouseLeave = spy(); const handleTouchStart = spy(); @@ -101,14 +105,16 @@ describe('', () => { expect(handleTouchStart).to.have.property('callCount', 0); // fake touch - fireEvent.touchStart(button, { touches: [{}] }); + const firstTouch = new Touch({ identifier: 0, target: button }); + fireEvent.touchStart(button, { touches: [firstTouch] }); expect(handleMouseEnter).to.have.property('callCount', 1); expect(handleMouseLeave).to.have.property('callCount', 1); expect(handleTouchStart).to.have.property('callCount', 1); fireEvent.mouseOver(button); - fireEvent.touchStart(button, { touches: [{}] }); + const secondTouch = new Touch({ identifier: 1, target: button }); + fireEvent.touchStart(button, { touches: [firstTouch, secondTouch] }); expect(handleMouseEnter).to.have.property('callCount', 2); expect(handleMouseLeave).to.have.property('callCount', 1);