-
Notifications
You must be signed in to change notification settings - Fork 598
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
many: add logger.MockLogger() and use it in the tests #3911
Conversation
We have some noise in the unittest runs right now because messages get logged to stdout which really should not be visible. With the new logger.MockLogger() they are now no longer visible.
Codecov Report
@@ Coverage Diff @@
## master #3911 +/- ##
==========================================
+ Coverage 75.9% 75.91% +0.01%
==========================================
Files 416 416
Lines 35986 35994 +8
==========================================
+ Hits 27316 27326 +10
+ Misses 6752 6751 -1
+ Partials 1918 1917 -1
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.
looks reasonable
logger/logger.go
Outdated
} | ||
SetLogger(l) | ||
return buf, func() { | ||
logger = oldLogger |
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.
this is not needed
logger/logger.go
Outdated
// MockLogger replaces the exiting logger with a buffer and returns | ||
// the log buffer and a restore function. | ||
func MockLogger() (buf *bytes.Buffer, restore func()) { | ||
buf = bytes.NewBuffer(nil) |
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.
this is just: &bytes.Buffer{}
We have two noisy tests in the unittest runs right now. Messages
get logged to stdout which really should not be visible. With the use
of the new logger.MockLogger() those are now no longer visible.
I also tweaked the existing pattern of:
to just use:
Please let me know what you think, if you think this simple setup should not have a MockLogger() helper I'm happy to adjust the PR to just switch to a different logger in the two noisy tests we currently have.