Skip to content
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

assert: handle TokenTooLong error scenario #1559

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
25 changes: 18 additions & 7 deletions assert/assertions.go
Expand Up @@ -300,14 +300,25 @@ func messageFromMsgAndArgs(msgAndArgs ...interface{}) string {
// basis on which the alignment occurs).
func indentMessageLines(message string, longestLabelLen int) string {
outBuf := new(bytes.Buffer)

for i, scanner := 0, bufio.NewScanner(strings.NewReader(message)); scanner.Scan(); i++ {
// no need to align first line because it starts at the correct location (after the label)
if i != 0 {
// append alignLen+1 spaces to align with "{{longestLabel}}:" before adding tab
outBuf.WriteString("\n\t" + strings.Repeat(" ", longestLabelLen+1) + "\t")
msgScanner := bufio.NewScanner(strings.NewReader(message))

// a buffer is set manually to store the tokenization of the message string
// so that we can re-use the same buffer for each line of the message without
// any additional allocations. We set the maximum buffer length to 1 more
// than the length of the message to avoid exceeding the default
// MaxScanTokenSize while scanning lines. This can happen when there is a
// single very long line. Refer to issue #1525
msgScanner.Buffer(nil, len(message)+1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this safe? As we know from the assert.Len case this can contain a very large formatted slice. Having more than doubled the memory consumed by the large slice by creating a string containing the formatted version of it. Do we want to double the formatted version again? Plus all the intermediate buffers that were allocated on the heap by msgScanner before the next GC cycle?

We could at least pre-allocate the buffer for msgScanner rather than passing in nil. But I'd prefer that we either split or truncate lines that are unreasonably long, or we could just truncate the entire message to less than bufio.MaxScanTokenSize?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could at least pre-allocate the buffer for msgScanner rather than passing in nil.

Agreed. I've made this change.

But I'd prefer that we either split or truncate lines that are unreasonably long, or we could just truncate the entire message to less than bufio.MaxScanTokenSize?

If we decide to truncate, we could probably truncate based on the dimensions of the terminal? This makes more sense from a UX point of view.


isFirstLine := true
indent := fmt.Sprintf("\n\t%*s\t", longestLabelLen+1, "")
for msgScanner.Scan() {
if !isFirstLine {
outBuf.WriteString(indent)
}
outBuf.WriteString(scanner.Text())

outBuf.Write(msgScanner.Bytes())
isFirstLine = false
}

return outBuf.String()
Expand Down
87 changes: 87 additions & 0 deletions assert/assertions_priv_test.go
@@ -0,0 +1,87 @@
package assert

import (
"bufio"
"bytes"
"strings"
"testing"
)

func Test_indentMessageLines(t *testing.T) {
tt := []struct {
name string
longestLabelLen int

// the input is constructed based on the below parameters
bytesPerLine int
lineCount int
}{
{
name: "single line - over the bufio default limit",
longestLabelLen: 11,
bytesPerLine: bufio.MaxScanTokenSize + 10,
lineCount: 1,
},
{
name: "multi line - over the bufio default limit",
longestLabelLen: 11,
bytesPerLine: bufio.MaxScanTokenSize + 10,
lineCount: 3,
},
{
name: "single line - just under the bufio default limit",
longestLabelLen: 11,
bytesPerLine: bufio.MaxScanTokenSize - 10,
lineCount: 1,
},
{
name: "single line - just under the bufio default limit",
longestLabelLen: 11,
bytesPerLine: bufio.MaxScanTokenSize - 10,
lineCount: 1,
},
{
name: "single line - equal to the bufio default limit",
longestLabelLen: 11,
bytesPerLine: bufio.MaxScanTokenSize,
lineCount: 1,
},
{
name: "multi line - equal to the bufio default limit",
longestLabelLen: 11,
bytesPerLine: bufio.MaxScanTokenSize,
lineCount: 3,
},
{
name: "longest label length is zero",
longestLabelLen: 0,
bytesPerLine: 10,
lineCount: 1,
},
}

for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
var input bytes.Buffer
for i := 0; i < tc.lineCount; i++ {
input.WriteString(strings.Repeat("#", tc.bytesPerLine))
input.WriteRune('\n')
}

output := indentMessageLines(
strings.TrimSpace(input.String()), tc.longestLabelLen,
)
outputLines := strings.Split(output, "\n")
for i, line := range outputLines {
if i > 0 {
// count the leading white spaces. It should be equal to the longest
// label length + 3. The +3 is to account for the 2 '\t' and 1 extra
// space. Read the comment in the function for more context
Equal(t, tc.longestLabelLen+3, strings.Index(line, "#"))
}

Len(t, strings.TrimSpace(line), tc.bytesPerLine)
}
})
}
}