Unit Testing Non Public Member Functions

UnitTesting non-public member functions is somewhat tricky in C++ (and, I guess, in Java and other languages, too).

In C++, we have the following options:

Make the test class a friend of the class to be tested. This breaks encapsulation and enables fraud (call me paranoid, but clients of the class can define their own 'test class' and access the internals of the class to be tested -- see Lakos LargeScaleCppSoftwareDesign). Possibly we could use conditional compilation (ugly) to remove the friend declaration in production code. Variation: use conditional compilation to remove the access restrictions.
- Of course, if you define class Foo and declare class FooTest? to be a friend, and someone on your team writes a (non-test) class called FooTest? just to frob Foo's internals, this is a management problem. There are many other ways to subvert the CeePlusPlus access controls (via casting, etc.)--all of these are in the category of cheating. In other words, don't do them.
Create a subclass of the class to be tested for test purposes only. Make all member functions public in the subclass. This is even uglier than option 1 and adds quite some overhead. Apart from that, there are issues with non-virtual functions.
Delegate the methods to a private class. The methods would be public for testing of the new class, but would remain private for the use of the existing class.
Just make everything public, use some other NamingConvention (like PythonLanguage) to mark the private bits. Works, goes a bit against the C++ grain.
Write the tests at the level of what is declared public. After all, if the private method is not invoked from a public method, one does not need it all.

One could argue only to unit test public member functions -- this is impractical when you try to unit test classes with complex non-public member functions, such as ApplicationCommand? or BusinessTransaction objects.

Option 1 seems to be the easiest solution from an implementation point of view, but friend declarations tend to make me nervous.

What's your opinions/suggestions on this?

-- OliverKamps

There's actually one more, simpler method for C++ that does not require decorating the class header; in the unit test:

 #define protected public
 #define private   public
 #include "headeroftheclassbeingtested.h"
 #undef protected
 #undef private

and now the entire class is exposed.

-- RonFox

I'm a UnitTests newbie, but I consider private methods to be just that -- private. UnitTests track the public API of a class, not the internals. At some point, I may need to test protected methods, in which case I'd use a subclass. I program in Java, so I'd probably use an inner class for the subclass, since the outer class is already a subclass of junit.framework.TestCase. In C++, you could just use multiple inheritance.

Public interfaces of classes may sometimes be too coarse-grained to be unit tested effectively -- for example, we use a variation of the CommandPattern to implement ApplicationCommand? or BusinessTransaction classes. Such a class may implement a large chunk of functionality (possibly using layering and inheritance to keep the code organized etc.), but may only have a few public accessors to set and get domain objects and a public execute function. The functions implementing the business logic may very well be private. What would you use the subclass for? To implement the test or to make the functions to be tested accessible to a test class? -- OliverKamps

As far as friend functions go, if you can't trust your fellow developers not to forge friend functions, maybe that lack of trust is the problem. -- JohnBrewer

I possibly don't trust them not to get carried away with friend declarations... But I agree, that might be the underlying problem. -- OliverKamps

Oliver, I'm not sure I see why friend classes make you so nervous. Sure somebody could abuse it, but since you are programming in C++, your clients must have access to the header file that declares your class, friends and all. They could edit your class and add all the friends they wanted, not to mention changing private members to public, and the like. I see your point. --ok

Public, protected, and private are in C++ to protect you from innocent mistakes. They are not there to protect you from deliberate abuse, nor can they.

I see the friend solution as simple and clean -- this is exactly the kind of problem that friend is in the language to solve.

-- CurtisBartley

Would you bother to use conditional compilation to remove friend declarations for test purposes from production code? -- OliverKamps

I would just leave them in, since they don't actually generate any code. Conditional compilation can be really useful, but it also clutters up the code a lot, so I'm inclined to use it sparingly just on that basis alone. I would however use it to remove the test classes themselves if (for example) they were declared in the same file as the main class. -- CB

You want to be able to refactor a class's internals without changing the UnitTest -- it's a bad idea to have to change the class and its test at the same time. For that reason, don't reach into the class and test its internals. Sometimes you may have to. See above for ApplicationCommand? example. --ok

