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

✨ Filter bookmark commit out #54

Merged
merged 5 commits into from
Oct 29, 2018
Merged

✨ Filter bookmark commit out #54

merged 5 commits into from
Oct 29, 2018

Conversation

fabienjuif
Copy link
Collaborator

@fabienjuif fabienjuif commented Oct 27, 2018

fixes #40

expect(changes).toHaveLength(1)
expect(changes[0].groups).toHaveLength(1)
expect(changes[0].groups[0].commits).toHaveLength(1)
expect(changes[0].groups[0].commits[0]).toEqual(lipstickCommit)
Copy link
Owner

Choose a reason for hiding this comment

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

The last expect need the previous ones to be true so you can delete them, no ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah maybe.
When I wrote those I was more about: "if jest tells me lines where expect is false, I could tell where there is a problem in the code"

And I thank it was better than a JS error.

What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

I guess i would write 4 different test for this purpose. Ping @Taranys

Copy link
Contributor

Choose a reason for hiding this comment

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

Jest produces codeframes so you get a diff between what's expected and what you got.
Then you'll know exactly what is wrong in changes.

Copy link
Contributor

@charlyx charlyx Oct 29, 2018

Choose a reason for hiding this comment

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

expect(changes).toEqual([{
  groups: [{ commits: [lipstickCommit] }],
}])

You could also use expect.objectContaining and expect.arrayContaining if you wish to ignore some parts in changes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok I'll try that, thank you @charlyx

@bpetetot bpetetot merged commit 7858140 into master Oct 29, 2018
@bpetetot bpetetot deleted the sparkles/bookmark branch October 29, 2018 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove the bookmark emoji
4 participants