Skip to content

Commit

Permalink
assert: fix error message formatting for NotContains (#1362)
Browse files Browse the repository at this point in the history
* 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"
  • Loading branch information
wwade authored May 5, 2023
1 parent c5fc9d6 commit 437071b
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 9 deletions.
4 changes: 2 additions & 2 deletions assert/assertions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
59 changes: 52 additions & 7 deletions assert/assertions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io"
"math"
"os"
"path/filepath"
"reflect"
"regexp"
"runtime"
Expand Down Expand Up @@ -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)
}
})
}
}

Expand Down

0 comments on commit 437071b

Please sign in to comment.