There are some features in the isolation framework (Mocking framework) Moq which I see is used blindly. Most of the time it can which reduce test readability and maintainability. One of the worst offender in this regards is VerifyAll.
In this post I’m going to elaborate why this method is so pernicious to readability and maintainability of our test. I also show how it can make our test brittle and cause a lot of pain over time in terms of maintainability of tests.
VerifyAll Violates Test One Thing At A Time Principle
One of the most important principle when we test a piece of code is our code should only fail for one reason. That means it’s OK to have multiple asserts per test as long as all of those are in line to test one thing. But a lot of times VerifyAll used at the end of the test after we asserted some kind of return value. Let me show you what I mean by an example.
As you can see above, this method first verify that all the expectations that we set up for our fake instances are actually met. Then it’s asserting the return value of the method on our sut. Also we are assuming that this verification of expectation is necessary. Which is not the case in this instance.
VerifyAll Unnecessarily Test All the Interactions
The most important problem with this test is not that it asserts multiple things in a test. But it is the fact that when we have a return value, we shouldn’t test for interaction. Interaction testing is really frowned upon by people such as Roy Oshrove because it makes our test brittle. It must be use as he puts it as last option. That’s because we should not care how our method do something, in other word we should not test the internal implementation of the method. We should only measure the result of what that method is doing. If for some reason we could not measure that directly, we measure it through testing the interaction.
Maybe it was acceptable to use VerifyAll if and only if we are testing a process that employ multiple calls to others methods. Also the method that calls those methods wasn’t returning anything, something like below.
But then again I would say that is not very clear. We can write more readable tests which are far more intention revealing than this. Unfortunately Moq framework make things worse as far as readability is concerned. That’s why I think frameworks such as FakeItEasy or NSubstitute are superior. Another problem with this unit test is that it uses moq’s Setup to test the interactions. When in fact we can just pass those values into the verify method, instead of using the Setup method like this.
VerifyAll Is Not Readable and Clear
Unit test should test our code, but another important function of unit test is that it could act as specification and show what our code actually suppose to do. I quote from an article.
The Unit Test Is The Specification
UnitTests codify the requirements, what is wanted, extremely well. A program may pass the UnitTests, in which case it is to specification. Or it may fail the UnitTests, in which case it is not to specification (bugs in the ‘specification’ notwithstanding).
First of all we should avoid testing interaction if we can. Because most of the time testing the internal workings of the method is not necessary. It can makes or Unit tests inflexible to change and with a smallest changes to the method, we have a lot of failed unit tests on our hands. That can be demotivating for teams who are under time pressure to deliver. So what happens in those cases? They abandon the unit tests.
Here’s an example where I’ve assumed that we have to test the interactions. Here the test is more readable and we distinctly separated the use of setup when stubbing and the use of verify when we test an interaction with a mock object. It’s even reflected in the names, I called one instance mediatorMock and another one which will be used for stubbing
campaignServiceFake. Another thing to take notice of is that I didn’t use Setup method for testing any expectation. Also note that our method here doesn’t return anything. So I have to test the interaction because that’s the only thing I can test in this case.
Summary
In this post I explained why using VerifyAll is not a good idea. I went into more detail as to why that is. Also we saw some examples of how we can change our test to make it more resilient to change and readable. Hopfuly I could convince you to not use VerifyAll most of the time.
Nice article. I’ve worked on codebases that both use Verify/VerifyAll and those that don’t. I can see the benefits as well as the maintainability issues. So, I agree to an extent but the idea that using Verify/VerifyAll makes the tests more brittle is more or less irrelevant since the internals of the methods being tested are already “exposed” because of the need for the Mock setups.