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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add testing.TB to mock.Mock in constructors #455
Add testing.TB to mock.Mock in constructors #455
Conversation
Codecov Report
@@ Coverage Diff @@
## master #455 +/- ##
==========================================
- Coverage 70.76% 70.73% -0.04%
==========================================
Files 7 7
Lines 1293 1295 +2
==========================================
+ Hits 915 916 +1
- Misses 325 326 +1
Partials 53 53
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know about this functionality before...nice one. LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this technically wouldn't be a breaking change so long as some test in a suite would fail. I like this change, looks good. Thatnks!
README.md
Outdated
func NewSendFunc(t testing.TB) *SendFunc { | ||
mock := &SendFunc{} | ||
mock.Mock.Test(t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting issue here (tab vs space), could you correct the line before this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed!
Description
Following the recent addition of constructors for mocks by @grongor in #406, this PR is a suggestion to also register the
testing.TB
interface onmock.Mock
.This is a feature in testify's mock.Mock that makes it so that whenever a mock is not matched, the whole suite does not panic, but instead makes the test fails. It is in some way breaking in the sense that the behaviour that some might be used to (ie. stop the world panic) will not be the same, but while we added a cleanup function I think this is desirable.
This is of course debatable 馃槂
Type of change
Version of Golang used when building/testing:
How Has This Been Tested?
Added generation to related files and tested locally with
go test ./...
Checklist