Skip to content
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

fix: improve tests #1504

Merged
merged 2 commits into from
Jul 30, 2021
Merged

fix: improve tests #1504

merged 2 commits into from
Jul 30, 2021

Conversation

ramniel
Copy link
Contributor

@ramniel ramniel commented Jul 21, 2021

With the help of @jcamposmk, consulting the documentation of the react testing library and analyzing some articles about the author of the library, we found some recommendations on tests, which may be useful. We would like to know your opinions on this.

  • There is no need to call the cleanup function as it is called automatically for Jest and other test frameworks.

  • The use of screen is recommended:
    The benefit of using screen is you no longer need to keep the render call destructure up-to-date as you add/remove the queries you need. You only need to type screen. and let your editor's magic autocomplete take care of the rest.

    *It seems like the returned object from the render function could be framework specific but the screen object will be framework agnostic, making your tests more resilient and consistent across projects.

  • Wrapping things in act unnecessarily. Some queries and functions are already wrapped in act

  • Use *byRole most of the time:
    There's not much you can't get with this (if you can't, it's possible your UI is inaccessible).
    Queries priority

  • Do not use waitFor to wait for items that can be queried with find*
    (find* queries use waitFor under the hood)

  • Do not put multiple assertions in a single waitFor callback
    By putting a single assertion, we can wait for the UI to settle to the state we want to assert on, and also fail faster if one of the assertions do end up failing.

  • Use @testing-library/user-event
    Is a package that's built on top of fireEvent, but it provides several methods that resemble the user interactions more closely. It provides more advanced simulation of browser interactions than the built-in fireEvent method.

Common mistakes
Cheatsheet

Other changes:

  • The data-testid attribute was added to the Loading component to obtain the component by this attribute and not by the name of the class. This way we make sure that the test keeps working no matter if the class name changes.
    In production: You can compile this attribute away with babel-plugin-react-remove-properties.

  • The aria-label attribute was added to the form and the inputs fields to differentiate them by their role and accessibility name

Using aria-label attribute

@cbernat
Copy link
Contributor

cbernat commented Jul 29, 2021

The changes seem reasonable, perhaps you could update this PR and merge it. And also be on the lookout for tests reviews to apply this way of doing thinks, from now on 😅

@ramniel ramniel force-pushed the DK-74-improve-tests branch from 9b72c55 to c96771a Compare July 30, 2021 12:01
@ramniel ramniel requested a review from cbernat July 30, 2021 12:03
@ramniel ramniel marked this pull request as ready for review July 30, 2021 12:03
@ramniel ramniel force-pushed the DK-74-improve-tests branch from c96771a to de5d3b6 Compare July 30, 2021 20:40
@ramniel ramniel merged commit e239dc1 into FromDoppler:master Jul 30, 2021
@andresmoschini
Copy link
Member

🎉 This PR is included in version 1.217.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants