CallBase() is missing on ISetup<T>

[Nikkelmann]

I have a service method that calls a bunch of other methods in the service.
In this case I want to make a partial mock without using mock.CallBase = true; since I'm only interested in testing this one method in this unit test (I will have a unit test for each separate method).

Specifically in my case I want to verify that Method2ThatReturnsStuff was given the data from Method1ThatReturnsStuff:

// Arrange
var mock = Mock.Get(service);
mock.Setup(x => x.MethodThatReturnsVoid()).CallBase(); // <--- CallBase() is missing here
mock.Setup(x => x.Method1ThatReturnsStuff()).Returns(listOfStuff);
mock.Setup(x => x.Method2ThatReturnsStuff(It.IsAny<ICollection<Stuff>>()));

// Act
mock.Object.MethodThatReturnsVoid();

// Assert
mock.Verify(x => x.Method2ThatReturnsStuff(It.Is<ICollection<Stuff>>(t => t == listOfStuff)));

As you can see CallBase() is missing when the return type is void.

[stakx]

@Nikkelmann - I tend to agree that CallBase appears to be missing in this particular case. It'll need some additional looking into... perhaps there is (or was) a good reason why it's missing?

We currently have a freeze on Moq 4's API while Moq 5 is being made ready for publication so we cannot proceed on this right away. I'll check with @kzu whether we can add this anyway (since it appears to be a fairly obvious omission).

[Code-Grump]

CallBase is currently defined by the IReturns<TMock, TResult>, presumably because it involves the return value?

1. Is this the right place for this method? Should we consider moving it to its own interface/file, as with the other "verbs"?
2. Should this also be available for property setters? It currently doesn't seem to be, perhaps for the same reason as void-returning methods?
3. Where should the void-returning CallBase method be defined?

[Code-Grump]

No opinions on this one? If not, I'll put together a pull request on my immediate ideas.

[stakx]

@Tragedian, sorry for being non-responsive. I haven't quite found the time yet to review this properly. The thing is, there are several things that currently appear to be wrong in the Moq.Language and Moq.Language.Flow namespace and in the way MethodCall et al. implement these interfaces. I'd like to mentally straighten things out a bit before I write down an opinion, or we risk putting things in the wrong place once more. Sorry for being a bit slow on this.

[Code-Grump]

No rush. Just want to make sure we're making the best changes.

[stakx]

@Tragedian: I've been looking at Moq's fluent API interfaces last weekend, and things look rather messy. My current suggestion would be that we do the following:

• Introduce an interface Moq.Language.ICallBase that contains a single method Moq.Language.Flow.ICallBaseResult CallBase().

• Introduce a new interface Moq.Language.Flow.ICallBaseResult that inherits the same interfaces as Moq.Language.Flow.ICallbackResult. (The reason behind this is that CallBase for void methods has roughly the same purpose as Callback, i.e. providing the method implementation.)

• Let ISetup<TMock> inherit both ICallBase and ICallBaseResult (the latter because CallBase is optional and can be skipped).

• Implement the new CallBase interface method in MethodCall.

Alternatively, if we want to allow CallBase after a preceding Callback as with non-void methods, then we'd let ICallbackResult (instead of ISetup<TMock>) inherit the new ICallBase and ICallBaseResult interfaces.

Either of these options should be good enough for the next 4.9.1 release and not cause any breaking changes.

I am planning to clean up & straighten out the fluent API interfaces to give them a consistent and more readily understandable structure, but since that will introduce binary breaking changes I'd like to hold that off until 4.10.0.

[stakx]

Update: Please ignore this comment, I made a mistake. See #615 (comment) below.

---

@Nikkelmann - I've finally found some time to look into the missing .CallBase() setup method for void methods. And I've stumbled onto a possible roadblock. Citing from the commit message of 939f70f:

[The] current behavior of CallBase at the mock level [...] doesn't work for void methods at all.

This is a problem because it means that if we introduce a setup-level CallBase for void methods, it should be a no-op for consistency with mock-level CallBase. Any inconsistencies sooner or later cause "bug" reports. But at the same time, implementing setup-level CallBase as a no-op would be totally pointless.

When implementing setup-level CallBase, we will therefore have to choose one of four options:

1. Accept inconsistency between setup and mock-level CallBase.

2. Introduce a breaking change by making mock-level CallBase work for void methods, too.

3. Let clients choose between behaviours (1) and (2) through a new feature switch (Mock.Switches).

4. Don't introduce setup-level CallBase to avoid the problem.

I'm undecided which way to go. (1) bears the risk of the Moq team receiving lots of "bug" reports; (2) is the right solution from a purely technical point of view, but bears the risk of breaking user code. (3) is an easy way out, at the cost of avoidable API complexity. And (4) is another easy way out with the disadvantage of an incomplete API.

Any opinions on these four options, or do you see any other options?

P.S.: I'm going to default to "do nothing" (4) unless we find a good solution here.

[stakx]

@Nikkelmann - please ignore the above comment. Turns out that the tests were written incorrectly, mock-level CallBase does work for void methods, and so there won't be any inconsistent behavior when implementing setup-level CallBase.

[stakx]

@Nikkelmann, @Tragedian - I've just pushed Moq 4.10.0 (which includes voidMethodSetup.CallBase()) to NuGet. It should be available shortly.