test(text-area): Change tests to react-testing-library and improve coverage #394
Conversation
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
stephanerangaya
changed the title
They work I think, aren't they? |
| const ref: React.RefObject<HTMLInputElement> = React.createRef(); | ||
| render(<TextArea inputRef={ref} id={id} />); | ||
| expect(ref.current).not.toBeNull(); | ||
| expect(ref.current).toHaveAttribute('id', id); |
There was a problem hiding this comment.
I think the following should work:
expect(await getByRole('textbox')).toEqual(ref.current)Also, as a side-note, I think it is safer for Testing Library APIs to be used with the promise API since rendering can be asynchronous. The synchronous API could break if we slightly change the implementation of components or React slightly changes timing (e.g. useEffect vs componentDidMount or concurrent mode)
There was a problem hiding this comment.
@NicholasBoll Are you saying that we should aways be using expect(await getBy*()) instead of expect(getBy*())?
There was a problem hiding this comment.
I know a bit more about the React Testing Library APIs. There are get*, query*, and find* queries.
find* is most likely what we want for React because of timing. find* returns a promise that resolves when the element is found (retrying until it is found). If there is a delay in rendering something (hooks, concurrent mode, etc), find* will eventually find the element and return it for testing.
So I should update my previous code snippet:
expect(await findByRole('textbox')).toEqual(ref.current)There was a problem hiding this comment.
I'm not sure if await find* vs get* is strictly necessary. React Testing Library hooks into some testing internals to fix timing issues introduced with hooks. I just consider timing to be an implementation detail of the component that should not introduce failures or flakiness of tests.
|
@mannycarrera4 Do you want to approve the IconButton differences? I think they are from the IconButton changes, but I wanted to be sure. The other change is new visual states for the text area. I'm looping in UX to make sure we got it right and that it makes sense. |
Summary
Related to #286
Converts test to RTL, improves coverage, adds a static states story for visual regression testing, and uses a more BDD (behavioral-driven) approach for unit tests.
Checklist
yarn testpassespackage.json