Repository: stretchr/testify
Pull request #895 (2020-02-21 04:08:56)

Disable using Stringer/error interfaces for diffing
*NOTE*: this changes something that is the default behavior. I intentionally am doing this as a proposal to change that behavior because I think the new one is better. If this is a problem, we could also achieve the goal of this PR by making `spewConfig` somewhat accessible so that the user of the library can choose to `DisableMethods` themselves.

`spew`'s default formatter will call `String()` or `Error()` in structs
that implement the `fmt.Stringer` or `error` interfaces. Depending on
the implementation of those, the diff can become quite useless to read
(see the example struct I used for the test case in this commit).

This changes `spew`'s configuration to `DisableMethods` so that it will
always use it's own pretty printer. This makes testing structs less
surprising and generally more useful, without tying the tests to the
implementation of `String()` (the user here can always choose to
`require.Equal(a.String(), b.String())` if testing those is important to
them.

boyan-soubachov: I think this change makes sense but am I correct in thinking that we're now delegating our own test output format to `spew`'s output format? Would this be wise?

Given that the result of our test assertions are definitely a key feature/purpose of this framework, would it make more sense to rather update our formatting output to match `spew's or, at the least, version-lock our spew dependency?
luan: @boyan-soubachov we were always delegating it to `spew` to be fair. This change merely makes us _always_ delegate as opposed to only when the object being tested doesn't implement `fmt.Stringer`.

I think maintaining a formatter as part of `testify` would work. But given that we've been using `spew` for this for a while it feels like a much larger step.

I do agree with you that the failure messages that `testify` generates are a key part of the framework and we should find a way to make them as high quality and consistent as we can. So I agree with version locking. But isn't that already done in [`go.mod` here](https://github.com/stretchr/testify/blob/master/go.mod#L4)?

Patch:
diff --git a/assert/assertions.go b/assert/assertions.go
index bdd8138..b73b2d5 100644
--- a/assert/assertions.go
+++ b/assert/assertions.go
@@ -1553,6 +1553,7 @@ var spewConfig = spew.ConfigState{
 	DisablePointerAddresses: true,
 	DisableCapacities:       true,
 	SortKeys:                true,
+	DisableMethods:          true,
 }
 
 type tHelper interface {
diff --git a/mock/mock.go b/mock/mock.go
index 58e0798..6be8937 100644
--- a/mock/mock.go
+++ b/mock/mock.go
@@ -527,6 +527,45 @@ func (m *Mock) AssertNotCalled(t TestingT, methodName string, arguments ...inter
 	return true
 }
 
+// IsMethodCallable checking that the method can be called
+// If the method was called more than `Repeatability` return false
+func (m *Mock) IsMethodCallable(t TestingT, methodName string, arguments ...interface{}) bool {
+	if h, ok := t.(tHelper); ok {
+		h.Helper()
+	}
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+
+	for _, v := range m.ExpectedCalls {
+		if v.Method != methodName {
+			continue
+		}
+		if len(arguments) != len(v.Arguments) {
+			continue
+		}
+		if v.Repeatability < v.totalCalls {
+			continue
+		}
+		if isArgsEqual(v.Arguments, arguments) {
+			return true
+		}
+	}
+	return false
+}
+
+// isArgsEqual compares arguments
+func isArgsEqual(expected Arguments, args []interface{}) bool {
+	if len(expected) != len(args) {
+		return false
+	}
+	for i, v := range args {
+		if !reflect.DeepEqual(expected[i], v) {
+			return false
+		}
+	}
+	return true
+}
+
 func (m *Mock) methodWasCalled(methodName string, expected []interface{}) bool {
 	for _, call := range m.calls() {
 		if call.Method == methodName {


Test patch:
diff --git a/assert/assertions_test.go b/assert/assertions_test.go
index 35a643e..ebcb8c3 100644
--- a/assert/assertions_test.go
+++ b/assert/assertions_test.go
@@ -1750,6 +1750,15 @@ func TestYAMLEq_ArraysOfDifferentOrder(t *testing.T) {
 	False(t, YAMLEq(mockT, `["foo", {"hello": "world", "nested": "hash"}]`, `[{ "hello": "world", "nested": "hash"}, "foo"]`))
 }
 
+type diffTestingStruct struct {
+	A string
+	B int
+}
+
+func (d *diffTestingStruct) String() string {
+	return d.A
+}
+
 func TestDiff(t *testing.T) {
 	expected := `
 
@@ -1829,6 +1838,42 @@ Diff:
 		map[string]int{"one": 1, "three": 3, "five": 5, "seven": 7},
 	)
 	Equal(t, expected, actual)
+
+	expected = `
+
+Diff:
+--- Expected
++++ Actual
+@@ -1,3 +1,3 @@
+ (*errors.errorString)({
+- s: (string) (len=19) "some expected error"
++ s: (string) (len=12) "actual error"
+ })
+`
+
+	actual = diff(
+		errors.New("some expected error"),
+		errors.New("actual error"),
+	)
+	Equal(t, expected, actual)
+
+	expected = `
+
+Diff:
+--- Expected
++++ Actual
+@@ -2,3 +2,3 @@
+  A: (string) (len=11) "some string",
+- B: (int) 10
++ B: (int) 15
+ }
+`
+
+	actual = diff(
+		diffTestingStruct{A: "some string", B: 10},
+		diffTestingStruct{A: "some string", B: 15},
+	)
+	Equal(t, expected, actual)
 }
 
 func TestTimeEqualityErrorFormatting(t *testing.T) {
diff --git a/mock/mock_test.go b/mock/mock_test.go
index 7da34b7..bfa123f 100644
--- a/mock/mock_test.go
+++ b/mock/mock_test.go
@@ -1143,6 +1143,41 @@ func Test_Mock_AssertNotCalled(t *testing.T) {
 
 }
 
+func Test_Mock_IsMethodCallable(t *testing.T) {
+	var mockedService = new(TestExampleImplementation)
+
+	arg := []Call{{Repeatability: 1}, {Repeatability: 2}}
+	arg2 := []Call{{Repeatability: 1}, {Repeatability: 1}}
+	arg3 := []Call{{Repeatability: 1}, {Repeatability: 1}}
+
+	mockedService.On("Test_Mock_IsMethodCallable", arg2).Return(true).Twice()
+
+	assert.False(t, mockedService.IsMethodCallable(t, "Test_Mock_IsMethodCallable", arg))
+	assert.True(t, mockedService.IsMethodCallable(t, "Test_Mock_IsMethodCallable", arg2))
+	assert.True(t, mockedService.IsMethodCallable(t, "Test_Mock_IsMethodCallable", arg3))
+
+	mockedService.MethodCalled("Test_Mock_IsMethodCallable", arg2)
+	mockedService.MethodCalled("Test_Mock_IsMethodCallable", arg2)
+
+	assert.False(t, mockedService.IsMethodCallable(t, "Test_Mock_IsMethodCallable", arg2))
+}
+
+func TestIsArgsEqual(t *testing.T) {
+	var expected = Arguments{5, 3, 4, 6, 7, 2}
+	var args = make([]interface{}, 5)
+	for i := 1; i < len(expected); i++ {
+		args[i-1] = expected[i]
+	}
+	args[2] = expected[1]
+	assert.False(t, isArgsEqual(expected, args))
+
+	var arr = make([]interface{}, 6)
+	for i := 0; i < len(expected); i++ {
+		arr[i] = expected[i]
+	}
+	assert.True(t, isArgsEqual(expected, arr))
+}
+
 func Test_Mock_AssertOptional(t *testing.T) {
 	// Optional called
 	var ms1 = new(TestExampleImplementation)