nytimes/redux-taxi

Refactor tests to Jest

oziniak opened this issue · 5 comments

oziniak commented

This PR #2 introduces migration to prop-types package, introduced with React v15.5.0. New prop-types don't throw an error, but console.error (facebook/prop-types#28).

Would be great to refactor tests to Jest for the Future maintenance and re-enable skipped test

frytyler commented

@oziniak I'm down to help out with this. I'll get started and submit a PR when it's ready. Any advice or things I should know before jumping in?

tizmagik commented

Go for it @frytyler -- it should be a fairly straightforward conversion. If you have any questions don't hesitate to post back. Thanks for taking this on! 🙌

frytyler commented

@tizmagik Awesome, I've got it working on my end but before I PR it just wondering if the mocha.setup.js file needs to be carried over. I don't have any failing tests with it left out so I'm thinking it can be removed. Might have been a previous dependency.

Also was the intent to just use Jest instead of mocha and keep using Chai's assert? I haven't made any changes to the actual test suits.

I'll PR once I hear back from you.

frytyler commented

@tizmagik One more question I don't have access to push a branch here. How do you want to handle the PR? Should I fork and create a PR instead?

tizmagik commented

@frytyler great! Feel free to leave off mocha.setup.js if it works without it.

Jest comes with its own assertion library, so it would be nice to drop Chai in favor of Jest's. Feel free to do that as 2 separate PRs.

Yes, fork and PR from your fork.