Mock objects are the cancer slowly killing your unit tests
You start working on a project that has been going on for some time now. Obviously, your first stop is to open the unit tests and try to understand what this is all about. You gasp in horror when you open the first test and your eyes land on this:
In this second post in my series on testing, we explore what I think is the modern cancer of unit tests, mock objects. I’ve seen tests like this one and much worst too many times in the past. Is this a bad test? Most people would say no, but I would definitely say YES! I’m going to try to lay out a couple of reasons why.
The first obvious issue is the test name, “testBackup”. This name doesn’t give us any clues as to what we are testing (besides backups, which is obvious). A better name could be something like whenStoringABackupPassedTheWarningThreshold_thenTheAStorageEntryIsCreatedAndAnAlertIsSentByEmail. I can already hear you say:
Seriously man, this is way too long for a test name
This is indeed a very long name, but this is mostly due to Java and jUnit. You have to use a method name as a test name when doing jUnit, which will result in long test name. But seriously, who cares? You’re never gonna type this method name, you’re just gonna read it, so it’s better if the name gives a lot of context.
One cool solution would be to use nested inner classes for preconditions so that the GivenABackupPassedTheWarningThreshold would become an inner class name, and the test name would be itShouldCreateAStorageAndSendAnAlertByEmail. This is better and more readable. For more information about nested runner with jUnit, check out this article from Petri.
The second obvious issue is that we are testing two things at once. Given the name of our test, “it should create a storage, and it should send an alert by email”. This should be two separated tests. More on that later …
The third issue is the list of verify statements at the end of the test. Imagine that you change something in the code, run all the tests and BOOM this test fails in one of the verify. What do you do? You read the test and try to understand the problem, but honestly, I doubt that anyone could figure out what’s wrong just looking at the test, not even the original author of the code, and even more if a couple of days have passed. So why is this bad? Because there is a clear lack of intent in what we are asserting. In essence what the author is doing is asserting the flow of operations happening in the method we are testing, and this is the main issue here.
Ideally, a good unit test should know as little as possible about the unit under test. It should know the inputs and outputs, but it should ignore as much as possible what happens in between and treat the unit under test as a black box.
There are two sources of inputs, method inputs, or dependencies.
Inputs to the method under test fall into two categories.
You either have to pass some basic value, a number, a string… in that case it’s a no brainer.
A method can also require passing an object. If the object is easy to build, using a constructor or such, it’s fine, but what if you have to create an object that itself requires other objects and when you’re done you had to create 6–7 different objects. Due to this, you will need to create stubs. Stubs are fake objects that are intended to represent hardcoded data/behavior.
But even if you can use stubs to fake a complex parameter, should you? In my opinion, having to pass a very complex object as parameter is a code smell.
- The first case when this happens is that you pass an object to a method, but the method only needs a single value or a very limited range of values from the object you’re passing. So instead of requesting the whole object, your method should just ask for the values it needs. If your methods have to dig down inside a parameter to extract a value they need, you’re violating a design principle called the law of demeter.
- The second case is that your method really needs the complex object. In this case, my solution would be to split the method into multiple components, unit test each component, and then create an integration test for the big method.
The pain of having to build and maintain a test for a method with complex input is not worth the gain as the test will likely be brittle and will require recurring maintenance.
Dependencies are the second type of inputs for a unit under test. A dependency is an object that the method under test is using to perform the computation, but that is not passed directly as a method argument. Again, the solution for this is a stub or a mock. If the dependency is only used as an input for the unit under test, it should be a stub. If the dependency is a targeted side-effect of the method (like in our example, the mail service), then it should be a mock, so that the side-effect can be asserted at the end of the test.
Stubs vs Mocks: Both are fake objects, but the difference is that a mock is used to assert a behavior, whereas a stub is there to fake data or computation. It’s unfortunate that some test frameworks like mockito don’t enforce the distinction between them. With Mockito, everything is a mock, but in reality, some mocks are used as stub.
There are three types of outputs from a unit under test.
Method return value
This the simplest output of a unit under test. You call a method, it returns something, and you assert on that something. There is also a second case in that category, when the method throws an exception. Then you catch the exception and assert on it.
The second type of output is more subtle, it’s a state change. So I call a method (that would return void), and then the state inside the unit under test has changed. Then you need to assert on the unit to validate the new state. Sometimes there is no easy way to assert the change since the object may not expose any public method to extract the state. In that case you sometimes can use a mock or a spy, but again it’s not always possible.
IMHO, you should avoid void methods as much as possible. There are some cases where it makes sense, but not that much. If you follow the trend about functional programming, you know that mutable state is your enemy and you should try to minimize the use of it. It’s always easier to test something with clear inputs and outputs that to test a state change.
This is the final type of output, when your unit under tests calls something external, for example, a database, the filesystem to write something, a SMTP server to send an email, Facebook or Twitter to post something, or Twillio to send a SMS … stuff like that. To test that, you definitely need a mock to verify that the expected behavior has been triggered.
Black box testing
So back to black box testing, your tests should treat the unit under test as a blackbox. If we take the example from the beginning, we have a slightly complicated Backup object to create as a test input, but it’s not that complex, so let’s say it’s ok for now.
Then what are the outputs? The unit under test returns void, so there is no direct output. Then we assert that an email has been sent. This falls into the third category, external calls. And finally there seems to be a save of some kind. It could be a state change, but without looking at the code it’s hard to say.
Of the seven verify statements, only the last two are outputs that need to be verified. Everything else is internal logic to the method and should not be verified. Why? because it makes the test less resilient and more brittle. We want tests to break as little as possible for two reasons:
- because we are lazy, and nobody likes to re-open an old test to fix it.
- when we do have to fix a test, it costs time, time you could use to do something else, so even better if the tests don’t break in the first place.
The first thing you should do when writing a test, is ask yourself, what are the inputs and the outputs of this unit. Then you build the inputs, or break the unit into multiple sub-units if it’s too hard to build the inputs for the unit. Once you have the inputs, you execute the code and verify/assert on the outputs. And then you’re done. What you should be left with is a test that is short, easier to read (for you and for the next person who’s gonna have to understand it).
Mocks are a cancer
My title was pretty bold, so let me explains exactly why I dislike mocks. First there is a lot of blur around the term mock. Depending on the framework you’re using, it means different thing. If we apply the premise that a mock is something that is used to verify behavior, and a stub is something to fake input, then in a unit test, you should have only ONE mock, but as many stubs as you want. If you have more than one mock, it probably means that you’re testing multiple things in the same test.
Looking at the initial example, if we keep only the last two verify statements, we still have two mocks, and we should have two tests. The first test validates that, under the right condition, we save a storage object. The second test verifies that under the same condition, we send an email. Both tests use the same setup and the same stubs, but they should have different names as they both test something different. Using Petri’s method of nested inner class that I mentioned at the beginning, you can centralize the code to prepare the stubs for the two tests so that you don’t have to copy/paste the first test to write the second one.
The example I gave was pretty basic, but I’ve seen tests that had almost 100 verify statements. Theses tests are very very brittle, and they have no clear purpose, so they are almost impossible to understand. Then someday the test fails. The first time this happens, you take the time to understand the test, and why it fails, which could take several hours, but then it happens again and again every 3–4 months, and you quickly surrender. If you only have a couple of such tests, it’s not that bad, but if all your tests are like that, the time you spend fixing tests quickly skyrockets and you just surrender.
- Identify your test inputs and outputs
- Avoid hard to build inputs
- Limit the number of mocks in your test. You can use as many stubs as you want, but try to use a single mock per test
TDD is a nice way to write tests that help a lot in that regard. It allows you to create very testable design, and coding the test first makes you see the problems with testing your method before coding the implementation. If you see that an input is very hard to build in your test BEFORE writing the actual implementation, you will probably change your design to make it easier to create that input. Same goes for the outputs.
Maybe the third story of this unit testing series will be about TDD. Until then, think twice before mocking!
You can read the first article of this series here.