Skip to content

docs: Add more info about slow tests in the FE #6584

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

Merged
merged 1 commit into from
Mar 14, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions docs/contributing/frontend.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,29 @@ Choosing what to test is not always easy since there are a lot of flows and a lo
- Things that can block the user
- Reported bugs
- Regression issues

### Tests getting too slow

A few times you can notice tests can take a very long time to get done. Sometimes it is because the test itself is complex and runs a lot of stuff, and sometimes it is because of how we are querying things. In the next section, we are going to talk more about them.

#### Using `ByRole` queries

One thing we figured out that was slowing down our tests was the use of `ByRole` queries because of how it calculates the role attribute for every element on the `screen`. You can read more about it on the links below:

- https://stackoverflow.com/questions/69711888/react-testing-library-getbyrole-is-performing-extremely-slowly
- https://github.com/testing-library/dom-testing-library/issues/552#issuecomment-625172052

Even with `ByRole` having performance issues we still want to use it but for that, we have to scope the "querying" area by using the `within` command. So instead of using `screen.getByRole("button")` directly we could do `within(form).getByRole("button")`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Even with `ByRole` having performance issues we still want to use it but for that, we have to scope the "querying" area by using the `within` command. So instead of using `screen.getByRole("button")` directly we could do `within(form).getByRole("button")`.
Despite its performance issues, `ByRole` is still a valuable selector. When using it, we should scope the "querying" area by using the `within` command. So instead of using `screen.getByRole("button")` directly we could do `within(form).getByRole("button")`.


❌ Not ideal. If the screen has a hundred or thousand elements it can be VERY slow.

```tsx
user.click(screen.getByRole("button"))
```

✅ Better. We can limit the number of elements we are querying.

```tsx
const form = screen.getByTestId("form")
user.click(within(form).getByRole("button"))
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit surprised to see this section in our README instead of in our wiki; however, I'm not opposed to having it here. I'm curious how this section would scale - I imagine there are all sorts of testing strategies and pitfalls we could elaborate on; which ones will we deem important enough to stick in our README? Should we eventually have an externally-facing wiki? Should we just link to a list of SO articles and Kent C Dodd's blogs we find insightful? Just some food for thought. I really appreciate the clarity of the writeup!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is better to have things on docs as markdown so it is also added to the website docs improving SEO as a side effect.