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(add tests): add example of using enzyme assertions and jest snapshots #18

Merged
merged 1 commit into from
Feb 6, 2019
Merged

fix(add tests): add example of using enzyme assertions and jest snapshots #18

merged 1 commit into from
Feb 6, 2019

Conversation

ibolton336
Copy link
Member

No description provided.

michaelkro
michaelkro reviewed Dec 5, 2018
View reviewed changes
michaelkro
michaelkro reviewed Dec 5, 2018
View reviewed changes
Copy link
Collaborator

@michaelkro michaelkro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

snapshots need to be updated :)

@michaelkro
Copy link
Collaborator

Actually, not sure about comment. Something weird is happening for me locally

michaelkro
michaelkro approved these changes Dec 6, 2018
View reviewed changes
Copy link
Collaborator

@michaelkro michaelkro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

cdcabrera
cdcabrera approved these changes Dec 7, 2018
View reviewed changes
Copy link
Member

@cdcabrera cdcabrera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

down the path we're on wouldn't mind thinking for the future about grouping the __mocks__, test-setup.js , and the jest config somehow, but may need to find other examples first

seanforyou23
seanforyou23 previously requested changes Dec 10, 2018
View reviewed changes
Copy link
Collaborator

@seanforyou23 seanforyou23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of minor questions/nits - other than that it looks good to me. Nice work @ibolton336!

@seanforyou23
Copy link
Collaborator

Can also remove this stub - https://github.com/ibolton336/patternfly-seed/blob/update-tests/src/App/sum.js

dmiller9911
dmiller9911 reviewed Dec 22, 2018
View reviewed changes
@ibolton336 ibolton336 force-pushed the update-tests branch 4 times, most recently from 4266b76 to 707e8e5 Compare February 5, 2019 23:23
seanforyou23
seanforyou23 reviewed Feb 6, 2019
View reviewed changes
Copy link
Collaborator

@seanforyou23 seanforyou23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I think the snapshots need updating, maybe just needs yarn test -u? Other than that it looks good to me.

seanforyou23
seanforyou23 approved these changes Feb 6, 2019
View reviewed changes
Copy link
Collaborator

@seanforyou23 seanforyou23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice work!

@seanforyou23 seanforyou23 merged commit 1f59956 into patternfly:master Feb 6, 2019
@ibolton336 ibolton336 deleted the update-tests branch February 6, 2019 20:29
bhanvimenghani pushed a commit to bhanvimenghani/patternfly-react-seed that referenced this pull request Mar 18, 2022
@ibolton336
fix(add tests): add example of using enzyme assertions and jest (patt…
c617dd8 
…ernfly#18)

snapshots
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmiller9911 dmiller9911 dmiller9911 left review comments

@cdcabrera cdcabrera cdcabrera approved these changes

@seanforyou23 seanforyou23 seanforyou23 approved these changes

@michaelkro michaelkro michaelkro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ibolton336 @michaelkro @seanforyou23 @cdcabrera @dmiller9911