mock icon indicating copy to clipboard operation
mock copied to clipboard

Generate typed Times for mocks

Open christofferjerrebo opened this issue 2 years ago • 4 comments

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 avatar Aug 08 '23 12:08 christofferjerrebo

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

tchung1118 avatar Aug 08 '23 19:08 tchung1118

Sure, I'll give it a go :smile:

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! :sweat_smile:

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.

christofferjerrebo avatar Aug 09 '23 09:08 christofferjerrebo

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

tchung1118 avatar Aug 09 '23 14:08 tchung1118