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(switch): Redo switch tests in react-testing-library #386

Merged
merged 30 commits into from
Jan 28, 2020
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
b75ed3c
test: Move jest-emotion matchers to global setup
lychyi Dec 27, 2019
15701e2
test(switch): Redo tests with RTL
lychyi Dec 27, 2019
0369ad0
test(switch): Fix tests
lychyi Dec 27, 2019
eeb610a
test(switch): Remove old tests
lychyi Dec 27, 2019
fad6c15
test(switch): Organize tests, add style tests
lychyi Dec 28, 2019
2a368e0
test(switch): Clean up extra afterEach and cleanup
lychyi Dec 28, 2019
c718440
test(switch): Adjust a couple test names
lychyi Jan 2, 2020
f1af44b
test(switch): Add cypress tests
lychyi Jan 3, 2020
242daf5
test(switch): Add static state table for vizreg
lychyi Jan 3, 2020
c279eca
Merge branch 'master' into switch-tests
lychyi Jan 3, 2020
172b4cf
test(switch): Convert state table to maps
lychyi Jan 3, 2020
bda17c9
test(switch): Remove style tests for error states
lychyi Jan 3, 2020
2614174
test(switch): Decouple act/assert from tests
lychyi Jan 3, 2020
992a4ca
Merge branch 'master' of github.com:Workday/canvas-kit into switch-tests
lychyi Jan 8, 2020
4f9d770
test(switch): Remove unnecessary code/test
lychyi Jan 8, 2020
017392f
Merge branch 'master' of github.com:Workday/canvas-kit into switch-tests
lychyi Jan 13, 2020
0e1a9dc
test(switch): Simplify and align Cypress tests
lychyi Jan 14, 2020
bda5e6a
test(switch): Organize and improve tests
lychyi Jan 14, 2020
5b24c60
test(switch): Move style specs to other blocks
lychyi Jan 14, 2020
592dec2
Merge branch 'master' into switch-tests
lychyi Jan 14, 2020
4b48608
test(switch): Add hover to visual testing
lychyi Jan 15, 2020
62f78d9
test(switch): Combine input ref tests
lychyi Jan 15, 2020
0d2b99f
test(switch): Fix typo in test desc
lychyi Jan 17, 2020
0d33e9f
test(switch): Remove focus/active state when disabled
lychyi Jan 17, 2020
df9c776
Merge branch 'master' into switch-tests
lychyi Jan 22, 2020
8e1d247
test(switch): Add StaticStatesTable
lychyi Jan 22, 2020
9f76dc7
Merge branch 'master' into switch-tests
lychyi Jan 23, 2020
d8d06e1
Merge branch 'master' into switch-tests
anicholls Jan 24, 2020
14dde09
test(switch): Use new ComponentState wrapper
lychyi Jan 25, 2020
5cd6033
Merge branch 'master' into switch-tests
lychyi Jan 28, 2020
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
46 changes: 46 additions & 0 deletions cypress/integration/Switch.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import * as h from '../helpers';

const getSwitch = () => {
return cy.get(`[type="checkbox"]`);
};

describe('Switch', () => {
before(() => {
h.stories.visit();
});
['Default', 'Alert', 'Error'].forEach(story => {
context(`given the '${story}' story is rendered`, () => {
beforeEach(() => {
h.stories.load('Components|Inputs/Switch/React/Top Label', story);
});

it('should pass accessibility checks', () => {
cy.checkA11y();
});

context('when clicked', () => {
beforeEach(() => {
getSwitch().click();
});

it('should be checked', () => {
getSwitch().should('be.checked');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this duplication of RTL/enzyme tests?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cypress tests are about specifications from a user's perspective. This spec reads in full "Non-disabled Switch when the switch is clicked should be checked". Linking the given of the story being rendered, the when of the switch being clicked, and the then of the side effect of that action where the switch is now checked.

I might argue that "checked" is an implementation detail of a switch being "on", but there is a bit of controversy around how a "switch" should be labelled/read by screen readers from an accessibility standpoint. I don't remember the final say the role attribute of a Switch which effects how it will be read by a screen reader.

I personally avoid actions in unit-level tests including clicks. Most of the time firing a click event is enough for the semantic action we'd call "click", but some components will expect other browser events in a specific order and some events aren't as simple as a "click". I leave that all for Cypress which has wiring for all that. In Cypress, a "click" includes other events normally associated with "click" like "mouseover". I've seen unit tests that try to recreate all that. Yuck.

I wouldn't worry about possible duplication in testing levels. Each serve a specific purpose.

});
});
});
});

context(`given the 'Disabled' story is rendered`, () => {
beforeEach(() => {
h.stories.load('Components|Inputs/Switch/React/Top Label', 'Disabled');
});

it('should pass accessibility checks', () => {
cy.checkA11y();
});

it('should be disabled', () => {
getSwitch().should('be.disabled');
lychyi marked this conversation as resolved.
Show resolved Hide resolved
});
});
});
3 changes: 2 additions & 1 deletion jest/setupTests.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import Adapter from 'enzyme-adapter-react-16';
import {configure} from 'enzyme';
import {toHaveNoViolations} from 'jest-axe';
import serializer from 'jest-emotion';
import serializer, {matchers} from 'jest-emotion';
import '@testing-library/jest-dom/extend-expect';

