-
Notifications
You must be signed in to change notification settings - Fork 196
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
Improve recorded test logging #1571
Conversation
Here's an example of an output with 2 failing tests:
|
e49cbb2
to
e0b3e4d
Compare
Codecov Report
@@ Coverage Diff @@
## master #1571 +/- ##
==========================================
+ Coverage 63.41% 63.57% +0.15%
==========================================
Files 181 182 +1
Lines 11797 11856 +59
==========================================
+ Hits 7481 7537 +56
- Misses 3648 3650 +2
- Partials 668 669 +1
Continue to review full report at Codecov.
|
9f7461b
to
49b18f2
Compare
- It makes the output more readable if it's off
d30ff68
to
c7364a1
Compare
kvListFormat(b, t.values...) | ||
kvListFormat(b, keysAndValues...) | ||
|
||
t.t.Log(fmt.Sprintf("%s \"msg\"=\"%s\" \"err\"=\"%s\"%s", header, msg, err, b)) |
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'd like us to consider doing something smart with error logging - like breaking out messages across multiple lines so that it's possible to read errors without having to copy them out elsewhere and manually reformat them. #somethingforanothertime
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.
Yeah my intent with this is just to mirror as close as possible the default klog logging format, so that looking at test logs and actual "prod" logs are the same. Unfortunately because of how klog
+ golang testing.T
are written you can't actually just plug testing.T
into klog
(I tried, it almost worked...)
Closes #1431
What this PR does / why we need it: