MattHicks.com

Programming on the Edge

Mocking should be Mocked

Published by Matt Hicks under , , , , , on Wednesday, February 25, 2015
I've worked with and for a lot of companies over the years and with the ones that actually care about unit testing Mocking (Mockito or some other variation) typically quickly enters the scene. My argument is that in proper coding you should be able to write proper unit tests without any mock objects. I'll go a step further and say that encouraging mocking in your tests actually encourages poor code quality.

What's the purpose of Mocking?

Mocking is extremely common in unit tests, but why?  The basic idea is that you are attempting to test one specific "unit" of functionality, but you don't want calls to the database, third-party calls, or other side-effects creeping into your unit tests. On the surface this seems like a valid use-case.

Why is Mocking bad?

There are many problems related to mocking that I'll try to quickly step through.

Unnecessary Complexity and Decreased Modularity

Though the purpose of mocking is actually to reduce complexity it very often increases the complexity of your unit tests because you often end up with confusing and complicated mocking to avoid running aspects of your code that you don't want executed in your tests.  You often end up pulling your hair out trying to figure out how the internals of the system are supposed to work in order to get the mocks to operate as expected and then all your tests start breaking as soon as that internal logic changes.  You often end up in a scenario where your code works fine because it's written modularly to continue working but your unit tests break because they are essentially implementing the internal functionality via mocks.

Unit tests testing the Mocking framework

In extremely complex unit tests or even simpler unit tests that are managed over a long time or through many developers it becomes more and more confusing what the purpose of the test actually is. I can cite dozens of examples where I've been auditing unit tests only to find out that the unit test does nothing but test the mocking framework instead of the code.

Need for Mocking Represents Bad Code Separation

This is what my entire complaint really boils down to.  If you actually need mocking in your unit tests, your actual code that you're testing is poorly written in the first place.  You should be writing code in proper units to allow individual testing without the need for mocking at all.

For example:


You might have a scenario similar to the code above and you want to test the checkout functionality of your system, but you don't want the payment gateway to be called and you don't want it actually persisting to the database so you might mock those out to avoid those being called. Instead, consider the idea that there are four units of functionality clearly defined in this case:
  1. Validating the Order
  2. Calling the Payment gateway
  3. Persisting the information to the database
  4. Creating a Receipt
Herein lies the problem. You have tight coupling in your code of dissimilar things making mocking necessary. If instead you were to refactor your code to look more like this:


You can see now that each unit of functionality has been broken out into its own method that can be individually tested as needed without the necessity of mocking. Further, the code is significantly cleaner and more maintainable as a result of this refactoring. Not only does it make our unit tests cleaner and easier to maintain without mocks, but it also enforces a higher level of code quality through modularity in your code than was there before.

If you ever decided that the checkout method needed to be unit tested you could refactor it so it takes in a PaymentGateway implementation and Persistance implementation as arguments to avoid side-effects and allowing a test instance to be created to fulfill those needs. Though this is a form of mocking, it follows a better paradigm of consistency and modularity than mocking frameworks do.

As you can see, this is an all-around better way to write code and though it is understood that in existing projects mocking still may be necessary, there is no excuse for such to be necessary in new code.

5 comments:

Alexandre Delattre said... @ February 27, 2015 at 4:04 AM

Hello,

While I agree mocks can be over-abused, I think you are missing a valid use case of mocks, which is the ability to perform real isolated unit tests.

Basically, in your example there is not 4 but 5 units of responsibility to test, if you count the "glue" logic which is the checkout method itself.

Such method would be hard to test in isolation, because calling it implies calling the 4 dependent units, which would perform an integration test. That's where mocking become useful, because you can test this glue logic (and only this logic) by scripting the dependency.

For instance, you would be able to test such scenario as :
- given an invalid order, ensure the gateway is not called
- given a valid order, and a valid payment response, ensure a receipt is created.

To allow mock/real implementation to be switched, you can use constructor based dependency injection with single-method traits for representing the responsibility (ie refactoring the 4 methods in 4 traits with single method).

>> Need for Mocking Represents Bad Code Separation

I would say, on the contrary, that in a properly separated code, mocking allows you to test "glue" logic and interaction between layers more easily.

Matt Hicks said... @ February 27, 2015 at 9:22 AM

Alexandre, I responded to this at the end of my post. The method could take in functions that could provide alternative implementations if you want to test the glue. However, I would argue that the glue should be kept as simple as possible and not need to be tested in most cases, but rather the "work" which can be broken out into units.

I also have a problem with dependency injection, but that's an article for another time. :)

I still say in 100% of new code scenarios there are better alternatives to mocking.

Alexandre Delattre said... @ February 27, 2015 at 11:08 AM

Matt,

Whether or not you should test the glue is another interesting question!

I agree with you that you should the glue code as simple as possible, making it not worthwhile testing, in most occasions, especially for small projects.

But sometimes you HAVE to test the glue (for instance the glue logic being a critical piece of code, or you want to achieve 100% line coverage, etc ...), and that is where mocks shine.

So saying in 100% of new code, you should not use mock sounds incorrect to me. Like any piece of technology, you should use it sparingly knowing the pros and cons etc ...

Regarding Dependency Injection, I tend to stick with constructor based DI (I think it is great in both object and functional terms), with a manual or lightweight container (macwire) and found the produced code very clean and testable :)

Regards

Matt Hicks said... @ February 27, 2015 at 11:48 AM

Alexandre, I do appreciate your opinion on this, and I understand how you would come to this conclusion, but I still believe that mocks should never be used in tests with new code.

In my sample code I had this signature:

def checkout(order: Order): Receipt

If I really want to make this method testable I would change it to look more like this:

def checkout(order: Order, paymentGateway: Order => GatewayResponse, persistence: (Order, Payment) => Unit): Receipt

I might even create default arguments (if I were using Scala) so that only in the exceptional case (tests) would those additional args need be considered. Obviously it's not quite a elegant in Java (though 8 is a little nicer), but that gives you the ability to fully test the glue logic without compromising your tests with mocks.

This makes your unit tests far clearer and though there is some shared conceptual ideology between this and using mocks, the power of mocking is actually its biggest disadvantage.

In a perfect world mocks would be a viable option here, but in the real world developers are lazy and very rarely give tests the attention they deserve. As such, those tests often suffer because of the power that mocking provides.

Alexandre Delattre said... @ February 27, 2015 at 12:31 PM

What you are advocating sounds to me like call-site based dependency injection. Which is undoubtly a viable option. Reminds me a bit of functional approach of DI using the reader monad, an interesting read by the way.

The downside, I see, is that you have to have a modify the signature of checkout in order to make the API testable as an afterthought (which can be lowered by using default values or implicits)

For this reason, I tend to prefer the constructor based approach, where :
1) dependent components are glued at construction time (usually automatically with macwire for instance
2) each component call the other component methods without knowing their dependencies.

But this is more a styling / convention debate, both are viable, and this way of thinking is a better fit for my brain :)

Thanks for the discussion.

Post a Comment