testify icon indicating copy to clipboard operation
testify copied to clipboard

call.Unset() panics when call := mockObj.On("Foo", mock.AnythingOfType("foo"))

Open jeandeducla opened this issue 1 year ago • 4 comments

Description

When you use mock.AnythingOfType("foo") in On(...) and then try to call Unset() on the resulting call, the code panics.

Step To Reproduce

Here is a simple snippet that reproduces the issue:

import (
	"testing"

	"github.com/stretchr/testify/mock"
)

type Foo struct {
	doer Doer
}

type Doer interface {
	DoSomething(number int) (bool, error)
}

func (f Foo) Do(number int) (bool, error) {
	return f.doer.DoSomething(number)
}

type MyMockedObject struct {
	mock.Mock
}

func (m *MyMockedObject) DoSomething(number int) (bool, error) {
	args := m.Called(number)
	return args.Bool(0), args.Error(1)
}

func TestSomething(t *testing.T) {
	testObj := new(MyMockedObject)

	foo := Foo{doer: testObj}

	call := testObj.On("DoSomething", 2).Return(true, nil)
	_, _ = foo.Do(2)
	testObj.AssertExpectations(t)
	call.Unset() // Works!

	call = testObj.On("DoSomething", mock.AnythingOfType("int")).Return(true, nil)
	_, _ = foo.Do(2)
	testObj.AssertExpectations(t)
	call.Unset() // panics...
}

Expected behavior

I expect to be able to call Unset() on a call that has been mocked with mock.AnythingOfType (or did I miss something on how to use the API? Happy to get feedback here :-))

Actual behavior

When calling Unset() on a call that has been mocked with mock.AnythingOfType, it panics.

jeandeducla avatar May 17 '24 12:05 jeandeducla

from locally debugging, I have the impression the call to Diff in Unset is not correctly comparing the arguments here; it does not cover the case of actual being a anythingOfTypeArgument. Locally I did that:

              switch expected := expected.(type) {                                                                                                 
              case anythingOfTypeArgument:
                  // type checking
~                 switch actual := actual.(type) {
~                 case anythingOfTypeArgument:     
~                     if string(actual) != string(expected) {     
~                         differences++     
+                     }     
+                 default:     
+                     if reflect.TypeOf(actual).Name() != string(expected) && reflect.TypeOf(actual).String() != string(expected) {     
+                         // not match     
+                         differences++     
+                         output = fmt.Sprintf("%s\t%d: FAIL:  type %s != type %s - %s\n", output, i, expected, reflect.TypeOf(actual).Name(), actu
+                     }     
                  }                                                                        

which does the trick.

Happy to make a PR

jeandeducla avatar May 17 '24 12:05 jeandeducla

Suffering from the same bug when trying to mock argument of type *context.valueCtx. Any updates on this topic? @jeandeducla

mavvkel avatar Oct 04 '24 19:10 mavvkel

Aaah Unset, how I lothe thee.

Unset is rather inappropriately using mock.Arguments.Diff to find itself in ExpectedCalls. Diff is intended to match the code under test's calls against ExpectedCalls, so it works at finding Calls in ExpectedCalls when their arguments are values, such as On("Method", 59). But it cannot be made to work for anythingOfTypeArgument without also making that into an argument that matches when calling the mock. I dislike this already but it's made worse as anythingOfTypeArgument is exported via a type alias.

This bug also occurs with the exported IsTypeArgument from mock.IsType().

This bug also occurs with argumentMatcher from mock.MatchedBy() which is impossible to compare as it contains a function. So no modification to Diff can fix this problem.

The best workaround is to never use Unset, instead just don't set it in the first place.

brackendawson avatar Oct 05 '24 17:10 brackendawson

Aaah Unset, how I lothe thee.

Unset is rather inappropriately using mock.Arguments.Diff to find itself in ExpectedCalls. Diff is intended to match the code under test's calls against ExpectedCalls, so it works at finding Calls in ExpectedCalls when their arguments are values, such as On("Method", 59). But it cannot be made to work for anythingOfTypeArgument without also making that into an argument that matches when calling the mock. I dislike this already but it's made worse as anythingOfTypeArgument is exported via a type alias.

This bug also occurs with the exported IsTypeArgument from mock.IsType().

This bug also occurs with argumentMatcher from mock.MatchedBy() which is impossible to compare as it contains a function. So no modification to Diff can fix this problem.

The best workaround is to never use Unset, instead just don't set it in the first place.

well, this means there are a lot of problems with using Unset().

while I'm using uber's fx to inject my dependencies, so I fx.Replace the mockService into App at initial of the test function which lead to I had to clear mockService in every t.Run().

is there any workaround? thx

gray19950103 avatar Jan 20 '25 06:01 gray19950103