expect.addSnapshotSerializer(serializer);
expect.extend(toHaveNoViolations);
expect.extend(matchers);
configure({adapter: new Adapter()});
79 changes: 79 additions & 0 deletions modules/form-field/react/stories/stories_Switch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ import {storiesOf} from '@storybook/react';
import withReadme from 'storybook-readme/with-readme';
import {ControlledComponentWrapper} from '../../../../utils/storybook';

import styled from '@emotion/styled';
import {Switch} from '../../../switch/react/index';
import FormField from '../index';
import README from '../../../switch/react/README.md';
import {StaticStates} from '@workday/canvas-kit-labs-react-core/lib/StaticStates';
import {ErrorType} from '@workday/canvas-kit-react-common';

const control = (child: React.ReactNode) => (
<ControlledComponentWrapper controlledProp={ControlledComponentWrapper.ControlledProp.Checked}>
Expand All @@ -17,6 +20,20 @@ const control = (child: React.ReactNode) => (
const hintText = 'Helpful text goes here.';
const hintId = 'error-desc-id';

const Table = styled('table')({
width: '100%',
thead: {
textAlign: 'left',
paddingBottom: 16,
},
'td, th': {
minWidth: 100,
paddingBottom: 16,
paddingRight: 16,
textAlign: 'left',
},
});

storiesOf('Components|Inputs/Switch/React/Top Label', module)
.addParameters({component: Switch})
.addDecorator(withReadme(README))
Expand Down Expand Up @@ -90,3 +107,65 @@ storiesOf('Components|Inputs/Switch/React/Left Label', module)
{control(<Switch />)}
</FormField>
));

storiesOf('Components|Inputs/Switch/React/Visual Testing', module)
lychyi marked this conversation as resolved.
Show resolved Hide resolved
lychyi marked this conversation as resolved.
Show resolved Hide resolved
.addParameters({component: Switch})
.addDecorator(withReadme(README))
.add('States', () => (
<StaticStates>
<Table>
<thead>
<tr>
<th></th>
<th>No state</th>
<th>Hover</th>
<th>Focused</th>
anicholls marked this conversation as resolved.
Show resolved Hide resolved
<th>Active</th>
</tr>
</thead>
<tbody>
{[false, true].map(disabled => {
return [false, true].map(checked => {
return ['Default', 'Alert', 'Error'].map(variant => {
const type =
variant === 'Alert'
? ErrorType.Alert
: variant === 'Error'
? ErrorType.Error
: undefined;

const key = `${checked ? 'checked' : 'unchecked'} ${
disabled ? 'disabled' : 'enabled'
} ${variant}`;

lychyi marked this conversation as resolved.
Show resolved Hide resolved
const pseudoStates = ['', 'hover'];

if (!disabled) {
pseudoStates.push('focus', 'active');
}

return (
<tr key={key}>
<td>{`${disabled ? 'Disabled ' : ''}${variant} (${
checked ? 'checked' : 'unchecked'
})`}</td>
{pseudoStates.map(className => (
<td key={`${key} + ${className}`}>
<Switch
checked={checked}
onChange={() => {}} // eslint-disable-line no-empty-function
disabled={disabled}
error={type}
className={className}
lychyi marked this conversation as resolved.
Show resolved Hide resolved
/>
</td>
))}
</tr>
);
});
});
})}
</tbody>
</Table>
</StaticStates>
));
3 changes: 0 additions & 3 deletions modules/page-header/react/spec/PageHeader.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@ import PageHeader from '../lib/PageHeader';
import {mount} from 'enzyme';
import {IconButton} from '@workday/canvas-kit-react-button';
import {exportIcon, fullscreenIcon} from '@workday/canvas-system-icons-web';
import {matchers} from 'jest-emotion';

expect.extend(matchers);