Friend functions are generally inferior to alternatives such as Observer or PrivateInterface. The big problem is that a friend function or class has access to a class's entire footprint, not just to the parts that you want it to have access to. PrivateInterface let you control exactly what parts of a class are visible to whom. This makes refactoring the class easier, because you have explicit knowledge of what parts of the class are visible to the outside world. -- WayneConrad

Thank you for pointing out PrivateInterface to me. I agree with you on the problems of friend functions/classes. I can see me using PrivateInterface in production code if the only alternative were to use friend declarations, but isn't it too much overhead to implement just for UnitTesting? (That's potentially two extra classes and trying to remove it from production code is probably a pain?) -- OliverKamps

If testing is difficult, it won't be done. DoTheSimplestThingThatCouldPossiblyWork. For now, I'd suggest to use a friend declaration unless you can think of something better, then later upgrade the code when you learn a better solution. But then again, do you really need to test the non-public member functions? Why are they non-public? Why aren't they being flexed from the public interface? If you're supposed to subclass, subclass and test. -- SunirShah

Suppose you have a telecoms billing system. A customer has been in default for a period of time. The business rules dictate the customer's services have to be suspended (e.g., phone lines have to be disconnected) and that the customer won't be able to subscribe to new services until the debts are settled. All these things can be triggered from a single ApplicationCommand? object whose public interface may consist only of a setter for the customer number (or a reference to the customer domain object) and an execute function. All other functions implementing the business logic may and should be non-public. This ApplicationCommand? would most probably delegate sub-tasks to lower-level ApplicationCommand? object whose public interface would also be very small. -- OliverKamps

From WhoIsUsingJunit:

We've found that TestingPrivateMethods? seems to be of CodeSmells. TestingPrivateMethods? smells of debugging. TestingPrivateMethods? also prevents us from being able to RefactorMercilessly. If you ListenToTheCode, you learn that MethodsShouldBePublic. Therefore, execute the ExtractClass refactoring, have all the former private methods be publicly accessible, and test them ruthlessly. You may end up with a reusable abstraction whose responsibilities will grow over time. -- FrankWestphal

Obviously, I wasn't able to made myself understood on the use of the ApplicationCommand?. Let me try to both improve that and prove I've done my bit of reading Wiki and Martin Fowler's book: Complex functionality was implemented in big controller classes operating on dumb data objects. We've found that the complex functionality was often implemented in a single LongMethod? which we found to be a BadSmell. We decided to RefactorMercilessly and applied MoveMethod to move the LongMethod? to a newly created MethodObject. Then we applied ExtractMethod several times and created new methods for parts of the functionality of the original LongMethod?.

Or, to make a long story short, the non-public methods don't have any reason to be public. Clients of the ApplicationCommand? need to know what happens, but not how it happens. The point of applying GofCommand? is to have a single execute function (plus some accessors to set the receiver etc.) to trigger some behaviour. The non-public functions may just be implementation details, but the public execute function may be too coarse-grained to be UnitTested alone. (Don't think I mentioned that the first time.) -- OliverKamps

So, you're saying you can't possibly extract the details? Let me elaborate on what I had in mind:

  public class ApplicationCommand {
public ApplicationCommand(CommandMethodObject methodObject) {
_methodObject = methodObject;
}
private CommandMethodObject _methodObject;
public void execute() {
_methodObject.method1();
_methodObject.method2();
_methodObject.method3();
}
  }

Now, you should be able to easily write tests for your implementation details since those features will be public member functions on your MethodObject:

  public class CommandMethodObjectTest extends TestCase {
public CommandMethodObjectTest(String name) {
super(name);
}
public void testMethod1() {...}
public void testMethod2() {...}
public void testMethod3() {...}
  }

Sometimes you have to apply this refactoring recursively, so that you'll end up with LotsOfLittlePieces?. Hope this helps. -- FrankWestphal

I posted a variation of this to HidingByDelegating. -- DaveHarris.

