The pImpl idiom and Testability

Question

The pImpl idiom in c++ aims to hide the implementation details (=private members) of a class from the users of that class. However it also hides some of the dependencies of that class which is usually regarded bad from a testing point of view.

For example if class A hides its implementation details in Class AImpl which is only accessible from A.cpp and AImpl depends on a lot of other classes, it becomes very difficult to unit test class A since the testing framework has no access to the methods of AImpl and also no way to inject dependency into AImpl.

Has anyone come across this problem before? and have you found a solution?

-- edit --

On a related topic, it seems that people suggest one should only test public methods exposed by the interface and not the internals. While I can conceptually understand that statement, I often find that I need to test private methods in isolation. For example when a public method calls a private helper method that contains some non trivial logic.

Whenas

To respond to your edit, if your "private" functionality is of sufficient complexity that it needs to be tested independently, then you should put it in its own module that you test independently. – Airplane

Answer 1

The idea behind pimpl is to not so much to hide implementation details from classes, (private members already do that) but to move implementation details out of the header. The problem is that in C++'s model of includes, changing the private methods/variables will force any file including this file to be recompiled. That is a pain, and that's why pimpl seeks to eliminate. It doesn't help with preventing dependencies on external libraries. Other techniques do that.

Your unit tests shouldn't depend on the implementation of the class. They should verify that you class actually acts as it should. The only thing that really matter is how the object interacts with the outside world. Any behavior which your tests cannot detect must be internal to the object and thus irrelevant.

Having said that, if you find too much complexity inside the internal implementation of a class, you may want to break out that logic into a separate object or function. Essentially, if your internal behavior is too complex to test indirectly, make it the external behavior of another object and test that.

For example, suppose that I have a class which takes a string as a parameter to its constructor. The string is actual a little mini-language that specifies some of the behavior the object. (The string probably comes from a configuration file or something). In theory, I should be able to test the parsing of that string by constructing different objects and checking behavior. But if the mini-language is complex enough this will be hard. So, I define another function that takes the string and returns a representation of the context (like an associative array or something). Then I can test that parsing function separately from the main object.

Entoblast

Implementation independent tests are ok for black box testing. But in white box tests I want to test the implementation details. Thus it matters to have access the internals of the class/object. – Feune

You can place 'private' functions/classes in an unnamed namespace. And then #include the source file directly in your test code. This works well if you want to avoid creating a header file just to test something. This idea is mentioned in the google-test docs – Rennin

Update to link provided by @Willem: Testing Private Code – Axinomancy

Answer 2

Why does the unit test need access to the internals of A's implementation?

The unit test should be testing A, and as such should only care about input and output of A directly. If something isn't visible in A's interface (either directly or indirectly) then it may not actually need to be part of Aimpl at all (as its results aren't visible to the external world).

If Aimpl generates side effects you need to test, that indicates you should be taking a look at your design.

Fraenum

Agreed. You test the interface, not the internals. – Gulfweed

I think I am currently using pImpl in places where other techniques may be better suited. More specifically, I was using pImpl to hide the use of an external library that may be replaced in the future. I regarded this as an implementation detail and thought pImpl was a good fit but I guess there must be a better way to do it. (e.g Interface, Adapter, Strategy etc) But this has got me confused .. Can you give me an example of when pImpl is better suited than such patterns? – Whenas

@Rimo, the whole point of the pImpl, pimpl, read: en.wikipedia.org/wiki/… is to hide implementation. from a testing perspective, you need to test does x input produce expected y output. How that comes about is an implementation detail, and is not your responsibility to test (from a unit-test perspective). – Gulfweed

+1 Test to the specification - not to what your understanding of the class. – Anabatic

Implementation independent tests are ok for black box testing. But in white box tests I want to test the implementation details. Thus it matters to have access the internals of the class/object. – Feune

Answer 3

If you're doing dependency injection right, any dependencies class A as should be being passed in through its public interface - if your pImpl is interfering with your testing because of dependencies, it would seem that you're not injecting those dependencies.

Unit testing should only be concerned with the public interface that class A is exposing; what A does internally with the dependencies isn't your concern. As long as everything is being injected properly, you should be able to pass in mocks without needing to worry about A's internal implementation. In a sense, you could say testability and proper pImpl go hand-in-hand, in that a non-testable implementation is hiding details that shouldn't be hidden.

Profluent

You are saying only the public methods of class A need to be tested. But don't you sometimes need to unit-test private helper methods in isolation? For instance the google test framework supports the FRIEND_TEST macro especially for this. – Whenas

@Rimo: can you use the techniques you use to test private methods of non-pimpl classes to test the pimpl class? Or am I missing something? – Yand

Answer 4

The pImpl idiom makes testing far easier. It's strange enough to see a set of answers on the theme of "don't test the implementation" to motivate answering so long after the OP.

In usual, non-pimpl based C++ you have a class with public and private fields. Public fields are easy to test, private fields somewhat more tedious. The division between public and private is important though, since it decreases the width of the api and usually makes later changes easier.

When using this idiom a better option is available. You can have exactly the same "public" interface as with a single class, but now there's only one private field containing a pointer of some sort, e.g.

class my_things
{
  public:
    my_things();
    ~my_things();
    void do_something_important(int);
    int also_this();
  private:
    struct my_things_real;
    std::unique_ptr<my_things_real> state;
};

The my_things_real class is expected to be visible in the same source file as the destructor of the externally visible class, but not in the header. It isn't part of the public interface, so all the fields can be public.

void my_things::do_something_important(int x) { state->doit(x); } // etc

class my_things_real // I'd probably write 'struct'
{
  public:
    int value;
    void doit(int x) { value = x; }
    int getit() { return value; }
};

Unit tests are then written against the real class. Test as much or as little of it as you like. I've deliberately called it "real" instead of "impl" to help ensure that it isn't mistaken for a mere implementation detail.

Testing this class is very easy since all the fields are public. The external interface is very small since it's defined by the other class. The wafer-thin translation layer is difficult to get wrong, but you're still welcome to test through the external api as well. This is a clear win from more significantly separating interface and implementation.

On a vaguely related note, it strikes me as absurd that so many otherwise coherent people advocate skipping unit testing for anything that is not readily accessible through the external API. The lowest level functions are hardly immune to programmer errors. Testing to verify that the api is usable is both important and orthogonal to verifying that the implementation details are correct.

Incursive

Note how my_things::my_things_real is private to my_things, wonder how tests will be able to use it :) – Tunis

because my_things::my_things_real is defined in a .cpp source file, how would you #include it in a test file? – Adventitious

It's either defined in a header file or the tests go in the same source file. People other than me seem to like the former. I write the tests next to the thing they're testing when the codebase is my own. – Incursive

Answer 5

The unit testing should put the implementation class thru its paces. Once the PIMPL class is in the picture, you are already into the realm of "integration" - and hence U/T does not apply as such. PIMPL is all about hiding the implementation - you are not supposed to know the class setup of the implementation.

Smoot