Saturday, 2 June 2012

How to test private methods?

(Thanks go to Oleksandr Kravchenko for inspiration.)

This question gets asked over and over again. Actually, this is the question I've been asking myself for quite a long time.

Until I realized it's the wrong question to ask. So let's rephrase the question a little, rewind and start over:

What to do when you feel you need to test private methods?

Now, that's better :-)

Basically there are few ways to tackle this issue, depending on circumstances. I'll try to point them out starting from simplest.

1. Don't do it.

As Liz Keogh pointed out (using BDD language, but it applies to TDD as well), unit tests in TDD or BDD are examples of how to use objects of the class and how is this class valuable to other classes. As such, unit test (or unit-level specifications if you will) should go through the public interface only (with an exception, that I'm going to introduce later).

Let's consider a code example:

public class DishWasher
{
  private DishWashing dishWashing;
  private int capacity;

  public DishWasher(int capacity, DishWashing dishWashing)
  {
    this.dishWashing = dishWashing;
    this.capacity = capacity;
  }
  
  public void Wash(Dishes dishes)
  {
    if(IsEnoughPlaceForAll(dishes)
    {
      dishWashing.PerformOn(dishes);
    }
  }

  private bool IsEnoughPlaceFor(Dishes dishes)
  {
    return dishes.CountIsLessThan(this.capacity);
  }
}

Here, the private method is used solely to enhance readability. There's no need to specify it separately. We can easily write two specifications:

  1. Given I have a new set of dishes And the count of dishes is less than washer capacity, When I try to wash the dishes in the washer, Then it should perform the washing process.
  2. Given I have a new set of dishes And the count of dishes is equal to or greater than washer capacity, When I try to wash the dishes in the washer, Then it should not start the washing process.

And that's all - no need to excercise the private mehod, because from the specification point of view, it does not exist - the "washer capacity" thing is abstract enough to leave it in the spec.

However, let's look at what I did with the actual washing - I made it a separate class and passed its instance to the washer. That's interesting - why isn't washing just a private method of the DishWasher class?

2. Move a responsibility by Introducing a collaborator (TDD) or moving method (Refactoring)

First, let's look at it from TDD (and Test-First) point of view.

Usually, when doing TDD, we always think "outside the object", because the "inside" is always a result of failing spec/test and comes AFTER the spec. So, following the TDD process, we'd have no code and start with a specification in mind (let's remind it here):

Given I have a new set of dishes And the count of dishes is less than washer capacity, When I try to wash the dishes in the washer, Then it should perform the washing process.

Looking at this spec, we quickly run into conclusion that the behavior description of the dish washer ends with starting the washing process. We didn't go any further than that. How can we use this information? We can notice that there are actually two responsibilities involved.

The first one is maintaining the right functionality of controlling the device (here, we're dealing with making sure the washing does not start when too many dishes are put inside, but it will probably include other scenarios soon, like "do not start washing when washing is in progress" or "Only start washing when the washer is turned on", or "Stop washing when washer is turned off" etc.)

The other responsibility is the washing process, which is not started at the same time the washer turned on, so it is something more separate and has a logic of its own (e.g. how many phases are there, when to apply the detergent, how long does it take, how's the temperature of the water going to change along the washing process etc.).

As we discover, we did't specify the second part at all, so looks like for us, the responsibility of the washer ends with starting the washing process. Following Robert C. Martin's Single Responsibility Principle, it's good to move this washing process into a separate class. So, In TDD, you don't think about privacy or not - you think about _the_spec_ and it often tells you when you need to introduce a new collaborator by stressing different responsibilities. Private methods are introduced during the refactoring phase when you need to make some things a little more readable or eliminate redundancy.

Ok, now let's look from the refactoring perspective.

Imagine that you're dealing with a legacy code. All you have is the DishWasher class with the dish washing process coded inside as a private method. What you surely need to do is make sure that washing works OK by writing some unit tests that no one wrote ever before. In this case, it's usually useful to ask yourself a question: "Why do I think that I need to test the private method?". The answer is usually something like "because it has some complex logic, but well defined logic of it's own and I need to make sure that it works". Let's try to rephrase what you were trying to say by this (by the way, notice that a lot of this post up to now is about understanding your own reasoning :-) ). You actually said that "the washing is a separate responsibility and I have to make sure that it's fulfilled correctly". Well, if it's a separate responsibility, why is it in the DishWasher class? Quickly move the method into a new class and make it public! There - it's ready for testing! Wasn't THAT easy? :-)

Now, these two patterns (not testing and moving responsibilities) cover about 90% of cases. There is, however, a third case when you really want to test/specify private methods. Let's take a look at it now.

3. Use inheritance

(First, a disclaimer - I find this case very rare and usualy never run into it, so it was hard for me to find a good example. Please don't shout "hey, I can solve it by delegation instead of inheritance" - just bear with me :-)).

I'll start with the code, not the spec, since it's easier to understant the problem this way.

Let's say that you've got two models of dish washers. Each of them has a different policy for starting washing - the first one washes all dishes at once, and the seond one (a bigger one) implements a swapping functionality - it let's you put in twice as many dishes, then splits them into two chunks and performs washing for the dirst chunk, then the second chunk. Other than that, the washing process is the same. Let's say that we decided to make subclasses for different washers, overriding the "start washing" functionality. So, our DishWasher is now abstract and its Wash() method is implemented like this:

  public void Wash(Dishes dishes)
  {
    if(IsEnoughPlaceForAll(dishes)
    {
      PerformWashing(dishes, dishWashing); 
    }
  }

  protected abstract void PerformWashing(dishes, dishWashing);

Now, the "small" dish washer implements the method the following way:

protected void PerformWashing(Dishes dishes, DishWashing dishWashing)
{
  dishWashing.PerformOn(dishes);
}

...and the "bigger", two-chunk washer does it like this:

protected void PerformWashing(Dishes dishes, DishWashing dishWashing)
{
  var chunks = dishes.DivideIntoChunks(2);
  dishWashing.PerformOn(chunks[0]);
  dishWashing.PerformOn(chunks[1]);
}

We've got two classes now, each of them inheriting the Wash() method from DishWasher class and implementing a different variant of starting the washing process. Do specify the whole Wash() method in both of these derived classes?

No. It's the responsibility of DishWasher and should be specified for it. But it's abstract. What do we do?

First of all, we're gonna use a partial mock inheriting from DishWasher to specify DishWasher's behavior. The only thing we want to specify/test is that the abstract PerformWashing() method actually gets called in certain circumstances, because this is where the DishWasher's responsibility ends. We can achieve this easily either with a mocking framework or with a simple manual mock:

public class DishWasherMock : DishWasher
{
  public bool callCount = 0;
  public DishWashing passedWashing = null;
  public Dishes passedDishes = null;
  
  protected void PerformWashing(Dishes dishes, DishWashing dishWashing)
  {  
    callCount++;
    passedWashing = dishWashing;
    passedDishes = dishes;
  }
} 

In the spec, we only care about whether this method was called once with correct arguments and that's it.

Now the more interesting part - what to do with the protected methods in the subclasses? We're gonna specify them as well! Let's see, who is allowed to access protected methods? Derived classes! Ha! Why not make our little test suite class derived from specific dish washer to allow it to access its protected members? Let's see how it works for the "small" dish washer:

public class SmallDishWasherSpecification : SmallDishWasher
{
  [Test]
  public void ShouldJustStartWashing()
  {
    var dishWashingMock = new DishWashingMock();
    var anyDishes = Any.InstanceOf<Dishes>();
    this.PerformWashing(anyDishes, dishWashingMock);
  
    Assert.AreEqual(1, dishWashingMock.callCount);
    Assert.AreEqual(anyDishes, dishWashingMock.passedDishes);
  }
}

Note that ShouldJustStartWashing() is an instance method, so it has access to all public and protected instance methods of SmallDishWasher class.

"Bigger" washer follows the same patterns, so I'll leave it to you as an excercise. By the way, this technique works quite fine with classes that have protected constructors as well (although I'd never make such a class, but I know some folks do, e.g. in singletons or abstract classes).

But what if something is private? Just make it protected - maybe shed a small tear, but in the long run, it's no big deal.

The last 1%

This covers 99% of cases where you "need to" test the private method. The last 1% are some corner cases that are hard for me to even imagine. You can then use some of the "last resort techniques", like friends (C++), InternalsVisibleTo (C#), instrumentation/profiling API, reflection etc. but I really advise you to strongly consider the three options mentioned above before going that far.

Ok, that's it. Have a good weekend!

No comments: