Exposing private state to enable unit testing



This article is part of the series of posts about unit testing anti-patterns.

Exposing private state

Last time, we talked about making private methods public in order to enable unit testing. It’s not the only way people expose implementation details to the outside world for unit testing purposes, though. Today, we’ll look at a similar anti-pattern: exposing private state.

Let’s look at the following example:

Here, a customer starts with the Regular status and then can be promoted to Preferred and enjoy the 5% discount on everything.

How would you approach unit testing the Promote() method in this case? The status field is private and thus is not available in tests, so there’s no direct way to verify the promotion.

A tempting solution is to just make the status field public and examine it after calling Promote(). After all, that’s the desirable side effect of this method, so why not just test that side effect directly?

That would be an example of this anti-pattern in action. Remember: always write unit tests as if they were a regular client of the SUT, don’t expose state getters solely for satisfying a test. In the above example, the status field is not observable from the outside world and thus doesn’t belong to the class’s public API. Exposing it means that you would expose an internal implementation detail.

How to do that properly then?

What you should do instead is you need to mimic the behavior of regular clients of this class. In this particular situation, it seems that they don’t care about the status field, otherwise that piece of information would be public. What they do care about is the discount the customer gets after the promotion.

So that is what you should aim at verifying too. You need to check two things here:

  • A newly created customer has no discount.
  • Once the customer is promoted, the discount becomes 5%.

This way, you are binding your tests to the observable behavior of the SUT, not its implementation details. Later, if your application code starts to use this status field (say, for reporting purposes), you will be able to also start binding to it in tests because it’d officially become part of the SUT’s public API. But not until that happens. Widening the SUT’s API surface for the sake of testability is not a good design decision.

Testing a set of collaborating classes

Additional state getters are also often introduced when verifying collaborations between classes. In order to check the effect of one class calling another, you could decide to make some piece of information on the receiving part public.

Let’s take this example:

Here, aside from other work related to the checkout process, the Order class needs to record the fact of purchase using another class – OrderStatistics. But how to verify it? Note that the _log field is private, so there’s no direct way for us to do that.

Again, a tempting solution in this situation would be to just make the collection of log entries public and bind to it in tests.

Another possible way to fix this problem is to mock OrderStatistics out. You could introduce an interface for it, create a runtime wrapper using one of the mocking libraries and check that Order calls LogOrder.

Both of these decide decisions are suboptimal. The first one is because you would expose the internal implementation detail – the private field. And the approach with mocks – because you would bind to another implementation detail: the way the domain classes communicate with each other.

When it comes to mocking, it’s preferable to mock only collaborations between bounded contexts, not collaborations inside of one. Here I wrote more on this topic: When to use mocks. In fact, excessive mocking is often the root cause of the so-called test-induced design damage. You can read about why it is so in my review of the GOOS book.

So, what would be the solution to this one? How to verify that Order correctly logs the amount of spending?

The best way to approach this problem is to break the coupling between Order and OrderStatistics altogether by introducing a mediator between them.

Here’s the initial version once again:

Instead of calling OrderStatistics directly, modify Order so that its CheckOut method returns the customer Id and the money amount. Then, use the mediator to pass those results further to OrderStatistics:

This way, the problem of unit testing the Order class vanishes. You just verify the result of the operation (along with other side effects of the checkout process), and that’s it. No need in complicated mock machinery or exposing the private state.

That’s by the way generally a good approach to programming. Try to implement any feature in such a functional manner: as a sequence of operations. It would greatly simplify the code itself, as well as the unit tests covering it.

Summary

  • Exposing private state in order to enable unit testing is an anti-pattern.
  • Instead of exposing private state, mimic the regular clients’ behavior in tests and bind the SUT’s public API.
  • In case of collaborating classes, don’t expose private information either. Break the coupling apart and verify each class independently.

If you enjoy this article, check out my Pragmatic Unit Testing training course, you will love it too.

Other articles in the series

 