describe('Page Header', () => {
const cb = jest.fn();
Expand Down
152 changes: 116 additions & 36 deletions modules/switch/react/spec/Switch.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,44 +1,124 @@
import * as React from 'react';
import {mount} from 'enzyme';
import {render, fireEvent} from '@testing-library/react';
import Switch from '../lib/Switch';

describe('Switch', () => {
const cb = jest.fn();

/**
* Today, this is hardcoded but this could possibly be
* configurable in the future (e.g. role='switch').
* In fact, 'checkbox' probably isn't the correct role
* according to MDN and ARIA
* https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/Switch_role
* https://www.w3.org/TR/wai-aria-1.1/#switch
*/
const role = 'checkbox';

afterEach(() => {
cb.mockReset();
});

test('Switch should spread extra props', () => {
const component = mount(<Switch data-propspread="test" onChange={cb} />);
const input = component
.find('input')
// TODO: Standardize on prop spread location (see #150)
.getDOMNode();
expect(input.getAttribute('data-propspread')).toBe('test');
component.unmount();
});

test('Switch creates a unique id for each instance', async () => {
const fragment = mount(
<form>
<Switch checked={true} onChange={jest.fn()} disabled={false} />;
<Switch onChange={jest.fn()} disabled={false} />;
</form>
);

const id1 = fragment
.find('input')
.at(0)
.getDOMNode()
.getAttribute('id');

const id2 = fragment
.find('input')
.at(1)
.getDOMNode()
.getAttribute('id');

expect(id1).not.toEqual(id2);
fragment.unmount();
cb.mockClear();
});

describe('when rendered', () => {
it('should render an input element with `type=checkbox`', () => {
const {getByRole} = render(<Switch onChange={cb} />);
expect(getByRole(role)).toHaveProperty('type', 'checkbox');
});

it('should be unchecked by default', () => {
const {getByRole} = render(<Switch onChange={cb} />);
expect(getByRole(role)).toHaveProperty('checked', false);
});

test('should have a "pointer" cursor', () => {
const {getByRole} = render(<Switch onChange={cb} />);
expect(getByRole(role)).toHaveStyleRule('cursor', 'pointer');
});
});

describe('when rendered with checked=true', () => {
it('should render a checked checkbox input', () => {
const {getByRole} = render(<Switch checked={true} onChange={cb} />);
expect(getByRole(role)).toHaveProperty('checked', true);
});
});

describe('when rendered with an id', () => {
it('should render a checkbox input with that id', () => {
const id = 'switchy';
const {getByRole} = render(<Switch id={id} onChange={cb} />);

expect(getByRole(role)).toHaveAttribute('id', id);
});
});

describe('when rendered without an id', () => {
it('should create a unique id for each instance', () => {
const testIds = ['test1', 'test2'];
const {getByTestId} = render(
<>
<Switch data-testid={testIds[0]} onChange={cb} />
<Switch data-testid={testIds[1]} onChange={cb} />
</>
);

const id1 = getByTestId(testIds[0]).getAttribute('id');
const id2 = getByTestId(testIds[1]).getAttribute('id');

expect(id1).not.toEqual(id2);
});

it('should keep the same unique id if re-rendered', () => {
const {getByRole, rerender} = render(<Switch checked={false} onChange={cb} />);

const uniqueId = getByRole(role).getAttribute('id');
expect(getByRole(role)).toHaveProperty('id', uniqueId);

rerender(<Switch checked={true} onChange={cb} />);

expect(getByRole(role)).toHaveProperty('checked');
expect(getByRole(role)).toHaveProperty('id', uniqueId);
});
});

describe('when rendered with the disabled prop', () => {
it('should render a disabled checkbox input', () => {
const {getByRole} = render(<Switch disabled={true} onChange={cb} />);
expect(getByRole(role)).toHaveProperty('disabled', true);
});
it('should have a "not-allowed" cursor', () => {
const {getByRole} = render(<Switch disabled={true} onChange={cb} />);
expect(getByRole(role)).toHaveStyleRule('cursor', 'not-allowed');
});
});

describe('when rendered with extra, arbitrary props', () => {
it('should apply those extra props to the checkbox input', () => {
const testValue = 'test';

const {getByRole} = render(<Switch data-propspread={testValue} onChange={cb} />);
expect(getByRole(role)).toHaveAttribute('data-propspread', testValue);
});
});

describe('when rendered with an input ref', () => {
it("should set the ref's current property to the checkbox input element", () => {
const ref = React.createRef<HTMLInputElement>();

render(<Switch inputRef={ref} onChange={cb} />);

expect(ref.current).not.toBeNull();
expect(ref.current).toHaveAttribute('role', role);
});
});

describe('when clicked', () => {
it('should call the onChange callback', () => {
const {getByRole} = render(<Switch onChange={cb} />);

fireEvent.click(getByRole(role));

expect(cb).toHaveBeenCalledTimes(1);
});
});
});