From 7f5f3e39f72911657ec7dce70ee5254ac0820c2a Mon Sep 17 00:00:00 2001 From: Arjun Mahishi Date: Sun, 3 Mar 2024 16:31:28 +0530 Subject: [PATCH 1/9] assert: handle TokenTooLong error scenario while formatting assertion messages As pointed out in #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 #1525 --- assert/assertions.go | 14 ++++++++----- assert/assertions_test.go | 42 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index 0b7570f21..5fbc07988 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -301,13 +301,17 @@ func messageFromMsgAndArgs(msgAndArgs ...interface{}) string { 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 + msgScanner := bufio.NewScanner(strings.NewReader(message)) + msgScanner.Buffer([]byte{}, len(message)+1) + + first := true + for msgScanner.Scan() { + if !first { outBuf.WriteString("\n\t" + strings.Repeat(" ", longestLabelLen+1) + "\t") } - outBuf.WriteString(scanner.Text()) + + outBuf.WriteString(msgScanner.Text()) + first = false } return outBuf.String() diff --git a/assert/assertions_test.go b/assert/assertions_test.go index 2a6e47234..04be59b4c 100644 --- a/assert/assertions_test.go +++ b/assert/assertions_test.go @@ -3192,3 +3192,45 @@ func TestErrorAs(t *testing.T) { }) } } + +func Test_indentMessageLines(t *testing.T) { + tt := []struct { + name string + msg string + longestLabelLen int + expected string + }{ + { + name: "single line", + msg: "Hello World\n", + longestLabelLen: 11, + expected: "Hello World", + }, + { + name: "multi line", + msg: "Hello\nWorld\n", + longestLabelLen: 11, + expected: "Hello\n\t \tWorld", + }, + { + name: "single line - extreamly long", + msg: strings.Repeat("hello ", 20000), + longestLabelLen: 11, + expected: strings.Repeat("hello ", 20000), + }, + { + name: "multi line - extreamly long", + msg: strings.Repeat("hello\n", 20000), + longestLabelLen: 3, + expected: strings.TrimSpace( + strings.TrimPrefix(strings.Repeat("\thello\n\t ", 20000), "\t"), + ), + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + Equal(t, tc.expected, indentMessageLines(tc.msg, tc.longestLabelLen)) + }) + } +} From f37ae55f7e0fec9338d6e92bb37d6849ce6d3e77 Mon Sep 17 00:00:00 2001 From: Arjun Mahishi Date: Sun, 3 Mar 2024 16:51:45 +0530 Subject: [PATCH 2/9] assert: add comment that explains the MaxScanTokenSize handling logic --- assert/assertions.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/assert/assertions.go b/assert/assertions.go index 5fbc07988..64290d972 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -302,6 +302,12 @@ func indentMessageLines(message string, longestLabelLen int) string { outBuf := new(bytes.Buffer) 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 buffer length to 1 more than the + // length of the message to avoid exceeding the default MaxScanTokenSize + // while scanning lines. This CAN happen. Refer to issue #1525 msgScanner.Buffer([]byte{}, len(message)+1) first := true From 48a148ae2f1c5d37565852f5a10c6ff882c6fe61 Mon Sep 17 00:00:00 2001 From: Arjun Mahishi Date: Mon, 4 Mar 2024 10:58:17 +0530 Subject: [PATCH 3/9] assert: Fix typos in Test_indentMessageLines --- assert/assertions_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/assert/assertions_test.go b/assert/assertions_test.go index 04be59b4c..dd7e38741 100644 --- a/assert/assertions_test.go +++ b/assert/assertions_test.go @@ -3213,13 +3213,13 @@ func Test_indentMessageLines(t *testing.T) { expected: "Hello\n\t \tWorld", }, { - name: "single line - extreamly long", + name: "single line - extremely long", msg: strings.Repeat("hello ", 20000), longestLabelLen: 11, expected: strings.Repeat("hello ", 20000), }, { - name: "multi line - extreamly long", + name: "multi line - extremely long", msg: strings.Repeat("hello\n", 20000), longestLabelLen: 3, expected: strings.TrimSpace( From f24d768c5afcffd5cac025767a987f9705519a08 Mon Sep 17 00:00:00 2001 From: Arjun Mahishi Date: Mon, 4 Mar 2024 23:33:13 +0530 Subject: [PATCH 4/9] assert: move Test_indentMessageLines into a separate file --- assert/assertions.go | 2 +- assert/assertions_priv_test.go | 48 ++++++++++++++++++++++++++++++++++ assert/assertions_test.go | 42 ----------------------------- 3 files changed, 49 insertions(+), 43 deletions(-) create mode 100644 assert/assertions_priv_test.go diff --git a/assert/assertions.go b/assert/assertions.go index 64290d972..dd89bcd5e 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -308,7 +308,7 @@ func indentMessageLines(message string, longestLabelLen int) string { // any additional allocations. We set the buffer length to 1 more than the // length of the message to avoid exceeding the default MaxScanTokenSize // while scanning lines. This CAN happen. Refer to issue #1525 - msgScanner.Buffer([]byte{}, len(message)+1) + msgScanner.Buffer(nil, len(message)+1) first := true for msgScanner.Scan() { diff --git a/assert/assertions_priv_test.go b/assert/assertions_priv_test.go new file mode 100644 index 000000000..be144536f --- /dev/null +++ b/assert/assertions_priv_test.go @@ -0,0 +1,48 @@ +package assert + +import ( + "strings" + "testing" +) + +func Test_indentMessageLines(t *testing.T) { + tt := []struct { + name string + msg string + longestLabelLen int + expected string + }{ + { + name: "single line", + msg: "Hello World\n", + longestLabelLen: 11, + expected: "Hello World", + }, + { + name: "multi line", + msg: "Hello\nWorld\n", + longestLabelLen: 11, + expected: "Hello\n\t \tWorld", + }, + { + name: "single line - extremely long", + msg: strings.Repeat("hello ", 20000), + longestLabelLen: 11, + expected: strings.Repeat("hello ", 20000), + }, + { + name: "multi line - extremely long", + msg: strings.Repeat("hello\n", 20000), + longestLabelLen: 3, + expected: strings.TrimSpace( + strings.TrimPrefix(strings.Repeat("\thello\n\t ", 20000), "\t"), + ), + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + Equal(t, tc.expected, indentMessageLines(tc.msg, tc.longestLabelLen)) + }) + } +} diff --git a/assert/assertions_test.go b/assert/assertions_test.go index dd7e38741..2a6e47234 100644 --- a/assert/assertions_test.go +++ b/assert/assertions_test.go @@ -3192,45 +3192,3 @@ func TestErrorAs(t *testing.T) { }) } } - -func Test_indentMessageLines(t *testing.T) { - tt := []struct { - name string - msg string - longestLabelLen int - expected string - }{ - { - name: "single line", - msg: "Hello World\n", - longestLabelLen: 11, - expected: "Hello World", - }, - { - name: "multi line", - msg: "Hello\nWorld\n", - longestLabelLen: 11, - expected: "Hello\n\t \tWorld", - }, - { - name: "single line - extremely long", - msg: strings.Repeat("hello ", 20000), - longestLabelLen: 11, - expected: strings.Repeat("hello ", 20000), - }, - { - name: "multi line - extremely long", - msg: strings.Repeat("hello\n", 20000), - longestLabelLen: 3, - expected: strings.TrimSpace( - strings.TrimPrefix(strings.Repeat("\thello\n\t ", 20000), "\t"), - ), - }, - } - - for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - Equal(t, tc.expected, indentMessageLines(tc.msg, tc.longestLabelLen)) - }) - } -} From 6b0dfad66fdeddb089d559b2ceb51f771bab19f1 Mon Sep 17 00:00:00 2001 From: Arjun Mahishi Date: Wed, 6 Mar 2024 09:45:49 +0530 Subject: [PATCH 5/9] assert: make Test_indentMessageLines more deterministic Instead of using an arbitrary value like 20000, we can just use the value defined by bufio package (MaxScanTokenSize). --- assert/assertions.go | 7 ++++--- assert/assertions_priv_test.go | 16 ++++++++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index dd89bcd5e..d7388ae14 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -305,9 +305,10 @@ func indentMessageLines(message string, longestLabelLen int) string { // 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 buffer length to 1 more than the - // length of the message to avoid exceeding the default MaxScanTokenSize - // while scanning lines. This CAN happen. Refer to issue #1525 + // 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) first := true diff --git a/assert/assertions_priv_test.go b/assert/assertions_priv_test.go index be144536f..92288f421 100644 --- a/assert/assertions_priv_test.go +++ b/assert/assertions_priv_test.go @@ -1,10 +1,18 @@ package assert import ( + "bufio" "strings" "testing" ) +const ( + // set maxScanTokenSize to 1 more than the default set by bufio. This will + // cover the case where a single line is longer than the default + // maxScanTokenSize + maxScanTokenSize = bufio.MaxScanTokenSize + 1 +) + func Test_indentMessageLines(t *testing.T) { tt := []struct { name string @@ -26,16 +34,16 @@ func Test_indentMessageLines(t *testing.T) { }, { name: "single line - extremely long", - msg: strings.Repeat("hello ", 20000), + msg: strings.Repeat("hello ", maxScanTokenSize), longestLabelLen: 11, - expected: strings.Repeat("hello ", 20000), + expected: strings.Repeat("hello ", maxScanTokenSize), }, { name: "multi line - extremely long", - msg: strings.Repeat("hello\n", 20000), + msg: strings.Repeat("hello\n", maxScanTokenSize), longestLabelLen: 3, expected: strings.TrimSpace( - strings.TrimPrefix(strings.Repeat("\thello\n\t ", 20000), "\t"), + strings.TrimPrefix(strings.Repeat("\thello\n\t ", maxScanTokenSize), "\t"), ), }, } From 594ac7dd9afbb3a849b1c6d777a315925d891d91 Mon Sep 17 00:00:00 2001 From: Arjun Mahishi Date: Thu, 7 Mar 2024 20:06:29 +0530 Subject: [PATCH 6/9] assert: indentMessageLines - build message with bytes instead of string --- assert/assertions.go | 2 +- assert/assertions_priv_test.go | 9 ++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index d7388ae14..a3a67c7a0 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -317,7 +317,7 @@ func indentMessageLines(message string, longestLabelLen int) string { outBuf.WriteString("\n\t" + strings.Repeat(" ", longestLabelLen+1) + "\t") } - outBuf.WriteString(msgScanner.Text()) + outBuf.Write(msgScanner.Bytes()) first = false } diff --git a/assert/assertions_priv_test.go b/assert/assertions_priv_test.go index 92288f421..7d5adaf2c 100644 --- a/assert/assertions_priv_test.go +++ b/assert/assertions_priv_test.go @@ -6,14 +6,9 @@ import ( "testing" ) -const ( - // set maxScanTokenSize to 1 more than the default set by bufio. This will - // cover the case where a single line is longer than the default - // maxScanTokenSize - maxScanTokenSize = bufio.MaxScanTokenSize + 1 -) - func Test_indentMessageLines(t *testing.T) { + const maxScanTokenSize = bufio.MaxScanTokenSize + 1 + tt := []struct { name string msg string From f341282a712dee4ef85965604bf1f763427f7e8e Mon Sep 17 00:00:00 2001 From: Arjun Mahishi Date: Mon, 11 Mar 2024 19:13:58 +0530 Subject: [PATCH 7/9] assert: add more test cases to Test_indentMessageLines --- assert/assertions_priv_test.go | 42 +++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/assert/assertions_priv_test.go b/assert/assertions_priv_test.go index 7d5adaf2c..32f9f12c8 100644 --- a/assert/assertions_priv_test.go +++ b/assert/assertions_priv_test.go @@ -7,8 +7,6 @@ import ( ) func Test_indentMessageLines(t *testing.T) { - const maxScanTokenSize = bufio.MaxScanTokenSize + 1 - tt := []struct { name string msg string @@ -28,17 +26,45 @@ func Test_indentMessageLines(t *testing.T) { expected: "Hello\n\t \tWorld", }, { - name: "single line - extremely long", - msg: strings.Repeat("hello ", maxScanTokenSize), + name: "single line - over the bufio default limit", + msg: strings.Repeat("hello ", bufio.MaxScanTokenSize+10), + longestLabelLen: 11, + expected: strings.Repeat("hello ", bufio.MaxScanTokenSize+10), + }, + { + name: "multi line - over the bufio default limit", + msg: strings.Repeat("hello\n", bufio.MaxScanTokenSize+10), + longestLabelLen: 3, + expected: strings.TrimSpace( + strings.TrimPrefix(strings.Repeat("\thello\n\t ", bufio.MaxScanTokenSize+10), "\t"), + ), + }, + { + name: "single line - just under the bufio default limit", + msg: strings.Repeat("hello ", bufio.MaxScanTokenSize-10), + longestLabelLen: 11, + expected: strings.Repeat("hello ", bufio.MaxScanTokenSize-10), + }, + { + name: "multi line - just under the bufio default limit", + msg: strings.Repeat("hello\n", bufio.MaxScanTokenSize-10), + longestLabelLen: 3, + expected: strings.TrimSpace( + strings.TrimPrefix(strings.Repeat("\thello\n\t ", bufio.MaxScanTokenSize-10), "\t"), + ), + }, + { + name: "single line - equal to the bufio default limit", + msg: strings.Repeat("hello ", bufio.MaxScanTokenSize), longestLabelLen: 11, - expected: strings.Repeat("hello ", maxScanTokenSize), + expected: strings.Repeat("hello ", bufio.MaxScanTokenSize), }, { - name: "multi line - extremely long", - msg: strings.Repeat("hello\n", maxScanTokenSize), + name: "multi line - equal to the bufio default limit", + msg: strings.Repeat("hello\n", bufio.MaxScanTokenSize), longestLabelLen: 3, expected: strings.TrimSpace( - strings.TrimPrefix(strings.Repeat("\thello\n\t ", maxScanTokenSize), "\t"), + strings.TrimPrefix(strings.Repeat("\thello\n\t ", bufio.MaxScanTokenSize), "\t"), ), }, } From b83d206a622d940ae4e8c7675da564d9d19a93aa Mon Sep 17 00:00:00 2001 From: Arjun Mahishi Date: Thu, 21 Mar 2024 12:41:08 +0530 Subject: [PATCH 8/9] assert: pre-compute the indent spaces in indentMessageLines Also, fix the test cases for this function. This commit generates the input based on parameters like bytes per line and number of lines. The assertion is made against the pattern of the output rather than the exact output. --- assert/assertions.go | 10 ++-- assert/assertions_priv_test.go | 84 +++++++++++++++++++--------------- 2 files changed, 52 insertions(+), 42 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index a3a67c7a0..00ef8b283 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -300,7 +300,6 @@ func messageFromMsgAndArgs(msgAndArgs ...interface{}) string { // basis on which the alignment occurs). func indentMessageLines(message string, longestLabelLen int) string { outBuf := new(bytes.Buffer) - msgScanner := bufio.NewScanner(strings.NewReader(message)) // a buffer is set manually to store the tokenization of the message string @@ -311,14 +310,15 @@ func indentMessageLines(message string, longestLabelLen int) string { // single very long line. Refer to issue #1525 msgScanner.Buffer(nil, len(message)+1) - first := true + isFirstLine := true + indent := fmt.Sprintf("\n\t%*s\t", longestLabelLen+1, "") for msgScanner.Scan() { - if !first { - outBuf.WriteString("\n\t" + strings.Repeat(" ", longestLabelLen+1) + "\t") + if !isFirstLine { + outBuf.WriteString(indent) } outBuf.Write(msgScanner.Bytes()) - first = false + isFirstLine = false } return outBuf.String() diff --git a/assert/assertions_priv_test.go b/assert/assertions_priv_test.go index 32f9f12c8..82beb7922 100644 --- a/assert/assertions_priv_test.go +++ b/assert/assertions_priv_test.go @@ -2,6 +2,7 @@ package assert import ( "bufio" + "bytes" "strings" "testing" ) @@ -9,69 +10,78 @@ import ( func Test_indentMessageLines(t *testing.T) { tt := []struct { name string - msg string longestLabelLen int - expected string + + // the input is constructed based on the below parameters + bytesPerLine int + lineCount int }{ - { - name: "single line", - msg: "Hello World\n", - longestLabelLen: 11, - expected: "Hello World", - }, - { - name: "multi line", - msg: "Hello\nWorld\n", - longestLabelLen: 11, - expected: "Hello\n\t \tWorld", - }, { name: "single line - over the bufio default limit", - msg: strings.Repeat("hello ", bufio.MaxScanTokenSize+10), longestLabelLen: 11, - expected: strings.Repeat("hello ", bufio.MaxScanTokenSize+10), + bytesPerLine: bufio.MaxScanTokenSize + 10, + lineCount: 1, }, { name: "multi line - over the bufio default limit", - msg: strings.Repeat("hello\n", bufio.MaxScanTokenSize+10), - longestLabelLen: 3, - expected: strings.TrimSpace( - strings.TrimPrefix(strings.Repeat("\thello\n\t ", bufio.MaxScanTokenSize+10), "\t"), - ), + longestLabelLen: 11, + bytesPerLine: bufio.MaxScanTokenSize + 10, + lineCount: 3, }, { name: "single line - just under the bufio default limit", - msg: strings.Repeat("hello ", bufio.MaxScanTokenSize-10), longestLabelLen: 11, - expected: strings.Repeat("hello ", bufio.MaxScanTokenSize-10), + bytesPerLine: bufio.MaxScanTokenSize - 10, + lineCount: 1, }, { - name: "multi line - just under the bufio default limit", - msg: strings.Repeat("hello\n", bufio.MaxScanTokenSize-10), - longestLabelLen: 3, - expected: strings.TrimSpace( - strings.TrimPrefix(strings.Repeat("\thello\n\t ", bufio.MaxScanTokenSize-10), "\t"), - ), + 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", - msg: strings.Repeat("hello ", bufio.MaxScanTokenSize), longestLabelLen: 11, - expected: strings.Repeat("hello ", bufio.MaxScanTokenSize), + bytesPerLine: bufio.MaxScanTokenSize, + lineCount: 1, }, { name: "multi line - equal to the bufio default limit", - msg: strings.Repeat("hello\n", bufio.MaxScanTokenSize), - longestLabelLen: 3, - expected: strings.TrimSpace( - strings.TrimPrefix(strings.Repeat("\thello\n\t ", bufio.MaxScanTokenSize), "\t"), - ), + 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) { - Equal(t, tc.expected, indentMessageLines(tc.msg, tc.longestLabelLen)) + 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) + } }) } } From 9e3196424f4479c3569e73a7d22a9998b07e9d23 Mon Sep 17 00:00:00 2001 From: Arjun Mahishi Date: Thu, 4 Apr 2024 14:18:28 +0530 Subject: [PATCH 9/9] assert: reduce memory allocations in indentMessageLines --- assert/assertions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assert/assertions.go b/assert/assertions.go index 00ef8b283..ca7c8f06e 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -308,7 +308,7 @@ func indentMessageLines(message string, longestLabelLen int) string { // 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) + msgScanner.Buffer(make([]byte, len(message)+1), len(message)+1) isFirstLine := true indent := fmt.Sprintf("\n\t%*s\t", longestLabelLen+1, "")