chore(site): Try to fix flake test #6848
Conversation
BrunoQuaresma
commented
Mar 28, 2023
- Add a lint for the testing library to avoid bad patterns. I see this being very helpful for non-FE folks
- Increased the timeout for the loader to disappear. There are a few pages we do 3 ~5 requests to get the data and sometimes 1s is not enough to get them even with mock.
site/.eslintrc.yaml
Outdated
| # These rules are disabled because they can degrade test speed a bit since | ||
| # sometimes we need to access the directly in a lower scope to prevent the | ||
| # library to fetch a bunch of element |
There was a problem hiding this comment.
Sorry, I don't understand this comment. Could you try to explain a little differently?
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah, gotcha. Maybe we could link those docs and phrase it slightly differently:
"Occasionally, we must traverse the DOM when querying for an element to avoid the performance costs that come with using selectors like ByRole. You can read more about these performance costs here."
| it("should show stack when encountering runtime error", () => { | ||
| renderComponent() |
There was a problem hiding this comment.
What's the benefit of sticking this code scaffolding in the renderComponent util method instead of a beforeEach?
There was a problem hiding this comment.
Sure, it is a testing-library role: https://github.com/testing-library/eslint-plugin-testing-library/blob/main/docs/rules/no-render-in-setup.md I like not having too much nesting or things that are not running "close" on where it is running, but I have no strong opinions on that so we can just remove it if you think that is not usable at all
There was a problem hiding this comment.
Oh, interesting! Thanks for the link; I remember the nesting article you posted in the dev channel a while back, but I didn't realize the concern was not only abstraction but also mutable variables.
This looks good to me!