search

nytimes / redux-taxi

🚕 Component-driven asynchronous SSR in isomorphic Redux apps
Other
71 stars 8 forks source link

Refactor tests to Jest #3

Closed oziniak closed 6 years ago

oziniak commented 7 years ago

This PR https://github.com/NYTimes/redux-taxi/pull/2 introduces migration to prop-types package, introduced with React v15.5.0. New prop-types don't throw an error, but console.error (https://github.com/reactjs/prop-types/issues/28).

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

frytyler commented 6 years ago

@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 6 years ago

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 6 years ago

@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 6 years ago

@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 6 years ago

@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.