Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

Incorrect check for function arguments for Do and DoAndReturn in 1.6 #637

Closed
shenmo3 opened this issue Apr 22, 2022 · 3 comments
Closed
Labels
status: needs more info This issue need more information from the author. type: question

Comments

@shenmo3
Copy link

shenmo3 commented Apr 22, 2022

Actual behavior

In mock version 1.6, PR #558 added check to compare the function arguments and log if it mismatches. This is causing problems in which the variadic (...) argument in the function is counted as a fixed argument.

func test(b ...interface{}) {}

func main() {
	typ := reflect.TypeOf(test)
	fmt.Printf("NumIn: %d", typ.NumIn()) // this prints NumIn: 1
}

Due to this, if the Do function is called without any arguments, the function will fail due to arguments mismatch, even though the no argument behavior is expected.

Expected behavior

Ignore variadic argument when counting expected function arguments.
Note: This issue was not seen in 1.5. Most probably PR #558 caused this.

To Reproduce Steps to reproduce the behavior

N/A

Additional Information

  • gomock mode (reflect or source):
  • gomock version or git ref:
  • golang version:

Triage Notes for the Maintainers

@codyoss
Copy link
Member

codyoss commented Apr 24, 2022

I believe this is fixed on HEAD by #601 iirc. Would you mind installing from source to see if that fixes the issue for you. We might just need to cut a new release.

@codyoss codyoss added type: question status: needs more info This issue need more information from the author. labels Apr 24, 2022
@shenmo3
Copy link
Author

shenmo3 commented Apr 25, 2022

Thanks for the reply. I haven't tried, but it seems like #601 will still error out on variadic functions, and log says variadic cannot be used? So basically we cannot mock variadic function? It seems like #101 has support on variadic function matches, can't we do the same thing here?

A bit more context, we are using mock gen to mock k8s client methods like create, which has two argument and a variadic argument. After we tried to upgrade to 1.6 the test is breaking on Do(func(_ context.Context, _ runtime.Object)) {...}

@codyoss
Copy link
Member

codyoss commented May 26, 2022

If it takes a variadic func so should the method you are passing to Do, even if it is unused. There is context in the previous PR and issues that are linked. I am closing this for now unless you can reproduce on HEAD. Planning a release in the next month that will have these changes.

@codyoss codyoss closed this as completed May 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: needs more info This issue need more information from the author. type: question
Projects
None yet
Development

No branches or pull requests

2 participants