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

Testing approaches in this project #918

Closed
m0t0r opened this issue Nov 5, 2020 · 5 comments
Closed

Testing approaches in this project #918

m0t0r opened this issue Nov 5, 2020 · 5 comments

Comments

@m0t0r
Copy link

m0t0r commented Nov 5, 2020

@williaster @hshoff I have a few questions about testing approaches in this project. I understand Airbnb probably has its coding tools standards but I was wondering how strict they are ? :) For example, would it be possible to migrate from enzyme to a much better Testing Library. I also noticed use of shallow rendering in tests which is not the best approach. There is also cypress-react-unit-test which for projects like visx could be a nice option to consider as well, maybe not for all but some of the interactive tests because it renders in real browser. Could you share your thoughts ?

@williaster
Copy link
Collaborator

williaster commented Nov 6, 2020

Hey @m0t0r thanks for creating this issue 🙏 (and sorry about the complexities of this as it relates to the react@17 upgrade in #872, yak shaving for sure)

Generally Airbnb is moving toward using react-testing-library (which you've linked to), so I think we are supportive of moving in that direction for visx over enzyme. Currently I'm not sure we have the bandwidth to do this migration ourselves, so I'll tag this issue with 👋 help wanted. If you or others are interested in helping with this we would happily review any PRs 👀 .

As far as cypress / visual testing, we are planning to add screenshot testing with happo 🦛 soon (my plan is before end of 2020) for the demo package so that we can better-detect visual changes.

Not sure if @hshoff has any other thoughts to add.

@m0t0r
Copy link
Author

m0t0r commented Nov 6, 2020

Hi @williaster, great, thanks for your reply, I appreciate. Yes, migrating to react-testing-library may take quite a bit of efforts. I will try to start looking into that and maybe get an initial plan.

@hshoff
Copy link
Member

hshoff commented Nov 11, 2020

My preference would be to wait for the official enzyme 17 adapter. Migrating the entire test suite to react-testing-library should be a separate discussion vs supporting react 17 as a peerDep.

@whitetiger1399
Copy link

@hshoff are you also planning to use jest for testing?

@williaster
Copy link
Collaborator

We currently use jest for unit tests and I think this would be true even if we moved to react-testing-library since jest provides the runner + the assertion framework, not the react manipulations.

Also an update, we now use happo for visual testing of all gallery examples as added in #1030

@hshoff hshoff closed this as completed Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants