Repository: stretchr/testify (go)

Issue:
Functional Options testing broken for indirect calls
The changes introduced to support testing of functional options (https://github.com/stretchr/testify/pull/1023) are broken with indirect calls.

When the service which exposes the method that supports functional options is injected into another function and invoked, the mock assertions are broken.

The following test and mocks should be supported.

```diff
diff --git a/mock/mock_test.go b/mock/mock_test.go
index 60cf5d0..1c7ca47 100644
--- a/mock/mock_test.go
+++ b/mock/mock_test.go
@@ -20,6 +20,7 @@ import (
 // ExampleInterface represents an example interface.
 type ExampleInterface interface {
 	TheExampleMethod(a, b, c int) (int, error)
+	TheExampleMethodFunctionalOptions(x string, opts ...OptionFn) error
 }
 
 // TestExampleImplementation is a test implementation of ExampleInterface
@@ -1455,6 +1456,27 @@ func Test_Mock_AssertExpectationsFunctionalOptionsType(t *testing.T) {
 
 }
 
+func Test_Mock_AssertExpectationsFunctionalOptionsType_indirect(t *testing.T) {
+
+	indirectFunctionalOptions := func(ei ExampleInterface) {
+		ei.TheExampleMethodFunctionalOptions("testing", OpStr("test"), OpNum(1))
+	}
+
+	var mockedService = new(TestExampleImplementation)
+
+	mockedService.On("TheExampleMethodFunctionalOptions", "testing", FunctionalOptions(OpStr("test"), OpNum(1))).Return(nil).Once()
+
+	tt := new(testing.T)
+	assert.False(t, mockedService.AssertExpectations(tt))
+
+	// make the call now
+	indirectFunctionalOptions(mockedService)
+
+	// now assert expectations
+	assert.True(t, mockedService.AssertExpectations(tt))
+
+}
+
 func Test_Mock_AssertExpectationsFunctionalOptionsType_Empty(t *testing.T) {
 
 	var mockedService = new(TestExampleImplementation)
```

but instead results in the following error:

```sh
mock: Unexpected Method Call
-----------------------------

TheExampleMethodFunctionalOptions(string,[]mock.OptionFn)
		0: "testing"
		1: []mock.OptionFn{(mock.OptionFn)(0x11a8020), (mock.OptionFn)(0x11a8000)}

The closest call I have is: 

TheExampleMethodFunctionalOptions(string,*mock.FunctionalOptionsArgument)
		0: "testing"
		1: []mock.OptionFn{(mock.OptionFn)(0x11a0980), (mock.OptionFn)(0x11a0960)}


Diff: 0: PASS:  (string=testing) == (string=testing)
	1: FAIL:  [mock.Test_Mock_AssertExpectationsFunctionalOptionsType_indirect.func1 mock.Test_Mock_AssertExpectationsFunctionalOptionsType_indirect.func1] != [mock.Test_Mock_AssertExpectationsFunctionalOptionsType_indirect mock.Test_Mock_AssertExpectationsFunctionalOptionsType_indirect]
```

The culprit seems to be the [`funcName`](https://github.com/stretchr/testify/blob/4b2f4d2bcff3848b6a4e63d462da6ce3c21e9c78/mock/mock.go#L1220C1-L1223) call which is used in [`assertOpts`](https://github.com/stretchr/testify/blob/master/mock/mock.go#L1167C1-L1173) returning unexpected values.

Pull request description:
fix: compare functional option names for indirect calls
## Summary
Closes Functional Options testing broken for indirect calls #1380. The fix is consists of not comparing the the function names, as it is already asserted when comparing `expectedValues` and `actualValues`

## Changes
* Adds test `Test_Mock_AssertExpectationsFunctionalOptionsTypeIndirectly` which would fail without the fix
* Adds test `Test_Mock_AssertExpectationsFunctionalOptionsType_Diff` to examine behavior of slightly different functional options in expected vs actual.
* Changes the `funcName` implementation to only compare the relevant last part, if applicable. This aids in reading the diff that is produced when the function names don't. match (for go1.20 or higher)

## Motivation
The choice was made to omit the comparison of function names completely, instead of comparing only the relevant last part. The reason is that go1.19 and lower don't give enough any meaningful comparable part in the function name. Nevertheless the most relevant part of the function name is included in the diff result. Without it, the diff yielded in `Test_Mock_AssertExpectationsFunctionalOptionsType_Diff` would have resulted in (go1.20):
```
mock.Test_Mock_AssertExpectationsFunctionalOptionsType_Diff.OpNum &{num:0 str:1} != mock.Test_Mock_AssertExpectationsFunctionalOptionsType_Diff.OpNum &{num:1 str:}
```
The current implementation instead yields a diff:
```
OpStr &{num:0 str:1} != OpNum &{num:1 str:}
```

Though there could be added benefit in displaying the full location of the function, I can revert that part if desired.

## Related issues
Closes #1380


Patch:
diff --git a/mock/mock.go b/mock/mock.go
index 4932833..eb5682d 100644
--- a/mock/mock.go
+++ b/mock/mock.go
@@ -1204,32 +1204,38 @@ type tHelper interface {
 func assertOpts(expected, actual interface{}) (expectedFmt, actualFmt string) {
 	expectedOpts := reflect.ValueOf(expected)
 	actualOpts := reflect.ValueOf(actual)
+
+	var expectedFuncs []*runtime.Func
 	var expectedNames []string
 	for i := 0; i < expectedOpts.Len(); i++ {
-		expectedNames = append(expectedNames, funcName(expectedOpts.Index(i).Interface()))
+		f := runtimeFunc(expectedOpts.Index(i).Interface())
+		expectedFuncs = append(expectedFuncs, f)
+		expectedNames = append(expectedNames, funcName(f))
 	}
+	var actualFuncs []*runtime.Func
 	var actualNames []string
 	for i := 0; i < actualOpts.Len(); i++ {
-		actualNames = append(actualNames, funcName(actualOpts.Index(i).Interface()))
+		f := runtimeFunc(actualOpts.Index(i).Interface())
+		actualFuncs = append(actualFuncs, f)
+		actualNames = append(actualNames, funcName(f))
 	}
-	if !assert.ObjectsAreEqual(expectedNames, actualNames) {
+
+	if expectedOpts.Len() != actualOpts.Len() {
 		expectedFmt = fmt.Sprintf("%v", expectedNames)
 		actualFmt = fmt.Sprintf("%v", actualNames)
 		return
 	}
 
 	for i := 0; i < expectedOpts.Len(); i++ {
-		expectedOpt := expectedOpts.Index(i).Interface()
-		actualOpt := actualOpts.Index(i).Interface()
-
-		expectedFunc := expectedNames[i]
-		actualFunc := actualNames[i]
-		if expectedFunc != actualFunc {
-			expectedFmt = expectedFunc
-			actualFmt = actualFunc
+		if !isFuncSame(expectedFuncs[i], actualFuncs[i]) {
+			expectedFmt = expectedNames[i]
+			actualFmt = actualNames[i]
 			return
 		}
 
+		expectedOpt := expectedOpts.Index(i).Interface()
+		actualOpt := actualOpts.Index(i).Interface()
+
 		ot := reflect.TypeOf(expectedOpt)
 		var expectedValues []reflect.Value
 		var actualValues []reflect.Value
@@ -1247,9 +1253,9 @@ func assertOpts(expected, actual interface{}) (expectedFmt, actualFmt string) {
 		reflect.ValueOf(actualOpt).Call(actualValues)
 
 		for i := 0; i < ot.NumIn(); i++ {
-			if !assert.ObjectsAreEqual(expectedValues[i].Interface(), actualValues[i].Interface()) {
-				expectedFmt = fmt.Sprintf("%s %+v", expectedNames[i], expectedValues[i].Interface())
-				actualFmt = fmt.Sprintf("%s %+v", expectedNames[i], actualValues[i].Interface())
+			if expectedArg, actualArg := expectedValues[i].Interface(), actualValues[i].Interface(); !assert.ObjectsAreEqual(expectedArg, actualArg) {
+				expectedFmt = fmt.Sprintf("%s(%T) -> %#v", expectedNames[i], expectedArg, expectedArg)
+				actualFmt = fmt.Sprintf("%s(%T) -> %#v", expectedNames[i], actualArg, actualArg)
 				return
 			}
 		}
@@ -1258,7 +1264,25 @@ func assertOpts(expected, actual interface{}) (expectedFmt, actualFmt string) {
 	return "", ""
 }
 
-func funcName(opt interface{}) string {
-	n := runtime.FuncForPC(reflect.ValueOf(opt).Pointer()).Name()
-	return strings.TrimSuffix(path.Base(n), path.Ext(n))
+func runtimeFunc(opt interface{}) *runtime.Func {
+	return runtime.FuncForPC(reflect.ValueOf(opt).Pointer())
+}
+
+func funcName(f *runtime.Func) string {
+	name := f.Name()
+	trimmed := strings.TrimSuffix(path.Base(name), path.Ext(name))
+	splitted := strings.Split(trimmed, ".")
+
+	if len(splitted) == 0 {
+		return trimmed
+	}
+
+	return splitted[len(splitted)-1]
+}
+
+func isFuncSame(f1, f2 *runtime.Func) bool {
+	f1File, f1Loc := f1.FileLine(f1.Entry())
+	f2File, f2Loc := f2.FileLine(f2.Entry())
+
+	return f1File == f2File && f1Loc == f2Loc
 }


Test patch:
diff --git a/mock/mock_test.go b/mock/mock_test.go
index d286867..2de2773 100644
--- a/mock/mock_test.go
+++ b/mock/mock_test.go
@@ -50,11 +50,22 @@ func OpStr(s string) OptionFn {
 		o.str = s
 	}
 }
+
+func OpBytes(b []byte) OptionFn {
+	return func(m *options) {
+		m.str = string(b)
+	}
+}
+
 func (i *TestExampleImplementation) TheExampleMethodFunctionalOptions(x string, opts ...OptionFn) error {
 	args := i.Called(x, opts)
 	return args.Error(0)
 }
 
+func TheExampleMethodFunctionalOptionsIndirect(i *TestExampleImplementation) {
+	i.TheExampleMethodFunctionalOptions("test", OpNum(1), OpStr("foo"))
+}
+
 //go:noinline
 func (i *TestExampleImplementation) TheExampleMethod2(yesorno bool) {
 	i.Called(yesorno)
@@ -1522,6 +1533,51 @@ func Test_Mock_AssertExpectationsFunctionalOptionsType_Empty(t *testing.T) {
 
 }
 
+func Test_Mock_AssertExpectationsFunctionalOptionsType_Indirectly(t *testing.T) {
+
+	var mockedService = new(TestExampleImplementation)
+
+	mockedService.On("TheExampleMethodFunctionalOptions", "test", FunctionalOptions(OpNum(1), OpStr("foo"))).Return(nil).Once()
+
+	tt := new(testing.T)
+	assert.False(t, mockedService.AssertExpectations(tt))
+
+	// make the call now
+	TheExampleMethodFunctionalOptionsIndirect(mockedService)
+
+	// now assert expectations
+	assert.True(t, mockedService.AssertExpectations(tt))
+
+}
+
+func Test_Mock_AssertExpectationsFunctionalOptionsType_Diff_Func(t *testing.T) {
+
+	var mockedService = new(TestExampleImplementation)
+
+	mockedService.On("TheExampleMethodFunctionalOptions", "test", FunctionalOptions(OpStr("this"))).Return(nil).Once()
+
+	tt := new(testing.T)
+	assert.False(t, mockedService.AssertExpectations(tt))
+
+	assert.Panics(t, func() {
+		mockedService.TheExampleMethodFunctionalOptions("test", OpBytes([]byte("this")))
+	})
+}
+
+func Test_Mock_AssertExpectationsFunctionalOptionsType_Diff_Arg(t *testing.T) {
+
+	var mockedService = new(TestExampleImplementation)
+
+	mockedService.On("TheExampleMethodFunctionalOptions", "test", FunctionalOptions(OpStr("this"))).Return(nil).Once()
+
+	tt := new(testing.T)
+	assert.False(t, mockedService.AssertExpectations(tt))
+
+	assert.Panics(t, func() {
+		mockedService.TheExampleMethodFunctionalOptions("test", OpStr("that"))
+	})
+}
+
 func Test_Mock_AssertExpectations_With_Repeatability(t *testing.T) {
 
 	var mockedService = new(TestExampleImplementation)