Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MockGeneric doesn't take into account the provided type #74

Open
bioform opened this issue Dec 17, 2024 · 12 comments
Open

MockGeneric doesn't take into account the provided type #74

bioform opened this issue Dec 17, 2024 · 12 comments

Comments

@bioform
Copy link

bioform commented Dec 17, 2024

I'm writing a custom test matcher.
That matcher looks like this:

type CallActionMatcher[A action.Action] struct {
	ExpectedAction    A
}

// CallAction initializes the matcher with the expected action.
func CallAction[A action.Action](expectedAction A) *CallActionMatcher[A] {
	return &CallActionMatcher[A]{
		ExpectedAction: expectedAction,
	}
}

func (m *CallActionMatcher[A]) Match(actual any) (success bool, err error) {
        originNew := action.New[A]
	mockNew := func(a A) *action.Performer[A] {
	    // prints `ActionB` two times - one for real value, another one when we call the `New` method for `ActionA`
	    // the printed value has `ActionB` with some unexpected field values
	    fmt.Printf("Mocking action.New for %T: %v\n", a, a) 
	     
	    return originNew(a)
	}
	// Mock action.New to intercept action performer creation.
	guardNew := mockey.MockGeneric(action.New[A]).To(mockNew).Origin(&originNew).Build()
	defer guardNew.UnPatch()

	// Run the actual function
	if fn, ok := actual.(func()); ok {
		fn()
	} else {
		return false, fmt.Errorf("CallActionMatcher expects a function to execute")
	}

        // some checks...
}

And I call that matcher like:

func TestActionA(t *testing.T) {
	Expect(func() {
		// Code that triggers the action
                action.New(&ActionB{})
		action.New(&ActionA{})
	}).To(
                CallAction(&ActionB{})
              )
}

The definition of the action.Action, action.Performer looks like:

 package action
 
 type Action interface{}
 type Performer[A Action] struct {
	action A
 }
 func New[A Action](action A) *Performer[A] {
	return &Performer[A]{action: action}
 }

The thing is that the action.New[A] mock:

guardNew := mockey.MockGeneric(action.New[A]).To(mockNew).Origin(&originNew).Build()

applies for both ActionA and ActionB New method calls.
Somehow it casts ActionA to ActionB(I see it in the VSCode debugger as well). It shouldn't be possible at all since they are two completely different classes.

The expected behavior: it should fire only once for ActionB since it is defined in the matcher likes CallAction(&ActionB{})

@Sychorius
Copy link
Collaborator

Im not sure if ActionA and ActionB has the same GC way,golang has a special implementation of generic functions if two types has the same 'GC' way. for example:

type String string
MockGeneric(Foo[String]).Return("mocked").Build()
var s string
Foo(s) // mocked, which should not happen

Here String is in different type with string. But golang would still create the same implementation of their generic functions, which means that calls to Foo[String] and Foo[string] are essentially the same. (not only types like type XXX xxx have this 'feature')
If you happen to hit this case, we have a not-so-elegant solution:

func Foo[T any](t T) T { return t }

type String string

func TestXxx(t *testing.T) {
	mockey.PatchConvey("use generic info", t, func() {
		// 1. get _stringInfo unique identifier
		var _StringInfo mockey.GenericInfo
		mocker := mockey.MockGeneric(Foo[String]).To(func(_info mockey.GenericInfo, s String) String {
			_StringInfo = _info
			return s
		}).Build()
		Foo[String]("just call once for _stringInfo init")

		// 2. only String will return mocked, other type with the same gc way will call origin
		var origin = Foo[String]
		mocker.To(func(_info mockey.GenericInfo, s String) String {
			if _info == _StringInfo {
				return String("mocked")
			}
			return origin(s)
		}).Origin(&origin)

		t.Log(Foo("123"))         // 123
		t.Log(Foo(String("123"))) // mocked
	})

	// or here is another way not recommended
	mockey.PatchConvey("not recommended but works", t, func() {
		var origin = Foo[String]
		mocker := mockey.MockGeneric(Foo[String]).To(func(_info mockey.GenericInfo, s String) String {
			// About UsedParamType(n):
			// If index n is out of range, or the derived types have more complex structure(for example: define an generic struct
			// in a generic function using generic types, unused parameterized type etc.), this function may return unexpected value
			// or cause unrecoverable runtime error if passed a wrong value 'n'. So it is NOT RECOMMENDED to use this function unless 
                        // you actually knows what you are doing.
			if _info.UsedParamType(0) == reflect.TypeOf(String("")) {
				return String("mocked")
			}
			return origin(s)
		}).Origin(&origin).Build()
		t.Log(Foo("123"))         // 123
		t.Log(Foo(String("123"))) // mocked
		mocker.UnPatch()
	})
}

