Interfacing application code with unit tests

Question

I am working on a project in which we have to implement and unit test some new module. I had a quite clear architecture in mind so I quickly wrote down the main classes and methods and then we started writing unit tests.

While writing the tests we had to make quite a few modifications to the original code, such as

• Making private methods public in order to test them
• Adding extra methods to access private variables
• Adding extra methods to inject mock objects that should be used when the code runs inside a unit test.

Somehow I have the feeling that these are symptoms that we are doing something wrong, e.g.

1. the initial design was wrong (some functionalities should have been public from the beginning),
2. the code was not designed properly for being interfaced with unit tests (maybe due to the fact that we started to design the unit tests when quite a few classes had been designed already),
3. we are implementing unit tests the wrong way (e.g. unit tests should only directly test / address the public methods of an API, not the private ones),
4. a mixture of the above three points, and maybe some additional issues I have not thought about.

Since I have some experience with unit testing but I am far from being a guru, I would be very interested to read your thoughts regarding these issues.

Beside the above general questions, I have some more specific, technical questions:

Question 1. Does it make sense to directly test a private method m of a class A and even make it public in order to test it? Or should I assume that m is indirectly tested by unit tests covering other, public methods that call m?

Question 2. If an instance of class A contains an instance of class B (composite aggregation), does it make sense to mock B in order to test A? My first idea was that I should not mock B because the B instance is part of the A instance, but then I started to doubt about this. My argument against mocking B is the same as for 1: B is private wrt A and only used for its implementation, therefore mocking B seems like I am exposing private details of A like in (1). But maybe these problems indicate a design flaw: maybe we should not use composite aggregation but a plain association from A to B.

Question 3. In the above example, if we do decide to mock B, how do we inject the B instance into A? Here are some ideas we had:

• Inject the B instance as an argument to the A constructor instead of creating the B instance in the A constructor.
• Pass a BFactory interface as an argument to the A constructor and let A use the factory to create its private B instance.
• Use a BFactory singleton that is private to A. Use a static method A::setBFactory() to set the singleton. When A wants to create the B instance it uses the factory singleton if it is set (the test scenario), it creates B directly if the singleton is not set (the production code scenario).

The first two alternatives seem cleaner to me, but they require changing the signature of the A constructor: changing an API just to make it more testable seems awkward to me, is this a common practice?

The third one has the advantage that it does not require changing the signature of the constructor (the change to the API is less invasive), but it requires calling the static method setBFactory() before starting the test, which is IMO error-prone (implicit dependency on a method call for the tests to work properly). So I do not know which one we should choose.

Answer 1

I think testing public methods are enough most of the time.

If you have a great deal of complexity in your private methods, then consider putting them in another class as public methods and use them as private calls to those methods in your original class. This way you can ensure both methods in your original and utility classes work correctly.

Relying heavily on private methods is something to consider about design desicions.

Answer 2

To Question 1: That depends. Typically, you start with unit tests for public methods. Sometimes you encounter a method m you want to keep private to A, but you think that it makes sense to test m in isolation either. If that's the case, you should either make m public or make the test class TestA a friend class of A. But beware, adding a unit test for testing m makes it a little bit harder to change the signature or the behaviour of m afterwards; if you want to keep that an "implementation detail" of A, it may be better not to add a direct unit test.

Question 2: (C++ build-in) composite aggregation does not work well when it comes to mock out an instance. In fact, since the construction of the B happens implicitly in the constructor of A, you have no chance to inject the dependency from outside. If that is a problem depends on the way you want to test A: if you think it makes more sense to test A in isolation, with a mock of B instead of B, better use a plain association. If you think you can write all the needed unit tests for A without mocking out B, then a composite will probably be ok.

Question 3: changing an API to make things more testable is common as long as you don't have much code so far relying on that API. When you are doing TDD, you don't have to change you API afterwards to make things more testable, you start initially with an API designed for testability. If you want to change an API later to make things more testable, you may encounter problems, that's true. So I would go with the first or second of the alternatives you described as long as you can change your API painlessly, and use something like your third alternative (remark: this works also without the singleton pattern) only as a last resort if I must not change the API under any circumstances.

About your concerns that you might "do it wrong": every big engine or machine has maintenance openings, so IMHO the need of adding something like that to software is not too surprising.

Answer 3

You should look up Dependency Injection and Inversion of Control. Misko Hevery explains a lot of it on his blog. DI and IoC is effectively a design pattern for making code that is easy to unit test and mock.

Question 1: No, do not make private methods public to test them. If the method is sufficiently complex, you can create a collaborator class that contains just that method, and inject it (pass into the constructor) into your other class. ¹

Question 2: Who constructs A in this case? If you have a factory/builder class that constructs A, then there is no harm in passing the collaborator B into the constructor. If A is in a separate package/namespace from the code that uses it, you can even make the constructor package-private and make it so the factory/builder is the only class that can construct it.

Question 3: I answered this in Question 2. Some extra notes though:

• Using a builder/factory pattern allows you to do dependency injection as much as you want, without having to worry about making code using your class difficult to use.
• It separates object construction time from object use time which means that code using your API can be simpler.

¹ _{This is the C#/Java answer - C++ may have extra features that facilitate this}

As an answer to your comments:

What I meant was that your production code would be changed from (please forgive my pseudo-C++):

void MyClass::MyUseOfA()
{
  A* a = new A();
  a->SomeMethod();
}

A::A()
{
  m_b = new B();
}

to:

void MyClass::MyUseOfA()
{
  A* a = AFactory.NewA();
  a->SomeMethod();
}

A* AFactory::NewA()
{
  // Construct dependencies
  B* b = new B();
  return new A(b);
}

A::A(B* b)
{
  m_b = b;
}

Then your test can be:

void MyTest::TestA()
{
  MockB* b = new MockB();
  b->SetSomethingInteresting(somethingInteresting);

  A* a = new A(b);

  a->DoSomethingInteresting();

  b->DidSomethingInterestingHappen();
}

In this way, you don't need to pass the factory around, the code calling A doesn't need to know how to construct A, and the test can custom-construct A to allow behaviour to be tested.

In your other comment, you asked about nested dependencies. So, for example, if your dependencies were:

A -> C -> D -> B

The first question to ask is if A uses C and D. If they aren't, why are they included in A? Assuming they are used, then perhaps it is necessary to pass in C in your factory, and have your test construct a MockC that returns a MockB, allowing you to test all possible interactions.

If this is starting to get complicated, it might be a sign that your design is perhaps coupled too tightly. If you can loosen the coupling and keep cohesion high then this kind of DI becomes easier to implement.