Print error context when test fails#2437
Conversation
We used to did print the context but after valkey-io#2276, we lost the context. unstable: ``` *** Extract version and sha1 details from info command and print in tests/unit/info-command.tcl ``` now: ``` *** [err]: Extract version and sha1 details from info command and print in tests/unit/info-command.tcl Expected '0' to be equal to '1' (context: type source line 7 file /xxx/info-command.tcl cmd {assert_equal 0 1} proc ::test) ``` We can see the different, we have provided enough context when asserting fail. Otherwise we need to scroll back (which is usually a lot of server logs) to see the context. Signed-off-by: Binbin <[email protected]>
| set err "\[[colorstr red $status]\]: $data" | ||
| puts $err | ||
| set test_name [lindex [split $data "\n"] 0] | ||
| lappend ::failed_tests $test_name |
There was a problem hiding this comment.
we can print the whole err actually, it have more info
There was a problem hiding this comment.
Oh so you are referring to the print at the end of the test? we do have a print when the test is reporting the error during the run right?
There was a problem hiding this comment.
yes, the line 476 print the log, it happen during the run.
and here, the lappend ::failed_tests, we will print it at the end of the test.
There was a problem hiding this comment.
ok LGTM, if err has the test file name and the context, then we can append it directly.
But I don't think we had context before as mentioned in #2267.
There was a problem hiding this comment.
only timeout test don't have the context and you made the change, the normal assert_xxx both have the context. Some other like may not have the context but we can fix it when we need it.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #2437 +/- ##
============================================
- Coverage 71.52% 71.38% -0.15%
============================================
Files 125 125
Lines 69213 69213
============================================
- Hits 49505 49406 -99
- Misses 19708 19807 +99 🚀 New features to boost your workflow:
|
ranshid
left a comment
ranshid
left a comment
There was a problem hiding this comment.
Basically LGTM. There will be error print duplication, but I guess no harm in that.
| set err "\[[colorstr red $status]\]: $data" | ||
| puts $err | ||
| set test_name [lindex [split $data "\n"] 0] | ||
| lappend ::failed_tests $test_name |
There was a problem hiding this comment.
Oh so you are referring to the print at the end of the test? we do have a print when the test is reporting the error during the run right?
We used to did print the context but after valkey-io#2276, we lost the context. unstable: ``` *** Extract version and sha1 details from info command and print in tests/unit/info-command.tcl ``` now: ``` *** [err]: Extract version and sha1 details from info command and print in tests/unit/info-command.tcl Expected '0' to be equal to '1' (context: type source line 7 file /xxx/info-command.tcl cmd {assert_equal 0 1} proc ::test) ``` We can see the different, we have provided enough context when asserting fail. Otherwise we need to scroll back (which is usually a lot of server logs) to see the context. Signed-off-by: Binbin <[email protected]>
3 participants
We used to did print the context but after #2276, we lost the context.
unstable:
now:
We can see the different, we have provided enough context when asserting fail. Otherwise we need to scroll back (which is usually a lot of server logs) to see the context.