@bioform
Copy link
Author

bioform commented Dec 18, 2024

@Sychorius, thank you for your response 🙇

I suppose the issue is a little bit different...
I just created real steps to reproduce:

// main.go

package main

import (
	"fmt"
)

func main() {
	New(&ActionA{AttrA: "A!"}).Perform()
	New(&ActionB{AttrB: "B!"}).Perform()
}

/////////////////////////////////////////////////////////////////////////////////////////
// Interfaces implementation
/////////////////////////////////////////////////////////////////////////////////////////

type Action interface {
	Perform()
}

type Performer[A Action] struct {
	action A
}

func New[A Action](action A) *Performer[A] {
	return &Performer[A]{action: action}
}

func (p Performer[A]) Perform() {
	p.action.Perform()
}

/////////////////////////////////////////////////////////////////////////////////////////
// Example Action implementation
/////////////////////////////////////////////////////////////////////////////////////////

type ActionA struct {
	AttrA string
}

func (a *ActionA) Perform() {
	fmt.Printf("Do something: %T. AttrA %v\n", a, a.AttrA)
}

type ActionB struct {
	AttrB string
}

// Implement the specific behavior for ActionB.
func (b *ActionB) Perform() {
	fmt.Printf("Do something: %T. AttrB %v\n", b, b.AttrB)
}

And the related test:

// main_test.go

package main_test

import (
	"fmt"
	"testing"

	main "action_usage_example3"
	"github.com/bytedance/mockey"

	. "github.com/onsi/gomega"
)

func TestActionA(t *testing.T) {
	RegisterTestingT(t)

	Expect(func() {
		main.New(&main.ActionA{AttrA: "A!"}).Perform()
		main.New(&main.ActionB{AttrB: "B!"}).Perform()
	}).To(CallAction(&main.ActionB{}))
}

/////////////////////////////////////////////////////////////////////////////////////////
// Matcher implementation
/////////////////////////////////////////////////////////////////////////////////////////

type CallActionMatcher[A main.Action] struct {
	ExpectedAction A
}

// CallAction initializes the matcher with the expected action.
func CallAction[A main.Action](expectedAction A) *CallActionMatcher[A] {
	return &CallActionMatcher[A]{
		ExpectedAction: expectedAction,
	}
}

func (m *CallActionMatcher[A]) Match(actual any) (success bool, err error) {
	originNew := main.New[A]
	mockNew := func(a A) *main.Performer[A] {
		// prints `ActionB` two times - one for real value, another one when we call the `New` method for `ActionA`
		// the printed value has `ActionB` with some unexpected field values
		fmt.Printf("Mocking action.New for %T: %v\n", a, a)

		return originNew(a)
	}
	// Mock action.New to intercept action performer creation.
	guardNew := mockey.MockGeneric(main.New[A]).To(mockNew).Origin(&originNew).Build()
	defer guardNew.UnPatch()

	// Run the actual function
	if fn, ok := actual.(func()); ok {
		fn()
	} else {
		return false, fmt.Errorf("CallActionMatcher expects a function to execute")
	}

	// some checks...
	return true, nil
}

