unit-testingtestingjunitpowermockwhite-box

When is it appropriate to bypass encapsulation in unit tests?


I've recently become aware of powermock's Whitebox functionality. (In summary, it allows you to directly test private methods or modify private members directly from unit tests--while keeping them private!)

I know that there are some frames of thought that frown upon unit testing anything other than visible methods, but darn it, sometimes you just want a simple test to ensure a deep level helper method is doing what it is supposed to do...without going through what could be huge overhead to prepare the parameters and mocks needed for the parent methods to filter down to your method..and then you have to just about do magic to extract the results of that inner method. In short, sometime testing those inner methods requires the most complex and hard to follow unit tests. (And, fyi, our company has a policy of 100% unit test coverage. Those methods must be tested!)

I'm aware that one methodology is to change method accessors in order to allow for unit tests (e.g., change from private to protected, but powermock Whitebox allows a direct test without changing the source code.

Still, Mamma always said, "Just because you can, doesn't mean you should."

Is it ever appropriate to test inner methods like this? If so, what is the rule of thumb I should use?


Solution

  • Let's start off by acknowledging that this is a partially religious debate.

    It is my point of view that testing private methods is something you should often avoid, but there are exceptions.

    Why shouldn't I test private methods?

    Because you're testing whether or not your application works, not necessarily how it works. A private method is not part of the API that your program exposes and it is essentially an implementation detail.

    A common argument against working around the limitations set upon you by encapsulation and still testing it, is that it might break your tests that test for that specific implementation.

    In "normal" testing, we test the public methods because they form our program: if we know all public APIs work then we know the program will work. If your application is nicely structured, you will have several API layers which constantly refine the tests into smaller units so you have a clear overview where something might go wrong somewhere down the line.

    The thing to realize about private methods is that they will always get called at some point by a public method, which will get tested. A private method is an implementation detail of your unit under test and as such should not be tested separately.

    This helper method is not bound by the API and thus there is no guarantee that it effects will remain the same; changing an implementation detail without adjusting the overall functionality will now break your test on that method. So even though your program still works perfectly fine, you now have a broken test. These tests are volatile: a refactor that doesn't change your functionality can still cause you to have to remove unit tests because they are no longer relevant (which is dangerous in itself because you'll have to be very sure the test actually is redundant).

    Why should I test private methods?

    Only a few things are black and white and this isn't one of them. I believe new code should not test private methods but that's where I make the distinction: new code.

    If you inherit a legacy codebase, chances are the architecture isn't very pretty. You might have to jump through hoops and the overall codeflow can probably be better.

    So the first thing you do is write some unit tests that are guaranteed to tell you when you break functionality. So far so good, we can now get to the actual refactoring process. Let's start with this 500-lines private method?

    Keeping the previous remarks in mind, you would now have to look through the codeflow and see what public methods use that private method and keep an eye on them as you make your changes.

    For the sake of ease-of-use, I would write tests here that test just the contract of that private method against the rest of your application: as long as your result from the method doesn't differ, you know that it won't affect any of the other legacy methods. With this knowledge you can now comfortably refactor everything inside that method. When something does break; you can immediately work with your helper method instead of having to go through the public API methods.

    Keep in mind that I would only do this for big refactoring operations: these are temporary tests in the first place and the "implementation detail" is in reality 500 lines of code: that's a detail with a lot of details on its own.

    How should I test private methods?

    You have a few options here on how to make these testable:

    Reflection

    I believe this is what WhiteBox uses. It raises some eyebrows though: there is no static typing (changing the methodname means breaking the test), it's expensive and code using reflection tends to be harder to read.

    PrivateObject

    I haven't looked at the exact implementation but I'm pretty sure this uses reflection behind the scenes. Basically it provides the reflection approach through a convenient interface. I haven't seen it used much (partly because I would use reflection as an absolute last resort) but it's there.

    MSDN

    Extraction

    This would be my first approach: is this helper method important enough to make it stand on its own? Chances are you will say "yes" to this since apparently it is already important enough that you want an explicit test for it.

    Public

    The most obvious choice: simply make it public. This follows the same idea as Extraction: "can it stand on its own?". The difference here is that you still consider this as a part of the current class, you just elevate its access whereas extracting it to a different class also indicates that you're talking about more than just a helper method.

    Internal

    This option takes the middle route: if I make it internal (C# context) and use the [InternalsVisibleTo] attribute, you can make it internal while giving your test assembly the possibility to test it (and still keeping it away from the public API).

    This brings the risk that people only see it as an implementation detail and change the implementation while breaking the tests (that accounted for a specific implementation).

    Furthermore this also increases coupling between your application and the tests.


    All considered it is up to your own preference: there are several options to test private methods and there are some arguments for both sides. Personally I would stay away from the hassle of testing such implementation details and instead see what I can do to improve my architecture. Chances are the problem will solve itself that way.