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

Generate typed Times for mocks #55

Open
christofferjerrebo opened this issue Aug 8, 2023 · 3 comments · May be fixed by #56
Open

Generate typed Times for mocks #55

christofferjerrebo opened this issue Aug 8, 2023 · 3 comments · May be fixed by #56

Comments

@christofferjerrebo
Copy link

Requested feature
Generate typed Times(), just as Return, Do and DoAndReturn

Why the feature is needed
I would like both of these cases supported:

// Supported, Return is typed
m.
  EXPECT().
  Bar(gomock.Eq(101)).
  Return(103).
  Times(1)
// Unsupported, Return is not typed since Times return *gomock.Call
m.
  EXPECT().
  Bar(gomock.Eq(101)).
  Times(1).
  Return(103)

(Optional) Proposed solution:
I added Times to be emitted by mockgen in my repository fork

@christofferjerrebo christofferjerrebo linked a pull request Aug 8, 2023 that will close this issue
@tchung1118
Copy link
Contributor

I'm not really sure what kind of benefit this brings. Could you explain more in detail why this feature is needed?

@christofferjerrebo
Copy link
Author

Sure, I'll give it a go 😄

As I understand it, gomock does method chaining. Given the example in the README, these two snippets should produce the same result:

m.
  EXPECT().
  Bar(gomock.Eq(101)).
  Return(103). // Return first
  Times(1) // Times last
m.
 EXPECT().
 Bar(gomock.Eq(101)).
 Times(1). // Times first
 Return(103) // Return last

If this is all wrong then let me know! 😅

Running gomock with the -typed argument, the Return func will be typed to Return(arg0 int) (instead of Return(rets ...interface{})) which is great! It helps me with my coding.

While using the first snippet above will help me (Return being typed to Return(arg0 int)) the second snippet will not. Since Times(1) returns *gomock.Call (which is not type-safe) this will result in Return not being type-safe (it will be typed to Return(rets ...interface{})). This issue and related PR resolves that.

Of course, I could just rewrite the test to always run Times(1) last and that would resolve the type-safe issue. Also, this change introduces more (generated) code and I'm not sure how big of an issue that is.

It's worth noting that if Times is generated as type-safe then AnyTimes, MinTimes and MaxTimes should also be generated as type-safe. But that's for the future.

@tchung1118
Copy link
Contributor

Thanks for the detailed explanation! I'll bring this up to the team.

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

Successfully merging a pull request may close this issue.

2 participants