// FailureMessage returns the failure message.
func (m *CallActionMatcher[A]) FailureMessage(actual any) string {
	return fmt.Sprintf("Expected to call action %T", m.ExpectedAction)
}

// NegatedFailureMessage returns the negated failure message.
func (m *CallActionMatcher[A]) NegatedFailureMessage(actual any) string {
	return fmt.Sprintf("Did not expect to call action %T", m.ExpectedAction)
}

The result:
image

But, if you change the AttrA type from string to int. Like:

type ActionA struct {
	AttrA int
}

and update the related code, you will get a completely unexpected error:

panic: runtime error: growslice: len out of range [recovered]
	panic: runtime error: growslice: len out of range

In the following line:

fmt.Printf("Mocking action.New for %T: %v\n", a, a)

It panics on %v parameter translation

@bioform
Copy link
Author

bioform commented Dec 18, 2024

It might be possible that the mockey receives a reference to the Action interface instead of a real struct type, and it should resolve a real struct type hidden behind the interface reference first 🤔

@Sychorius
Copy link
Collaborator

Actually that's the case i mentioned, you just want to mock New[ActionB], but New[ActionA] was also affected.
Try code below, you will see how it happened

func (m *CallActionMatcher[A]) Match(actual any) (success bool, err error) {
	originNew := New[A]
	mockNew := func(_info mockey.GenericInfo, a A) *Performer[A] {
+		if _info.UsedParamType(0) == reflect.TypeOf(&ActionA{}) {
+			fmt.Println("Opps, type *ActionA")
+			return originNew(a)
+		}
		// prints `ActionB` two times - one for real value, another one when we call the `New` method for `ActionA`
		// the printed value has `ActionB` with some unexpected field values
		fmt.Printf("Mocking action.New for %T: %v\n", a, a)

		return originNew(a)
	}
	// Mock action.New to intercept action performer creation.
+	fmt.Printf("Only mock %v\n", reflect.TypeOf(New[A]))
	guardNew := mockey.MockGeneric(New[A]).To(mockNew).Origin(&originNew).Build()
	defer guardNew.UnPatch()

	// Run the actual function
	if fn, ok := actual.(func()); ok {
		fn()
	} else {
		return false, fmt.Errorf("CallActionMatcher expects a function to execute")
	}

	// some checks...
	return true, nil
}

@bioform
Copy link
Author

bioform commented Dec 18, 2024