Share




  • Adrien

    I’m a bit baffled by your last point. The issue is how to test some code and your answer is to just put it in another class and not test it ? Because I mean, the issue on how to test that order is logged is still not solved at all. That was the original point.

    • http://enterprisecraftsmanship.com/ Vladimir Khorikov

      and your answer is to just put it in another class and not test it ?

      That’s correct.

      I know it sounds like this advice doesn’t help solve the original problem, but the thing is, the root cause of that problem lies in the way the code is organized, not it the way the tests verify it. This is a violation of the SR principle in action. The Order class should make a decision of what to log in terms of the customer and their purchase, but the actual effect of that decision (mutating the OrderStatistics class) is not its responsibility anymore.

      • Bartłomiej Kalinowski

        OK, Order class is now easier to test. What about the mediator? How do you plan to test it? (let’s assume logging is essential part of business logic, and we need to make sure that each order is logged properly)

        • Mheevun

          IMHO, the Order class need an integration test not a unit test. This integration test might use the real business data or mock data as input and verify the correctness of flow and the output (i.e. logging). Therefore the main benefit of this solution is separation of class responsibility. The Order calculate the order. The OrderStatistics save the order. The mediator ensures the flow of business logic is correct. Please correct me if i’m wrong. BTW i come with this conclusion because of this post: https://medium.com/javascript-scene/mocking-is-a-code-smell-944a70c90a6a.

          • Bartłomiej Kalinowski

            Thanks for the link, really interesting reading. I assume the mediator is considered an imperative way to achieve trivial composition of separated functionalities (thus the split improves both design and testability). Good point.

        • http://enterprisecraftsmanship.com/ Vladimir Khorikov

          @mheevun:disqus is right, you can test the mediator with an integration test. When doing so, use mocks to verify communications with external systems, and real data otherwise. More on this topic:
          http://enterprisecraftsmanship.com/2016/10/19/when-to-use-mocks/
          http://enterprisecraftsmanship.com/2016/06/21/pragmatic-integration-testing/

          • Bartłomiej Kalinowski

            OK, but I am worried about scalability of this approach. What if I now want to use this mediator as a building block in a higher level service (say, batch Order processing)? Should I test both mediator and the service with integration tests only? Or rather declare it “external” to somehow limit complexity of these integration tests?

          • http://enterprisecraftsmanship.com/ Vladimir Khorikov

            The mediator and the other service won’t contain much of domain logic and so they are not worth testing in and of themselves, only to check the integration between all components of the system. If the mediator stops being the most high-level component, don’t test it, test the higher level service instead.

  • http://www.draganstepanovic.com Dragan Stepanovic

    Don’t introduce mock, but introduce mediator 🙂 No real difference. One level of indirection added.
    Also, asserting on the return value is the same thing as asserting on the message being forwarded further.
    If testing forwarded message means exposing private details, asserting on the return value (which is exactly the same message) also means exposing private details.
    Message passing is object oriented style, while code with mediator is rather procedural.

    • http://enterprisecraftsmanship.com/ Vladimir Khorikov

      The difference is in how valuable the tests are in both scenarios. Asserting the communication pattern between domain classes with mocks results in false positives more often than asserting the return value, just because there are two responsibilities involved: deciding what the side effect should be and incurring that side effect.

      • http://www.draganstepanovic.com Dragan Stepanovic

        Message that is passed further (method args) is exactly the same message that is returned back (tuple in your case). If asserting the message passed further results in false positive, the message returned back to the caller will result with false positive as well. No difference there.

        If you interpret passing the message further as a side effect then returning back the same message to the caller can also be interpreted as a side effect; In the example given above, CheckOut method has two responsibilities: 1) calculate total amount and 2) pass back the message containing customer Id and total amount.

        Also, message passing doesn’t have to imply two responsibilities you’ve mentioned. Given method can publish that it finished processing while incurring side effect can be responsibility of some other class that gets notified on that event (event in general terms).

        One of the disadvantages of approach with returning back the message, which is not often mentioned, is that you’re forcing the calling method to play two roles (effectively strongly coupling two roles): invoker of the processing and receiver of the result, which is not the case with message passing since the calling method only plays the role of the invoker.

        • http://enterprisecraftsmanship.com/ Vladimir Khorikov

          This is basically the discussion about mockist vs classicist / OO vs functional approaches. I agree that in this particular example it doesn’t make a lot of difference to introduce the mediator as the code itself is simple enough (except for you don’t need to use mocks after the refactoring). However, this adds up in large projects. The ability to test Order without involving OrderStatistics and without using mocks goes a long way towards simplifying both the code base and the test suite. Example: http://enterprisecraftsmanship.com/2016/07/05/growing-object-oriented-software-guided-by-tests-without-mocks/

          • http://www.draganstepanovic.com Dragan Stepanovic

            Thanks, I’ll leave my response there.