No, I'm saying I possibly can't extract the details. (I'm not being picky here, but I think the location of 'possibly' makes a difference here.) Or, maybe, I don't want to expose the details. method1, method2 and method3 may not be supposed to be called directly in production code, so I don't want to break encapsulation and make them public. What's more, member data of ApplicationCommand? is not accessible for CommandMethodObject?, so I might have to duplicate this. IMHO, exposing implementation detail smells a lot more than TestingNonPublicMethods?. I can't detect the smell of debugging here. I disagree that TestingNonPublicMethods? prevents us from being able to RefactorMercilessly. Testing any method makes refactoring this method more difficult or, rather, more expensive, but this is not a good reason not to refactor or not to test. Limiting visibility of a method doesn't affect the number of tests that need to be modified when refactoring this method. Therefore, I can't see that refactoring a non-public method is any more expensive than refactoring a public one.

So, even though I disagree with it, your reply was definitely helpful. Thanks. -- OliverKamps

Oliver, you don't have to expose method1, method2 and method3 to the public. Package privacy would be sufficient. For not being able to access ApplicationCommand?'s member data, just pass it to your MethodObject. Writing tests will become easier if you make your methods more fine-grained. Your mileage may vary, though. -- FrankWestphal

Except that Oliver is writing in C++, which doesn't have a package-level access. He's stuck with public, private, protected and friend.

BTW, I don't use package visibility at all in my Java code. I explicitly place my UnitTests in a separate package, just to be certain that I'm only testing the public interface. -- JohnBrewer

The cost is having to make every class public, even those which you don't intend to be called from outside the package. -- WayneConrad

What exactly is the cost you're speaking of? What are your fears? -- FrankWestphal

I do not like to make classes public unless they are part of the package's API, for the same reasons I do not like to make methods public unless they are part of the class's API:

Documentation: Having only externally usable classes be 'public' documents which classes are part of the package's API. If all of the package's classes are public, the programmer has to work hard to figure out which classes are for part of the package's API and which are internal to the package.
Coupling: If all of a package's classes are public, the programmer can use the package's internal classes, either intentionally, or because the programmer guessed wrong about which classes were part of the API. Coupling to internal classes inhibits refactoring -- you can't eliminate or change an internal class without first discovering whether it has been used by another package. Also, it is harder to create a package that works if other packages can reach into its internals and mess with them.

-- WayneConrad

I don't see the problem. Why would you directly test package-level classes any more than you would test private methods? The unit tests are there to verify that the public behavior of classes - and packages - works as advertised. How it does it, what internals it uses, should not be relevant to such testing. -- RussellGold

Russell, Are you responding to what I wrote, or to this page in general? I'm in the same camp as you -- only UnitTest the public methods of a class. -- WayneConrad

I'm curious, are you guys (Wayne and Russell) proposing that private methods shouldn't be tested directly at all, or are you saying only that they shouldn't be "unit" tested? -- CurtisBartley

Curtis, I claim that it's better to have the unit test only call the public methods of the class, letting the private methods be tested only indirectly. My reasons are:

Having the UnitTest call private methods presents a problem when refactoring. If your unit test is directly testing private methods of the class, then refactoring the internals of the class may require you to change the unit test, and when the test fails you won't know whether the class or the unit test is at fault.
It is my experience that when a private method "wants" to be tested, it is probably not tightly coupled to the class's internal state (because methods that are tightly coupled to the class's internal state tend to be much harder to test, and being lazy, I don't like testing the hard parts as much as I like testing the easy parts). In this case, it may be best to make the method a public method of a different (possibly a new) class. For example, if a class is doing some string manipulation and finds itself with a private method called changeTabsToSpaces, that is an excellent candidate to be made a public static method of some other class, where it can then be properly tested and easily used from other classes. You may also find that there are several methods which operate upon a common subset of the class's state -- these may be a good candidate to become a new object. Although you might think that these refactorings are silly, being done just so that you can test these methods, they are not -- they are design improvements that needed to happen anyhow, and your desire to reach into the private methods to test them was simply a warning sign that the design was probably not factored as well as it could be.

-- WayneConrad

Wayne, would you say then that only methods that are tightly coupled to to their class's internal state should be private? -- CB

No, I wouldn't say that. Public as well as private methods should deal with as much of the class's state as they need to. What I'm saying is that a private method that deals with little or none of the class's state can often be moved into another class, both improving the design and making it public so that it can be tested. -- WayneConrad

Usually, I find that when there are complex private methods, it indicates that there is an internal class begging to be broken out. Once one does this, the methods in the new class will need to be public and testable, but by including the new class as a private object within the old class (encapsulation!), the methods remain private at the higher scope. This partition allows full testing of the functionality without the frustration of access through more general purpose methods. It is worthwhile to let testing requirements drive some of the design (in hardware development, this was called "design for testability"), and I usually have found that I may have fought the intial decomposition, that when I did it, it improved the application code design as well. --WayneMack

I don't think I follow how ApplicationCommand is being used here. When I use the CommandPattern, most of the code ends talking to the base class - here presumably ApplicationCommand. All the specific method1()s end up in a derived class - which seems to be missing here, say ApplicationCommandImp?. It doesn't much matter if they are public there, because so few places need to know about ApplicationCommandImp?. (If necessary, we can push them down into another subclass.) -- DaveHarris

What an amazing bunfight! Lookit, here's the easy way to UnitTestCpp.

I don't think that it is necessary to ever test anything beyond the public interface. If something is internally complex, and its producing errors, have it provide an accurate error report for debugging and repair.

If I were to test a method I'd check various things, primarily its post conditions (although in certain circumstances I'd also check things like performance). When testing this method I don't care about its internals, only that it does what it should.

