SetupSequence: CallBase Support for Void methods

[mpareja]

As an outsider looking in at the source, it would appear to me that it ought to be pretty easy to add CallBase support when setting up a Sequence on a void method.

Relevant code:

• void SetupSequence missing CallBase
• equivalent code for non-void

Can you see any reasons why this isn't/shouldn't be supported?

[stakx]

it ought to be pretty easy to add CallBase support when setting up a Sequence on a void method.

Agreed, it should be fairly straightforward.

Can you see any reasons why this isn't/shouldn't be supported?

From a user's perspective: The enhancement would appear to be useful. But since with void methods, .CallBase() would only be interesting for causing side effects, it should probably be accompanied by a .Callback() method to allow for side effects in a more general manner.

If this feature is really important for you, and you submitted a PR, then I'd probably merge it. Update: As recently discussed on our Discord server, with v5 once again just around the corner, I'm considering a feature freeze for Moq v4. Please don't submit a PR at this time.

From a maintainer's perspective: I must confess that I do not like the .SetupSequence and .Protected().Setup set of APIs at all. They get less attention than the main setup API and have apparently been treated a little like an afterthought. Bringing them to feature parity with the main setup API doesn't happen in a concerted effort, but in a piecemeal manner, which ends up feeling like neverending feature creep to me. 😃

For example, doing what I suggested above (adding .Callback along with .CallBase) will mean that people are next going to ask for .Callback overloads that allow accessing the invocation arguments (similar to #1048). While this is a justified request, it also means we'll have to add tons of repetitive overloads to the codebase (think something like this and this). Also, people will start wondering why there is a .Callback for void methods but not for non-void ones. (The reason would be that unlike with the regular setup API, one "verb" stands for one invocation in the sequence, so you cannot have both a Callback and a Returns/CallBase, the latter of which is required for non-void setups.)

As a maintainer, I'd much prefer if we didn't have to continue that neverending feature catch-up game. It's my (possibly unrealistic) hope that we'll one day be able to refactor Moq such that the same fluent API works on top of any type of setup (regular, sequential, protected) so we can get rid of all that code duplication. (That has actually happened for protected setups via .Protected().As<T>(), a solution for sequential setups is still pending.)

[Gamecock]

A use case that does not require .Callback is to test retry logic on a void function so you could SetupSequence:
.Throws().Throw().Succeeds()