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

mock: fix map[string]struct{} map[string]interface{} Argument compare panic #1401

Closed

Conversation

lecw199
Copy link

@lecw199 lecw199 commented Jun 10, 2023

Summary

Panic in https://go.dev/play/p/CPpk9RyaCI1 (test case added by @dolmen)

Changes

Change AnythingOfType matching by ignoring spaces in types names (description added by @dolmen)

Motivation

the case in the mock.go file will case panic

	Mock.On("Unmarshal", AnythingOfType("*map[string]interface{}")).Return().Run(func(args Arguments) {
		arg := args.Get(0).(*map[string]interface{})
		arg["foo"] = "bar"
	})

reflect.TypeOf(actual).String() will print *map[string]interface {},not *map[string]interface{}, there will be an additional space in the former.

Related issues

@dolmen dolmen added the pkg-mock Any issues related to Mock label Jul 5, 2023
@dolmen
Copy link
Collaborator

dolmen commented Jul 25, 2023

Please take more time to edit the description of this issue. This project gets many request, so please spare the valuable maintainers' time.

Also adding a reproducible test case running on go.dev/play would be helpful.

@dolmen dolmen changed the title fix map[string]struct{} map[string]interface{} Argument compare panic mock: fix map[string]struct{} map[string]interface{} Argument compare panic Oct 16, 2023
@lecw199
Copy link
Author

lecw199 commented Feb 20, 2024

I'm sorry, show the code: https://go.dev/play/p/JL7B_CU8tiJ

@lecw199 lecw199 force-pushed the hotfix-mock-type-compare-panic branch from ab0d20a to dbcac76 Compare February 20, 2024 14:55
@lecw199
Copy link
Author

lecw199 commented Feb 20, 2024

Pay attention to this sentence, reflect.TypeOf(actual).String() will print *map[string]interfac {},not *map[string]interfac{} , there will be an additional space in the former

@dolmen dolmen added the mock.ArgumentMatcher About matching arguments in mock label Mar 7, 2024
@dolmen
Copy link
Collaborator

dolmen commented Mar 7, 2024

Here is a simplified and self-contained test case:

https://go.dev/play/p/CPpk9RyaCI1

type X struct {
	mock.Mock
}

func (x *X) M(m *map[string]interface{}) {
	_ = x.Mock.Called(m)
}

func Test(t *testing.T) {
	var x X
	x.On("M", mock.AnythingOfType("*map[string]interface{}")).Return(nil).Run(func(args mock.Arguments) {
		arg := args.Get(0).(*map[string]interface{})
		(*arg)["foo"] = "bar"
	})

	m := make(map[string]interface{})
	x.M(&m)
}

@dolmen
Copy link
Collaborator

dolmen commented Mar 7, 2024

@lecw199 The workaround for this case is to use mock.IsType((*map[string]interface{})(nil)) instead of mock.AnythingOfType("*map[string]interface{}") (see the modified test case). This is type safe and do not depend on stringification of types (which might also change from one Go version to the other).

I am not convinced by the proposed patch which, I fear, could trigger other issues.

@dolmen dolmen added the wontfix label Mar 7, 2024
@dolmen
Copy link
Collaborator

dolmen commented Mar 7, 2024

I'm marking this as "wontfix" as the right solution is to use mock.IsType.

(My personal opinion: in v2 we should just drop mock.AnythingOfType(string) and keep only mock.IsType because AnythingOfType relies too much on reflect quirks).

@dolmen dolmen closed this Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted mock.ArgumentMatcher About matching arguments in mock pkg-mock Any issues related to Mock wontfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants