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

Refactor tests to Jest #3

Closed
oziniak opened this issue Apr 18, 2017 · 5 comments · Fixed by #6
Closed

Refactor tests to Jest #3

oziniak opened this issue Apr 18, 2017 · 5 comments · Fixed by #6

Comments

@oziniak
Copy link
Contributor

oziniak commented Apr 18, 2017

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
Copy link
Contributor

frytyler commented Oct 2, 2017

@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
Copy link
Contributor

tizmagik commented Oct 2, 2017

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
Copy link
Contributor

frytyler commented Oct 2, 2017

@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
Copy link
Contributor

frytyler commented Oct 3, 2017

@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
Copy link
Contributor

tizmagik commented Oct 3, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants