-
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
Editor: Refactor PostSavedState
tests to @testing-library/react
#43769
Conversation
Size Change: 0 B Total Size: 1.25 MB ℹ️ View Unchanged
|
jest.mock( '@wordpress/icons/src/icon', () => () => ( | ||
<div data-testid="test-icon" /> | ||
) ); |
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.
What is the issue preventing us from using the real Icon
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.
I didn't find a good way to query for the icon. Any recommendations?
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.
Mhh, I can't find either. Ideally we could at least assign a default role="presentation"
to these icons, or at otherwise label them with some accessible text. But it's not a topic for this PR
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.
Also, the Button
in PostSavedDate
can render 2 icons at the same time (one as children
and one via the icon
prop) — that's weird
What?
We've recently started refactoring
enzyme
tests to@testing-library/react
.This PR refactors the
<PostSavedState />
component 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.The purpose of this PR is not to improve or enrich the tests themselves, but rather to migrate them. Our main motivation is unblocking the upgrade to React 18.
How?
We're straightforwardly replacing
enzyme
tests with@testing-library/react
ones, usingjest-dom
matchers and mocks to avoid testing unrelated implementation details.Testing Instructions
Verify tests pass:
npm run test:unit packages/editor/src/components/post-saved-state/test/index.js