mock icon indicating copy to clipboard operation
mock copied to clipboard

Add `Once` and `None` for calls.

Open nstandif opened this issue 10 months ago • 1 comments

Summary

Provide convenience methods for calls happening only once or none. This is frequent pattern in assertions and would make testing more readable.

Details

Presently, gomock includes func (*Call) Times(int) to handle exactly once or none. There are several reasons why this is an antipattern.

First, it enforces author intention. Changing Times(0) to Times(1) requires only changing the parameter. This a very similar argument to == vs =, which causes issues to this day. Looking at something like Once or None is very helpful for reviewers of code, since they can see clearly the test author intended it to do.

Second, it is easy to get lost in the context when scanning over a complicated test.

For example:

// initialization logic
// ...
foo.EXPECT().Run(gomock.Any()).Return(context.Canceled).Times(1)
baz.EXPECT().Run(gomock.Any()).Return(context.Canceled).Times(2)
fizzBuzz.EXPECT().Run(gomock.Any()).Return(context.Canceled).Times(0)

Here it is important to know that baz is called exactly twice, but the repetitive calls on foo and fizzBuzz make this less emphasized. When using Once() or None(), the intention of the author is very clear.

Third, it makes the line long: Times(n) is eight characters. Having fewer characters is helpful when working with inlining parameters. Once() And None() are six characters.

Proposed solution

foo.EXPECT().Run(gomock.Any()).Return(context.Canceled).Times(1)
baz.EXPECT().Run(gomock.Any()).Return(context.Canceled).Times(2)
fizzBuzz.EXPECT().Run(gomock.Any()).Return(context.Canceled).Times(0)

becomes

foo.EXPECT().Run(gomock.Any()).Return(context.Canceled).Once()
baz.EXPECT().Run(gomock.Any()).Return(context.Canceled).Times(2)
fizzBuzz.EXPECT().Run(gomock.Any()).Return(context.Canceled).None()

nstandif avatar Feb 14 '25 17:02 nstandif

Isn't this already in place?

  • None - Calling a mock without a pre-existing expectation results in an error?
  • Once - That's the default option if you don't pass in Time(1).

If anything, I would argue that Times(0) seems to be an anti-pattern here (?).

rabbbit avatar Feb 16 '25 02:02 rabbbit