uber-go / mock

GoMock is a mocking framework for the Go programming language.
Apache License 2.0
1.81k stars 103 forks source link

Generate typed Times for mocks #55

Open christofferjerrebo opened 11 months ago

christofferjerrebo commented 11 months ago

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

tchung1118 commented 11 months ago

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

christofferjerrebo commented 11 months ago

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.

tchung1118 commented 11 months ago

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