-
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 a few component tests to @testing-library/react
#43376
Conversation
Size Change: +1.33 kB (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
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.
LGTM 🚀
Just a nit: when we're asserting for an element to be in the document / visible, we could use getBy...
queries instead of queryBy...
, since the getBy...
explicitly throw when the element is not found.
It wouldn't make a difference in this PR, so it's mostly a convention / stylistic choice — up to you!
Updated in 5f4497a, happy to keep things consistent. I can't stop wondering though, how is that useful, considering that using |
I don't feel super strongly about it to be honest, but the general recommendation given by Kent is to only use |
Ah, I was looking for that, thanks @mirka 🙌 |
What?
We've recently started refactoring
enzyme
tests to@testing-library/react
.This PR refactors a few of the simpler editor component tests from
enzyme
to@testing-library/react
. I've specifically picked the most straightforward ones that boil down to "should render" or "should not render".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
matches and screen queries.Testing Instructions
Verify tests pass:
npm run test:unit packages/editor/src/components