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.

Sign up for GitHub

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

Jump to bottom

Fix: Update test with .toMatchSnapshot to test UI #243

Merged
merged 2 commits into from
Oct 2, 2019
Merged

Fix: Update test with .toMatchSnapshot to test UI #243

merged 2 commits into from
Oct 2, 2019

Conversation

jaydinsweo
Copy link
Contributor

This PR is for issue #236 .
Replaced the .toEqual with the .toMatchSnapshot to test the UI.

@danielcaldas
Copy link
Owner

Hey, @nguyenami thanks a lot for the contribution! Just some feedback on your commit:

  • Small assertions such as expect(color).toEqual("yellow") do not benefit from .toMatchSnapshot() this jest functionality is rather more suitable for large object assertions.
  • In all the calls to .toMatchSnapshot you forgot to remove the arguments, this Jest method does not take any arguments.

If you want to get more into jest, I would suggest you take a closer look at the official documentation, if you want to get some quick tips I also suggest this blog post on unit testing with Jest

Cheers! And happy #Hacktoberfest

@danielcaldas danielcaldas added the Hacktoberfest Hacktoberfest label label Oct 2, 2019
@danielcaldas danielcaldas merged commit ba59513 into danielcaldas:master Oct 2, 2019
@danielcaldas danielcaldas mentioned this pull request Oct 2, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Hacktoberfest Hacktoberfest label
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jaydinsweo @danielcaldas