assert/require.Len doesn't print anything if the slice is too long #1525
Comments
|
The easiest workaround for now seems to be: package tester_test
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestVeryLongSlice(t *testing.T) {
var bigSlice []string
for i := 0; i < 20000; i++ {
bigSlice = append(bigSlice, "hello")
}
expected := 10
require.Len(
t,
bigSlice,
expected,
"Should have %d item(s), but has %d",
expected,
len(bigSlice),
)
} |
|
We use a scanner to get the text which should go there line by line (it can be multiline) to indent each line until after the field name: Lines 304 to 313 in 7caada5 This mishandles the We need to think of a thing to do about very long lines in this code. We should also probably not produce such a long line from |
That sounds good to me. Personally I'd even be happy for the contents of the slice not to be printed at all, but I suppose this output can be useful for shorter slices when troubleshooting why a test is failing. Cheers |
|
Hello |
Thanks, yep I'm using this already, but I think my point was more around the inconsistency of the API and how that particular function feels odd in the context of the rest. Of course I appreciate that this would be a breaking change but just thought I'd bring it up for consideration in case a major new version of testify was being planned. Cheers |
|
The argument order is covered by #146. Let's keep this issue about the missing message. |
… messages As pointed out in stretchr#1525, when the assertion message is too long, it gets completely truncated in the final output. This is because, `bufio.Scanner.Scan()` has a default `MaxScanTokenSize` set to `65536` characters (64 * 1024). The `Scan()` function return false whenever the line being scanned exceeds that max limit. This leads to the final assertion message being truncated. This commit fixes that by manually setting the internal scan buffer size to `len(message) + 1` to make sure that above scenario never occurs. Fixes stretchr#1525
I've raised a PR that will make sure that the
Agreed. But regardless of the call we take about displaying long messages, I think this fix will be useful. |
|
I hit this recently and debugged it to the same root cause. For example the following test: func TestContainsLonglineRepro(t *testing.T) {
for _, numEntries := range []int{100, 10000} {
t.Run(fmt.Sprintf("numEntries=%d", numEntries), func(t *testing.T) {
m := map[string]string{}
for i := 0; i < numEntries; i++ {
m[fmt.Sprintf("key%d", i)] = fmt.Sprintf("value%d", i)
}
require.Contains(t, m, "not-here")
})
}
}Produces: I think the idea of truncating the error after a certain length (1k?, 4k?) seems reasonable, and avoids spamming a huge amount of unnecessary output while making sure that the error message is useful. Looking at the usage code it might be a bit awkward to do this though (https://github.com/stretchr/testify/blob/master/assert/assertions.go#L938). I guess everywhere messages are formatted would need to be modified with a call to a function that would format and then truncate. I'd be happy to put together a PR if there is agreement on the right approach. |
I totally agree with you |
|
One naive solution could be to check the length of the sprinted value, and truncate it after let's say an arbitrary value of 20-30 characters. Something with a trailing … plus tge number of items. If the slice is short and simple, such as a slice 2-3 int/string/bool, the message displayed will be the same as now. The error message will be meaningful. In other cases, where the message is currently useless, it would be better than nothing. Any thoughts guys? |
A very rough draft, but potentially something like: func Contains(t TestingT, s, contains interface{}, msgAndArgs ...interface{}) bool {
if h, ok := t.(tHelper); ok {
h.Helper()
}
ok, found := containsElement(s, contains)
if !ok {
return Fail(t, fmt.Sprintf("%#v could not be applied builtin len()", s), msgAndArgs...)
}
if !found {
return Fail(t, fmt.Sprintf("%#v does not contain %#v", s, contains), msgAndArgs...)
}
return true
}Replaced with func truncatingSprintf(format string, a ...any) string {
if !strings.HasPrefix(format, "%#v") {
panic("TruncatingSprintf format argument must begin with '%#v'")
}
result := fmt.Sprintf(format, a...)
if len(result) < 1024 {
return result
}
truncatedFormat := "object of type %T" + format[3:]
return fmt.Sprintf(truncatedFormat, len(result))
}
func Contains(t TestingT, s, contains interface{}, msgAndArgs ...interface{}) bool {
if h, ok := t.(tHelper); ok {
h.Helper()
}
ok, found := containsElement(s, contains)
if !ok {
return Fail(t, truncatingSprintf("%#v could not be applied builtin len()", s), msgAndArgs...)
}
if !found {
return Fail(t, truncatingSprintf("%#v does not contain %#v", s, contains), msgAndArgs...)
}
return true
} |
|
Except we were talking about "Len", and not "Contains", yes it's exactly what I'm thinking about Should I assume there is the same issue (dumping a slice) with Contains? |
Ah yes, I guess there is the same issue for any function which operates on containers (at least Len and Contains, but perhaps others?) |
|
1024 seems a very high value. It's OK for the example reported here, but having 1024 characters dumped on screen, which means 13 screens of text (assuming 80x25 terminal) maybe 1024 is fine, but then we could simply add an "\n" in the formatted string. This way the message Len expected actual would be visible |
|
Let's wait for other people feedbacks. But, yes anything like that would be better than current messages |
|
In other code I have written is was presented a little differently: Where They key parts being:
|
|
I'm unsure there will be an agreement or consensus on inverting the message and the truncated string. But even if not inverted, I'm OK with it as long we keep the truncated string short so <= 32 |
|
Instead of I'm simply using If I'm asserting the length of a slice, it's precisely because I don't care about the slice items. Otherwise, I would have used |
|
There actually is already a |
Hey there, I encountered this when attempting to use
require.Lenas shown below.Output of test:
I haven't quite figured out why this happens, although I can imagine the output would be impractically long anyway. Worst of all, the main assertion message is not displayed which is quite confusing (especially to someone new using the library, like me 😄).
Personally, I think that it would be best not to display the slice at all for the
Lenfunction, and instead just display:However, I suspect my view on that may not be what some people want. The reason I think it is unwise to print the slice in any form, is that typically when many people are testing for the length of a slice, they may be comparing to a relatively large slice of items (e.g. the output of a mocked API call or similar).
An additional point that I believe has been discussed before is the inconsistency with argument ordering for the
Lenfunction which takes theactualfirst andexpectedsecond.Cheers
Fotis
The text was updated successfully, but these errors were encountered: