VerifyAll Should Report All Failures

[adamhewitt627]

Right now, VerifyAll throws at the first Mock with failures. Ideally, we throw an AggregateException instead, although that does affect any code that expects MockException from our verification. See #35.

[stakx]

@adamhewitt627: Visiting from moq/moq4. I'm currently doing some work on Verify / VerifyAll for the next 4.x release of Moq. Properly aggregating all verification messages, instead of stopping at the first error, is generally feasible. Two concerns though:

• This would generally result in longer error messages. Is this actually helpful in practice, or not? I could imagine that some unit tests might be written such that later verification steps don't even make sense when earlier ones are unsuccessful (but I could be wrong about that). In such cases, longer messages might be more confusing rather than more helpful.

• Like you said, throwing an AggregateException that wraps one or more MockExceptions all of a sudden might break existing code. While one isn't supposed to catch verification exceptions, there might be code that does just that.

  For this reason, Moq is currently just concatenating all error messages into a single MockException. Not exactly pretty, but it works. (Would this work for you?)

[adamhewitt627]

I think that would be the goal. However, since we are looping through multiple Mocks, your concatenation is only part of the solution. For our VerifyAll to report all failures, we would need a way to construct a MockException from multiple MockExceptions.

[stakx]

Moq v4 has an internal method for combining several MockException instances into a single MockException (see here), which could be exposed as a public static method MockException.Combine (or similar). Would that meet all your needs?

Making said method public would be a somewhat unusual addition to Moq's API, therefore I'd like to ask how you see the present issue's overall importance / priority?

[Keboo]

@stakx I will let @adamhewitt627 chime in as well, but I suspect MockException.Combine would likely suffice.
I am curious about exposing the constructors of MockException. By leaving everything internal/private it prevents extensibility. Given the code, I assume this was an explicit design decision. Is there any chance of revisiting that in the future to consider exposing constructors?

[stakx]

@Keboo - There should indeed be differences between Moq v4 and v5:

• Moq v4 is generally not very extensible—whether by intention or accident, I cannot say for sure. Its API consists of mostly concrete classes that are rather strongly coupled, and that cannot be easily changed. Regarding MockException in particular, it would be possible to make it subclassable, but it'd take some work cleaning up its contract first; MockException.Combine would be the easy way out (if perhaps not the most elegant or sensible solution).

  There is also this comment in the moq4 code base:

  Moq does not provide a richer hierarchy of exception types, as tests typically should not catch or expect exceptions from mocks. These are typically the result of changes in the tested class or its collaborators' implementation, and result in fixes in the mock setup so that they disappear and allow the test to pass.

  If you subscribe to this logic, then there'd be no reason to insist on throwing a MockException—throwing an AggregateException or any other exception type of your choice would be just as well.

• Moq v5, OTOH, has been designed from the start with proper composability in mind, it should be much more extensible and allow for better introspection of mocks. See e.g. Moq v5's version of MockException which is currently declared public abstract and allows subclassing. I cannot say whether this is the main Moq exception you'd want to keep using, and whether it's going to stay that way. @kzu could tell you the full story.