weaveworks / weave-gitops-enterprise

This repo provides the enterprise level features for the weave-gitops product, including CAPI cluster creation and team workspaces.
https://docs.gitops.weave.works/
Apache License 2.0
160 stars 30 forks source link

Look into eslint testing-library rules #3484

Open joshri opened 1 year ago

joshri commented 1 year ago

There are four rules I turned off in order to actually get the eslint PR merged that have to do with our tests. Are these rules worth following?

testing-library/no-node-access: https://github.com/testing-library/eslint-plugin-testing-library/blob/main/docs/rules/no-node-access.md - DOM things like .closest or lastChild should not be mixed with RTL selectors apparently

testing-library/no-unnecessary-act: https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning - this one is complicated and unfortunately in almost every single test

testing-library/render-result-naming-convention: there are some cases in tests that try to name the render result const variable as screen, which is a different thing from RTL.

testing-library/prefer-screen-queries: This rule recommends using testing library selectors instead of DOM selectors like querySelectorAll. Our tests should probably support this.

jpellizzari commented 1 year ago

Let's treat this like a Spike and research whether or not each of these are useful.