-
Notifications
You must be signed in to change notification settings - Fork 213
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
Changes from all 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 15701e2
test(switch): Redo tests with RTL
lychyi 0369ad0
test(switch): Fix tests
lychyi eeb610a
test(switch): Remove old tests
lychyi fad6c15
test(switch): Organize tests, add style tests
lychyi 2a368e0
test(switch): Clean up extra afterEach and cleanup
lychyi c718440
test(switch): Adjust a couple test names
lychyi f1af44b
test(switch): Add cypress tests
lychyi 242daf5
test(switch): Add static state table for vizreg
lychyi c279eca
Merge branch 'master' into switch-tests
lychyi 172b4cf
test(switch): Convert state table to maps
lychyi bda17c9
test(switch): Remove style tests for error states
lychyi 2614174
test(switch): Decouple act/assert from tests
lychyi 992a4ca
Merge branch 'master' of github.com:Workday/canvas-kit into switch-tests
lychyi 4f9d770
test(switch): Remove unnecessary code/test
lychyi 017392f
Merge branch 'master' of github.com:Workday/canvas-kit into switch-tests
lychyi 0e1a9dc
test(switch): Simplify and align Cypress tests
lychyi bda5e6a
test(switch): Organize and improve tests
lychyi 5b24c60
test(switch): Move style specs to other blocks
lychyi 592dec2
Merge branch 'master' into switch-tests
lychyi 4b48608
test(switch): Add hover to visual testing
lychyi 62f78d9
test(switch): Combine input ref tests
lychyi 0d2b99f
test(switch): Fix typo in test desc
lychyi 0d33e9f
test(switch): Remove focus/active state when disabled
lychyi df9c776
Merge branch 'master' into switch-tests
lychyi 8e1d247
test(switch): Add StaticStatesTable
lychyi 9f76dc7
Merge branch 'master' into switch-tests
lychyi d8d06e1
Merge branch 'master' into switch-tests
anicholls 14dde09
test(switch): Use new ComponentState wrapper
lychyi 5cd6033
Merge branch 'master' into switch-tests
lychyi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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'); | ||
}); | ||
}); | ||
}); | ||
}); | ||
|
||
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
|
||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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()}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||
}); | ||
}); | ||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 aSwitch
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.