From 671192438bc1376cae32cc0a8e8d409a54bd919d Mon Sep 17 00:00:00 2001 From: Wade Carpenter Date: Wed, 15 Mar 2023 10:14:14 -0700 Subject: [PATCH 1/2] assert: rename and refactor TestContainsFailMessage I've renamed it to TestContainsNotContains and added a test case structure so that I can use this test to validate the failure messages from both Contains and NotContains, There are no functional changes here, strictly refactoring. A new test case will be added in a subsequent change. --- assert/assertions_test.go | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/assert/assertions_test.go b/assert/assertions_test.go index 5eaf2af8d..7ce65df30 100644 --- a/assert/assertions_test.go +++ b/assert/assertions_test.go @@ -9,6 +9,7 @@ import ( "io" "math" "os" + "path/filepath" "reflect" "regexp" "runtime" @@ -688,15 +689,31 @@ func TestContainsNotContains(t *testing.T) { } } -func TestContainsFailMessage(t *testing.T) { - +func TestContainsNotContainsFailMessage(t *testing.T) { mockT := new(mockTestingT) - Contains(mockT, "Hello World", errors.New("Hello")) - expectedFail := "\"Hello World\" does not contain &errors.errorString{s:\"Hello\"}" - actualFail := mockT.errorString() - if !strings.Contains(actualFail, expectedFail) { - t.Errorf("Contains failure should include %q but was %q", expectedFail, actualFail) + cases := []struct { + assertion func(t TestingT, s, contains interface{}, msgAndArgs ...interface{}) bool + container interface{} + instance interface{} + expected string + }{ + { + assertion: Contains, + container: "Hello World", + instance: errors.New("Hello"), + expected: "\"Hello World\" does not contain &errors.errorString{s:\"Hello\"}", + }, + } + for _, c := range cases { + name := filepath.Base(runtime.FuncForPC(reflect.ValueOf(c.assertion).Pointer()).Name()) + t.Run(fmt.Sprintf("%v(%T, %T)", name, c.container, c.instance), func(t *testing.T) { + c.assertion(mockT, "Hello World", errors.New("Hello")) + actualFail := mockT.errorString() + if !strings.Contains(actualFail, c.expected) { + t.Errorf("Contains failure should include %q but was %q", c.expected, actualFail) + } + }) } } From 0c8fb8d1dc300984dcdd48190ab03afcc553f8bb Mon Sep 17 00:00:00 2001 From: Wade Carpenter Date: Wed, 15 Mar 2023 10:35:59 -0700 Subject: [PATCH 2/2] assert: fix error message formatting for NotContains It was using "%s" to format the s and contains arguments, but these could be any type that supports iteration such as a map. In this case of NotContains failing against a map, it would print something like: "map[one:%!s(int=1)]" should not contain "one" Fix this using "%#v" as was already done for Contains and added test cases covering both the "len()" / iterable failure messages as well as the Contains/NotContains failure case. The new message for this example map would be something like: map[string]int{"one":1} should not contain "one" --- assert/assertions.go | 4 ++-- assert/assertions_test.go | 30 +++++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/assert/assertions.go b/assert/assertions.go index b362b4a29..f7c3bf28e 100644 --- a/assert/assertions.go +++ b/assert/assertions.go @@ -877,10 +877,10 @@ func NotContains(t TestingT, s, contains interface{}, msgAndArgs ...interface{}) ok, found := containsElement(s, contains) if !ok { - return Fail(t, fmt.Sprintf("\"%s\" could not be applied builtin len()", s), msgAndArgs...) + return Fail(t, fmt.Sprintf("%#v could not be applied builtin len()", s), msgAndArgs...) } if found { - return Fail(t, fmt.Sprintf("\"%s\" should not contain \"%s\"", s, contains), msgAndArgs...) + return Fail(t, fmt.Sprintf("%#v should not contain %#v", s, contains), msgAndArgs...) } return true diff --git a/assert/assertions_test.go b/assert/assertions_test.go index 7ce65df30..999f8c14a 100644 --- a/assert/assertions_test.go +++ b/assert/assertions_test.go @@ -692,6 +692,10 @@ func TestContainsNotContains(t *testing.T) { func TestContainsNotContainsFailMessage(t *testing.T) { mockT := new(mockTestingT) + type nonContainer struct { + Value string + } + cases := []struct { assertion func(t TestingT, s, contains interface{}, msgAndArgs ...interface{}) bool container interface{} @@ -704,11 +708,35 @@ func TestContainsNotContainsFailMessage(t *testing.T) { instance: errors.New("Hello"), expected: "\"Hello World\" does not contain &errors.errorString{s:\"Hello\"}", }, + { + assertion: Contains, + container: map[string]int{"one": 1}, + instance: "two", + expected: "map[string]int{\"one\":1} does not contain \"two\"\n", + }, + { + assertion: NotContains, + container: map[string]int{"one": 1}, + instance: "one", + expected: "map[string]int{\"one\":1} should not contain \"one\"", + }, + { + assertion: Contains, + container: nonContainer{Value: "Hello"}, + instance: "Hello", + expected: "assert.nonContainer{Value:\"Hello\"} could not be applied builtin len()\n", + }, + { + assertion: NotContains, + container: nonContainer{Value: "Hello"}, + instance: "Hello", + expected: "assert.nonContainer{Value:\"Hello\"} could not be applied builtin len()\n", + }, } for _, c := range cases { name := filepath.Base(runtime.FuncForPC(reflect.ValueOf(c.assertion).Pointer()).Name()) t.Run(fmt.Sprintf("%v(%T, %T)", name, c.container, c.instance), func(t *testing.T) { - c.assertion(mockT, "Hello World", errors.New("Hello")) + c.assertion(mockT, c.container, c.instance) actualFail := mockT.errorString() if !strings.Contains(actualFail, c.expected) { t.Errorf("Contains failure should include %q but was %q", c.expected, actualFail)