From 437071b948cd89bdbaaf43a41f19fbe1a0945f6f Mon Sep 17 00:00:00 2001
From: wwade <2576056+wwade@users.noreply.github.com>
Date: Thu, 4 May 2023 21:05:51 -0700
Subject: [PATCH] assert: fix error message formatting for NotContains (#1362)
* 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: 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 | 59 ++++++++++++++++++++++++++++++++++-----
2 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/assert/assertions.go b/assert/assertions.go
index b362b4a..f7c3bf2 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 5eaf2af..999f8c1 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,59 @@ 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)
+ type nonContainer struct {
+ Value string
+ }
+
+ 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\"}",
+ },
+ {
+ 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, 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)
+ }
+ })
}
}