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

Use react-testing-library #18

Closed
dferber90 opened this issue Sep 4, 2018 · 9 comments
Closed

Use react-testing-library #18

dferber90 opened this issue Sep 4, 2018 · 9 comments
Labels
🌗 good rotation topic Good to rotate a member into Application Kit to work on this topic

Comments

@dferber90
Copy link
Contributor

dferber90 commented Sep 4, 2018

I would like to try out react-testing-library instead of using enzyme. In my experience it leads to more test coverage and more confidence while writing less code. The tests are also decoupled from the implementation that way.

If this is successful, I'd like to move all tests to react-testing-library and get rid of enzyme completely.

Additionally, our current tests are extremely slow in CI, some taking almost a minute.

 PASS   test  src/components/buttons/link-button/link-button.spec.js (52.084s)
 PASS   test  src/components/typography/text/text.spec.js (52.227s)
 PASS   test  src/components/inputs/number-input/number-input.spec.js (45.348s)
 PASS   test  src/components/switches/toggle/toggle-switch.spec.js (51.875s)
 PASS   test  src/utils/localized.spec.js (51.361s)
 PASS   test  src/components/buttons/ghost-button/ghost-button.spec.js (54.128s)
 PASS   test  src/components/buttons/accessible-button/accessible-button.spec.js (52.417s)

Source: https://travis-ci.org/commercetools/ui-kit/builds/424350666.

This might improve with react-testing-library as well.

@tdeekens
Copy link
Contributor

tdeekens commented Sep 4, 2018

I think (and I would be pleased to be wrong) that performance will not increase. My suspicion with the performance problems is that we don't have a --maxWorkers set on CI. That leads to Jest spawning more processes than the CI system is capable of handling eventually doing more harm than good in a slowdown of runs. Try to set --maxWorkers=5 or something and see how that affects things.

Still, adoption of react-testing-library is awesome!

@dferber90
Copy link
Contributor Author

Sure! I debugged the slowdown-over-time before and found that there is a memory leak somewhere in our setup, but I couldn't get to the bottom of it. That's why I have some hopes, that we could avoid the memory leak in case we get rid of enzyme and the related components. Now that you mention it though, the maxWorkers issue seems to be the cause of the slowdown observed here.

@tdeekens
Copy link
Contributor

tdeekens commented Sep 4, 2018

The node-sdk also has it specified. Same for the mc-fe. At a certain amount of specs Jest seems to go overboard with parallelism. Maybe it either: does not gather enough metrics of the underlying system or does not adjust itself when it detects it itself is slow or a CI systems lies offer insane resources and then throttles.

@emmenko
Copy link
Member

emmenko commented Sep 5, 2018

Out of scope Q: which label should we use to mark such issues?

@tdeekens
Copy link
Contributor

tdeekens commented Sep 5, 2018

Something like "Technical Exploration" or "Technical Experiment"?

@emmenko
Copy link
Member

emmenko commented Sep 5, 2018

Sounds good. Feel free to add it (or should I since I asked?)

@tdeekens
Copy link
Contributor

tdeekens commented Sep 5, 2018

I'd like a UIKit core contributor to confirm and then add whatever he/she thinks works best.

@dferber90
Copy link
Contributor Author

dferber90 commented Sep 5, 2018

Done

@dferber90 dferber90 added the 🌗 good rotation topic Good to rotate a member into Application Kit to work on this topic label Sep 18, 2018
@dferber90
Copy link
Contributor Author

We now have react-testing-library so I'm going to close this. We're currently using a mix of Enzyme (for old components) and react-testing-library (for new ones).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌗 good rotation topic Good to rotate a member into Application Kit to work on this topic
Projects
None yet
Development

No branches or pull requests

3 participants