Your workaround suggestion works as expected(I tried the fist suggestion since, as you've mentioned, it is safe).
See the fixed Match action:

func (m *CallActionMatcher[A]) Match(actual any) (success bool, err error) {
	originNew := main.New[A]

	// fix type resolving
	var _typeInfo mockey.GenericInfo
	mocker := mockey.MockGeneric(main.New[A]).To(func(_info mockey.GenericInfo, a A) *main.Performer[A] {
		_typeInfo = _info
		return nil
	}).Build()
	main.New[A](*new(A))
	mocker.UnPatch()

	// The real mocker
	mockNew := func(_info mockey.GenericInfo, a A) *main.Performer[A] {
		if _info != _typeInfo {
			return originNew(a)
		}

		fmt.Printf("Mocking action.New for %T: %v\n", a, a)

		return originNew(a)
	}
	// Mock action.New to intercept action performer creation.
	guardNew := mockey.MockGeneric(main.New[A]).To(mockNew).Origin(&originNew).Build()
	defer guardNew.UnPatch()

	// Run the actual function
	if fn, ok := actual.(func()); ok {
		fn()
	} else {
		return false, fmt.Errorf("CallActionMatcher expects a function to execute")
	}

	// some checks...
	return true, nil
}

It looks ugly but it works 😄
Thank you very much 🙇
However, I would fix the issue since it is related to the core mockey functionality.

@bioform
Copy link
Author

bioform commented Dec 18, 2024

@Sychorius, the second-way example looks more clear:

func (m *CallActionMatcher[A]) Match(actual any) (success bool, err error) {
	originNew := main.New[A]

	// The real mocker
	mockNew := func(_info mockey.GenericInfo, a A) *main.Performer[A] {
		if _info.UsedParamType(0) != reflect.TypeOf(m.ExpectedAction) {
			return originNew(a)
		}

		fmt.Printf("Mocking action.New for %T: %v\n", a, a)

		return originNew(a)
	}
	// Mock action.New to intercept action performer creation.
	guardNew := mockey.MockGeneric(main.New[A]).To(mockNew).Origin(&originNew).Build()
	defer guardNew.UnPatch()

	// Run the actual function
	if fn, ok := actual.(func()); ok {
		fn()
	} else {
		return false, fmt.Errorf("CallActionMatcher expects a function to execute")
	}

	// some checks...
	return true, nil
}

Is it safe enough for this specific case?

@Sychorius
Copy link
Collaborator

Yes, usually if not writing nested generic types, it's safe.
In unsafe scenario, tests will encounter unrecoverable runtime error.
So as long as it runs successfully once, it is safe.

@bioform
Copy link
Author

bioform commented Dec 18, 2024

So... as you've mentioned, the second way is completely unusable for more complex cases(I made a lot of tests) 😞
So, the first(the safe method) is more applicable, but it requires one extra call for each mocked method.

@bioform
Copy link
Author

bioform commented Dec 18, 2024

@Sychorius, both workarounds don't work if we extend the example with the case like:

mocker := mockey.MockGeneric((*main.Performer[A]).Perform).To(mockPerform).Origin(&originMethod).Build()

@Sychorius
Copy link
Collaborator

@bioform Is it panic or has it back to the original function? Can you give an example?

@bioform
Copy link
Author

bioform commented Dec 19, 2024

@Sychorius,

Is it panic

😄
it was a bit
See the code below to reproduce the issue.
The code is mostly the same - I made a couple of changes:

  1. In the test, I call ActionA Perform only.
  2. Now I call ActionB Perform inside ActionA Perform
  3. the test mocks the Perform method as well
  4. added one more attribute to the New method (ctx context.Context)

See the code to reproduce:

// main.go

package main

import (
	"context"
	"fmt"
)

func main() {
	ctx := context.Background()

	New(ctx, &ActionA{AttrA: "A!"}).Perform()
	New(ctx, &ActionB{AttrB: "B!"}).Perform()
}

/////////////////////////////////////////////////////////////////////////////////////////
// Interfaces implementation
/////////////////////////////////////////////////////////////////////////////////////////

type Action interface {
	Perform() (ok bool, err error)
	Context() context.Context
	SetContext(ctx context.Context)
}

type Performer[A Action] struct {
	action A
}

func New[A Action](ctx context.Context, action A) *Performer[A] {
	ap := &Performer[A]{action: action}
	ap.action.SetContext(ctx)
	return ap
}

func (p Performer[A]) Perform() (ok bool, err error) {
	return p.action.Perform()
}

/////////////////////////////////////////////////////////////////////////////////////////
// Example Action implementation
/////////////////////////////////////////////////////////////////////////////////////////

type ActionA struct {
	AttrA string
	ctx   context.Context
}

func (a *ActionA) Context() context.Context {
	return a.ctx
}

func (a *ActionA) SetContext(ctx context.Context) {
	a.ctx = ctx
}

func (a *ActionA) Perform() (ok bool, err error) {
	fmt.Printf("ActionA perform: %T. AttrA %v\n", a, a.AttrA)
	New(a.Context(), &ActionB{AttrB: "B!"}).Perform()
	return true, nil
}

type ActionB struct {
	AttrB string
	ctx   context.Context
}

func (b *ActionB) Context() context.Context {
	return b.ctx
}

func (b *ActionB) SetContext(ctx context.Context) {
	b.ctx = ctx
}

// Implement the specific behavior for ActionB.
func (b *ActionB) Perform() (ok bool, err error) {
	fmt.Printf("ActionB perform: %T. AttrB %v\n", b, b.AttrB)

	return true, nil
}

And the related test:

// main_test.go

package main_test

import (
	"context"
	"fmt"
	"reflect"
	"testing"

	main "example"

	"github.com/bytedance/mockey"

	. "github.com/onsi/gomega"
)

func TestActionA(t *testing.T) {
	RegisterTestingT(t)
	ctx := context.Background()

	Expect(func() {
		main.New(ctx, &main.ActionA{AttrA: "A!"}).Perform()
	}).To(CallAction(&main.ActionB{}))
}

/////////////////////////////////////////////////////////////////////////////////////////
// Matcher implementation
/////////////////////////////////////////////////////////////////////////////////////////

type CallActionMatcher[A main.Action] struct {
	ExpectedAction A
}

// CallAction initializes the matcher with the expected action.
func CallAction[A main.Action](expectedAction A) *CallActionMatcher[A] {
	return &CallActionMatcher[A]{
		ExpectedAction: expectedAction,
	}
}

func (m *CallActionMatcher[A]) Match(actual any) (success bool, err error) {
	originNew := main.New[A]

	// fix type resolving
	// var _typeInfo mockey.GenericInfo
	// mocker := mockey.MockGeneric(main.New[A]).To(func(_info mockey.GenericInfo, a A) *main.Performer[A] {
	// 	_typeInfo = _info
	// 	return nil
	// }).Build()
	// main.New[A](*new(A))
	// mocker.UnPatch()

	// The real mocker
	mockNew := func(_info mockey.GenericInfo, ctx context.Context, a A) *main.Performer[A] {
		firstParamType := _info.UsedParamType(0)

		// I've commented the below line since it fails
		//fmt.Printf("New first param type: %v\n", firstParamType)

                // both values are always "not equal"
		if firstParamType != reflect.TypeOf(m.ExpectedAction) {
			return originNew(ctx, a)
		}
		// if _info != _typeInfo {
		// 	return originNew(a)
		// }

		fmt.Printf("Mocking action.New for %T: %v\n", a, a)

		return originNew(ctx, a)
	}
	// Mock action.New to intercept action performer creation.
	guardNew := mockey.MockGeneric(main.New[A]).To(mockNew).Origin(&originNew).Build()
	defer guardNew.UnPatch()

	originPerform := (*main.Performer[A]).Perform
	mockPerform := func(_info mockey.GenericInfo, ap *main.Performer[A]) (bool, error) {
		firstParamType := _info.UsedParamType(0)

		// I've commented the below line since it fails
		// fmt.Printf("Perform first param type: %v\n", firstParamType)

                // both values are always "not equal"
		if firstParamType != reflect.TypeOf(m.ExpectedAction) {
			return originPerform(ap)
		}

		fmt.Printf("Mocking action.Perform for %T: %v\n", ap, ap)
		return true, nil
	}
	guardPerform := mockey.MockGeneric((*main.Performer[A]).Perform).To(mockPerform).Origin(&originPerform).Build()
	defer guardPerform.UnPatch()

	// Run the actual function
	if fn, ok := actual.(func()); ok {
		fn()
	} else {
		return false, fmt.Errorf("CallActionMatcher expects a function to execute")
	}

	// some checks...
	return true, nil
}

// FailureMessage returns the failure message.
func (m *CallActionMatcher[A]) FailureMessage(actual any) string {
	return fmt.Sprintf("Expected to call action %T", m.ExpectedAction)
}

// NegatedFailureMessage returns the negated failure message.
func (m *CallActionMatcher[A]) NegatedFailureMessage(actual any) string {
	return fmt.Sprintf("Did not expect to call action %T", m.ExpectedAction)
}

Pay attention to the line below the // I've commented the below line since it fails comment - with the initial example it worked without errors.

And to the // both values are always "not equal" comment

@bioform
Copy link
Author

bioform commented Dec 19, 2024

@Sychorius , the funny fact, if you run the above test and put the breakpoint inside the Perform mock, on the first stop you will see the ActionB in the debugger but, actually, it is ActionA but somehow its type is identified as ActionB:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants