-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Components: Refactor ColorPalette
tests to @testing-library/react
#44108
Conversation
2572596
to
ec15b93
Compare
Size Change: -119 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
|
||
test( 'should call onToggle on click.', () => { | ||
renderedToggleButton.find( 'button' ).simulate( 'click' ); | ||
await user.click( |
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.
Should we add a spy for the onToggle
callback prop and make sure that .toHaveBeenCalledTimes( 1 )
, like in the original test?
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.
Hm, how would you recommend this to happen without modifications to the original component?
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.
My bad, I didn't realize that the Dropdown
component is an implementation detail, internal to ColorPalette
and therefore it's not possible to pass a spy for onToggle
😅
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.
🚀
|
||
test( 'should call onToggle on click.', () => { | ||
renderedToggleButton.find( 'button' ).simulate( 'click' ); | ||
await user.click( |
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.
My bad, I didn't realize that the Dropdown
component is an implementation detail, internal to ColorPalette
and therefore it's not possible to pass a spy for onToggle
😅
What?
We've recently started refactoring
enzyme
tests to@testing-library/react
.This PR refactors the
ColorPalette
tests fromenzyme
to@testing-library/react
.Why?
@testing-library/react
provides a better way to write tests for accessible components that is closer to the way the user experiences them.How?
We're straightforwardly replacing
enzyme
tests with@testing-library/react
ones, usingjest-dom
matchers and mocks to avoid testing unrelated implementation details.Ideally, we should not be using snapshot tests here, however, the purpose of this PR is not to improve or enrich the tests themselves, but rather to migrate them
@testing-library/react
. Our primary motivation is unblocking the upgrade to React 18.In any case, it would be a good future step to improve the tests and test actual experience and interaction in order to improve the overall value of the tests.
Testing Instructions
Verify tests pass:
npm run test:unit packages/components/src/color-palette/test/index.js