assert: truncate very long objects in test failure messages #1646
Conversation
brackendawson
force-pushed
the
token-too-long
branch
from
ff41e65 to
2f8b16d
Compare
dolmen
added
pkg-assert
dolmen
changed the title
brackendawson
force-pushed
the
token-too-long
branch
from
2f8b16d to
1eb5fc0
Compare
brackendawson
removed
must-rebase
enhancement: output format
| return Fail(t, fmt.Sprintf("%s does not contain %s", truncatingFormat("%#v", list), truncatingFormat("%#v", subset)), msgAndArgs...) | ||
| } | ||
| if !ObjectsAreEqual(ev.Interface(), av.Interface()) { | ||
| return Fail(t, fmt.Sprintf("%#v does not contain %#v", list, subset), msgAndArgs...) | ||
| return Fail(t, fmt.Sprintf("%s does not contain %s", truncatingFormat("%#v", list), truncatingFormat("%#v", subset)), msgAndArgs...) |
There was a problem hiding this comment.
I'm a bit worried about these 2
We have 2 list that could be huge that would be truncated here, but displayed on the same line.
Maybe it works because of the max/2-200, but I feel like the line might difficult to read because of the lack of \n that we have for almost all others failures
There was a problem hiding this comment.
I agree about the hard to read part, but that would be a different issue. Again the only aim here is to show strings which previously got swallowed.
There was a problem hiding this comment.
I understand your point. But mine wasn't about readability.
I just wanted to make sure that comparing two slices that very long won't lead to a line that cannot be displayed.
I feel like that tge point of the max/2-200 but I wanted to make sure it would work.
There was a problem hiding this comment.
Yes, that doesn't sit 100% with me either. When we follow this up with #1235 and reduce the max length much further I will be very confident.
|
Thank you @ccoVeille. I've updated the PR to address some comments and responded with a defence in the others. |
Summary
Truncate object representations in test failure output messages that would cause the message to be too long to scan.
This is a different approach than that in #1559. I feel this is more appropriate because is it safer. The other approach has the potential to allocate excessive memory and to print excessive output to the console.
Changes
truncatingFormatfunction to truncate potentially long objects in assertions which format expected/actual values in full.truncatingFormat.truncatingFormatby almost half to support failure messages showing both long expected and long actual values on the same line.NotSubsetfrom%qto%#vto match that ofSubset.Motivation
So that people don't see no output in the failure message when making assertions (such as
Len) against very long objects.Related issues
Closes #1525