From f7fedd9f85b07f578782e2ca6b937677fd0ad171 Mon Sep 17 00:00:00 2001 From: Kevin Burke Date: Wed, 11 Oct 2023 11:50:43 -0700 Subject: [PATCH 1/3] assert: better formatting for Len() error Previously, the use of %s with array objects meant you would get an error like this: "[%!s(int=1) %!s(int=2) %!s(int=3)]\" should have 4 item(s), but has 3 Use %v instead, which provides a much nicer error. "[1 2 3]" should have 4 item(s), but has 3 Fixes #1482. --- assert/assertions.go | 4 ++-- assert/assertions_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index b3b309b8d..f8bee4d76 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -772,11 +772,11 @@ func Len(t TestingT, object interface{}, length int, msgAndArgs ...interface{}) } l, ok := getLen(object) if !ok { - return Fail(t, fmt.Sprintf("\"%s\" could not be applied builtin len()", object), msgAndArgs...) + return Fail(t, fmt.Sprintf("\"%v\" could not be applied builtin len()", object), msgAndArgs...) } if l != length { - return Fail(t, fmt.Sprintf("\"%s\" should have %d item(s), but has %d", object, length, l), msgAndArgs...) + return Fail(t, fmt.Sprintf("\"%v\" should have %d item(s), but has %d", object, length, l), msgAndArgs...) } return true } diff --git a/assert/assertions_test.go b/assert/assertions_test.go index a06d6e9e0..6d8afc401 100644 --- a/assert/assertions_test.go +++ b/assert/assertions_test.go @@ -1705,6 +1705,33 @@ func TestLen(t *testing.T) { for _, c := range cases { False(t, Len(mockT, c.v, c.l), "%#v have %d items", c.v, c.l) } + + formatCases := []struct { + in interface{} + wantLen int + want string + }{ + {[]int{1, 2, 3}, 3, "[1 2 3]"}, + {[...]int{1, 2, 3}, 3, "[1 2 3]"}, + {"ABC", 3, "ABC"}, + {map[int]int{1: 2, 2: 4, 3: 6}, 3, "map[1:2 2:4 3:6]"}, + + {[]int{}, 0, "[]"}, + {map[int]int{}, 0, "map[]"}, + + {[]int(nil), 0, "[]"}, + {map[int]int(nil), 0, "map[]"}, + {(chan int)(nil), 0, ""}, + } + + t.Run("Len() error message formatting", func(t *testing.T) { + for _, tt := range formatCases { + msgMock := new(mockTestingT) + Len(msgMock, tt.in, 1234567) + want := fmt.Sprintf(`"%s" should have 1234567 item(s), but has %d`, tt.want, tt.wantLen) + Contains(t, msgMock.errorString(), want) + } + }) } func TestWithinDuration(t *testing.T) { From f6ed021e60f6205c3ab904950cc2ea1cf51b47f8 Mon Sep 17 00:00:00 2001 From: Kevin Burke Date: Mon, 16 Oct 2023 09:57:04 -0700 Subject: [PATCH 2/3] assert: shorten cases --- assert/assertions_test.go | 66 +++++++-------------------------------- 1 file changed, 12 insertions(+), 54 deletions(-) diff --git a/assert/assertions_test.go b/assert/assertions_test.go index 6d8afc401..9806491ee 100644 --- a/assert/assertions_test.go +++ b/assert/assertions_test.go @@ -1661,77 +1661,35 @@ func TestLen(t *testing.T) { ch <- 3 cases := []struct { - v interface{} - l int - }{ - {[]int{1, 2, 3}, 3}, - {[...]int{1, 2, 3}, 3}, - {"ABC", 3}, - {map[int]int{1: 2, 2: 4, 3: 6}, 3}, - {ch, 3}, - - {[]int{}, 0}, - {map[int]int{}, 0}, - {make(chan int), 0}, - - {[]int(nil), 0}, - {map[int]int(nil), 0}, - {(chan int)(nil), 0}, - } - - for _, c := range cases { - True(t, Len(mockT, c.v, c.l), "%#v have %d items", c.v, c.l) - } - - cases = []struct { - v interface{} - l int - }{ - {[]int{1, 2, 3}, 4}, - {[...]int{1, 2, 3}, 2}, - {"ABC", 2}, - {map[int]int{1: 2, 2: 4, 3: 6}, 4}, - {ch, 2}, - - {[]int{}, 1}, - {map[int]int{}, 1}, - {make(chan int), 1}, - - {[]int(nil), 1}, - {map[int]int(nil), 1}, - {(chan int)(nil), 1}, - } - - for _, c := range cases { - False(t, Len(mockT, c.v, c.l), "%#v have %d items", c.v, c.l) - } - - formatCases := []struct { - in interface{} - wantLen int - want string + v interface{} + l int + format string }{ {[]int{1, 2, 3}, 3, "[1 2 3]"}, {[...]int{1, 2, 3}, 3, "[1 2 3]"}, {"ABC", 3, "ABC"}, {map[int]int{1: 2, 2: 4, 3: 6}, 3, "map[1:2 2:4 3:6]"}, + {ch, 3, ""}, {[]int{}, 0, "[]"}, {map[int]int{}, 0, "map[]"}, + {make(chan int), 0, ""}, {[]int(nil), 0, "[]"}, {map[int]int(nil), 0, "map[]"}, {(chan int)(nil), 0, ""}, } - t.Run("Len() error message formatting", func(t *testing.T) { - for _, tt := range formatCases { + for _, c := range cases { + True(t, Len(mockT, c.v, c.l), "%#v have %d items", c.v, c.l) + False(t, Len(mockT, c.v, c.l+1), "%#v have %d items", c.v, c.l) + if c.format != "" { msgMock := new(mockTestingT) - Len(msgMock, tt.in, 1234567) - want := fmt.Sprintf(`"%s" should have 1234567 item(s), but has %d`, tt.want, tt.wantLen) + Len(msgMock, c.v, 1234567) + want := fmt.Sprintf(`"%s" should have 1234567 item(s), but has %d`, c.format, c.l) Contains(t, msgMock.errorString(), want) } - }) + } } func TestWithinDuration(t *testing.T) { From 89c0872acdc8da8f27203804079975942b0de9a5 Mon Sep 17 00:00:00 2001 From: Kevin Burke Date: Tue, 24 Oct 2023 13:32:19 -0700 Subject: [PATCH 3/3] try to do the whole format string --- assert/assertions_test.go | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/assert/assertions_test.go b/assert/assertions_test.go index 9806491ee..ead2ff438 100644 --- a/assert/assertions_test.go +++ b/assert/assertions_test.go @@ -1661,33 +1661,32 @@ func TestLen(t *testing.T) { ch <- 3 cases := []struct { - v interface{} - l int - format string + v interface{} + l int + expected1234567 string // message when expecting 1234567 items }{ - {[]int{1, 2, 3}, 3, "[1 2 3]"}, - {[...]int{1, 2, 3}, 3, "[1 2 3]"}, - {"ABC", 3, "ABC"}, - {map[int]int{1: 2, 2: 4, 3: 6}, 3, "map[1:2 2:4 3:6]"}, + {[]int{1, 2, 3}, 3, `"[1 2 3]" should have 1234567 item(s), but has 3`}, + {[...]int{1, 2, 3}, 3, `"[1 2 3]" should have 1234567 item(s), but has 3`}, + {"ABC", 3, `"ABC" should have 1234567 item(s), but has 3`}, + {map[int]int{1: 2, 2: 4, 3: 6}, 3, `"map[1:2 2:4 3:6]" should have 1234567 item(s), but has 3`}, {ch, 3, ""}, - {[]int{}, 0, "[]"}, - {map[int]int{}, 0, "map[]"}, + {[]int{}, 0, `"[]" should have 1234567 item(s), but has 0`}, + {map[int]int{}, 0, `"map[]" should have 1234567 item(s), but has 0`}, {make(chan int), 0, ""}, - {[]int(nil), 0, "[]"}, - {map[int]int(nil), 0, "map[]"}, - {(chan int)(nil), 0, ""}, + {[]int(nil), 0, `"[]" should have 1234567 item(s), but has 0`}, + {map[int]int(nil), 0, `"map[]" should have 1234567 item(s), but has 0`}, + {(chan int)(nil), 0, `"" should have 1234567 item(s), but has 0`}, } for _, c := range cases { True(t, Len(mockT, c.v, c.l), "%#v have %d items", c.v, c.l) False(t, Len(mockT, c.v, c.l+1), "%#v have %d items", c.v, c.l) - if c.format != "" { + if c.expected1234567 != "" { msgMock := new(mockTestingT) Len(msgMock, c.v, 1234567) - want := fmt.Sprintf(`"%s" should have 1234567 item(s), but has %d`, c.format, c.l) - Contains(t, msgMock.errorString(), want) + Contains(t, msgMock.errorString(), c.expected1234567) } } }