Repository: stretchr/testify
Pull request #1362 (2023-03-15 17:50:43)

assert: fix error message formatting for NotContains
## Summary
The `assert.NotContains` error messages now uses `GoString()` for formatting instead of `Stringer()`.

## Changes

1. Renamed `TestContainsFailMessage` to `TestContainsNotContainsFailMessage` and refactored the test case to use a "test cases" structure.
2. Changed `\"%s\"` to `%#v` in `assert.NotContains`.
3. Added test cases covering both the "len()" / iterable failure messages as well as the Contains/NotContains failure message.

## Motivation
The existing `NotContains` error messages are hard to read.

WAS:

    "map[one:%!s(int=1)]" should not contain "one"

IS:

    map[string]int{"one":1} should not contain "one"


wwade: @mvdkleijn mind reviewing this one?
wwade: @boyan-soubachov mind having a look?
wwade: @glesica mind having a look at this?
wwade: @boyan-soubachov @glesica bump please.

Patch:
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


Test patch:
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)
+			}
+		})
 	}
 }