This can be extended to a class. When testing a class I am only interested in its doing what it should, what it says that it does. I am only interested in its fulfilling its contract. Therefore I only test its public interface. -- Bryan

I think this issue is all about scope & visibility. Most programs have several levels of interfaces: An external interface (GUI, command-line, protocol etc), and deeper inside, a package or library interface, followed by classes, and then their private members. Each level has an internal implementation that the outside world doesn't need to know about, and each level needs to be tested thoroughly and equally.

From most of the posters above it would seem that testing classes internal to a package is fine, if so then it follows that testing members internally to a class is also fine, as long as those tests themselves are internal to the class. If the internal implementation of the class changes, then so do the class's *internal* tests to support those changes (ouch! I mean if the tests change then the implementation changes...).

As far as i can tell, whether or not a complex class needs to be broken down into smaller simpler classes is an orthogonal issue, and not to be confused with how you decide to test it. -- Tim Macfarlane

>Make the test class a friend of the class to be tested. This breaks encapsulation and enables fraud.

I consider the unit of deployment to be a package/library, not the class. Anything in the package can be assumed to be safe to access the internals of other classes in the package. A class isn't of sufficient deployment granularity, especially if you make a lot of classes that must cooperate. Making things public that shouldn't is far worse because anyone can screw you up. With the package access you know the package developers have control.

And yes, test what needs testing. It's not a smell. Often a private method will be used by several other public methods, there's no reason not to test the base functionality independently. If you are afraid of not being able to refactor mercelessly then don't write that damn tests to start with. Tests aren't any good unless they are actually written.

Language itself rather than stand-alone application should provide unit testing features. Either the language may have *testable* access modifier or all methods should be testable by default. -- Alex Aizikovsky

That requires that the language have the ability to designate certain code as test code...

...which is a really good idea. I like it! -- JonathanTang

This is how I do unit testing in Java:

1. Use the test-first approach to test the public methods before they are implemented, use Unit Testing to drive the design of the class.

2. A series of private methods were implemented when implementing the public method, the private methods were driven by the public method instead of the TestCase.

3. Normally I do not unit test the private method because it breaks the encapsulation and also impedes refactoring.

4. I change the private visibility to package visibility to test the private method only if the private method is complex and I need the unit testing to give me the confidence. -- Yaxin Wang

I feel that everyone here is missing the point of gaining access to private and protected parts of a class. The problem isn't accessible functions, it's accessing the members themselves. When I'm testing a function, I'm testing the internal state of the class during and after the execution of the said function. I've been making my members protected only, so I should be able to just subclass to access, but I'm looking for a better way that DOESN"T involve friends. I find that syntax so ugly and will only use it in the most desperate occasion (aka, if I don't have time to refactor). So what do you guys think of this problem? How do you access the members of a class? Please also note that I'm using TUT, and thus can not use the friend syntax because my tests aren't classes. -- Jason Roelofs