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

Display arguments Diff during panic #557

Merged
merged 1 commit into from Mar 18, 2018

Conversation

devdinu
Copy link
Contributor

@devdinu devdinu commented Feb 11, 2018

when expected method call wasn't found, but there's a closest method display the diff during panic.
especially this helps for argumentMatcher. closes #556

res := m.GetTime(1)
require.Equal(t, "SomeTime", res)
m.AssertExpectations(t)
}
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a test without MatchedBy to make sure in the future we avoid messing up the messages in those cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i've updated the commit with a test for that case.

@devdinu devdinu force-pushed the master branch 2 times, most recently from 584ee2b to 7538ed3 Compare February 21, 2018 06:09
@ernesto-jimenez
Copy link
Member

@devdinu there is a bug when selecting which diff to show. Here is an example test that shows the bug and the fix: aa1c55e

@ernesto-jimenez
Copy link
Member

@devdinu can you fixup the commit into a single one?

Thanks!

@devdinu devdinu reopened this Mar 11, 2018
@ernesto-jimenez ernesto-jimenez merged commit 921da25 into stretchr:master Mar 18, 2018
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.

Inaccurate MatchedBy error message
2 participants