-
Notifications
You must be signed in to change notification settings - Fork 425
Conversation
I introduced a bug where we wouldn't snapshot cases after the first failure, even if failures are expected. This wasn't a problem in the original code because it separated JSX/no-JSX modes into separate tests. I fixed my bug by rewriting how error logic works. Hopefully it's more clear now too because it doesn't have to bubble errors all the way up just to run an assertion.
Useful for quick reruns when you don't care about JSX specifically. This lets us skip both 3/4 of tests, and Babel.
I think this is ready for a review now. I'll check remaining followups either later in this PR, or in the next one. This reduces the time it takes to run It gives you an ability to run a subset of tests from command line, e.g. When there's a bug in one output mode (e.g. createElement => createElement), it now fails immediately for a particular test case, and doesn't attempt it in other modes (since they would likely fail too). So failures should appear faster. Additionally, this PR adds I also added Prettier to tests so we can stop doing the formatting nits. |
Didn't help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
Okay, I figured out the last pieces. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gaearon is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Currently we have a single giant file with all tests, and a giant snapshot. This is both slow, and hard to work with and iterate on.
In this PR I will refactor our test setup.
yarn test-react-fast <Filename>
Potential followups:
this['React']
assignment with a funny comment in every test