15

I am wondering if unit testing private methods is a good practice?

Normally only public interface should be tested.

However, I have found out that during complex calculation, which calls tons of different private methods, it is easier to unit test the private methods first, and then make a simple test for the public interface method.

As an example let's say you have an audio player and you have functions:

void play(){ ... }
void pause(){ ... }
void seek(time t)
{
    //All Private methods
    checkIfValidTimeRange(...);
    moveToFilePos(...);    
    fillBuffers(...);      
}

Normally I would write unit tests for : checkIfValidTimeRange(...), moveToFilePos(...), fillBuffers(...).

But I am not sure if doing so is good practice.

Tome
  • 3,234
  • 3
  • 33
  • 36
Anton
  • 1,181
  • 2
  • 17
  • 27

4 Answers4

14

It's not a good practice (yet that doesn't mean you should never do that), and if possible you want to avoid it. Testing private method usually means your design could be better. Let's take a quick look at your player example:

  • moveToFilePos: sounds more like a responsibility of something doing I\O operations, not a music player's
  • fillBuffers: more of a memory manager's job rather than music player
  • checkIfValidTimeRange: again, probably could be moved out of player's scope to some simple validation class (seems like this one might be useful in other places aswell)

At the moment your music player does I/O, memory management and what not else. Is that all really in scope of its responsibilities?

Community
  • 1
  • 1
k.m
  • 30,794
  • 10
  • 62
  • 86
  • This example was just to show a simple demonstration. But lets say i create the MusicFileIO class and MusicFileBuffer class to check. Should i make these classes internal to the music player class ? (Since i dont want to expose their existence to the user). But if i make them internal and private, am i not back to my problem in the beginning? – Anton Feb 09 '12 at 00:00
  • 1
    @Anton: How much exposed those new classes end up being is irrelevant as long as they get properly tested. We're drifting away from real problem here - testing private method is not bad because the *method is private*, but because it usually means *poor design*. You don't want to find yourself in a situation when changing `fillBuffer` (*"Now buffer comes filled from elsewhere!" - anonymous client*) breaks music playing in your program. It makes little sense now, it will make no sense at all later. – k.m Feb 09 '12 at 14:36
  • Still. Untested code becomes "legacy" code (see [Working Effectively with Legacy Code](http://www.amazon.co.uk/Working-Effectively-Legacy-Robert-Martin/dp/0131177052)). So if the real complexity of the public interface is such that it's unfeasible to cover it completely (i.e. a facade) testing the private methods is, imho, the lesser of two evils. – Txangel May 31 '13 at 13:14
5

If your private methods are complex enough to warrant testing, you're likely missing some classes where those private methods are turned public.

You certainly can test private methods, but you should take the need to do it as a hint there's something wrong in your design.

Don Roby
  • 40,677
  • 6
  • 91
  • 113
4

IMHO it's a very good idea, I do it all the times. I usually create a helper class which makes the private methods accessable and test it..

Usually it's even easier to test private methods, since they do something very specific. On the other hand you might have a big public method which is a bit harder to test. So it certainly simplifies unit tests.

duedl0r
  • 9,289
  • 3
  • 30
  • 45
0

Which part of your code base is your private methods relying on ? If somebody changes the way one of the method you are relying on works and thus breaks your method, isn't it worth knowing it ? Testing is not only for checking that your method behaves as it should, but also to check that changes in other parts of the codebase doesn't break your method.

So unless your method is only using basic constructs of your language, test it !

Jonathan Merlet
  • 275
  • 3
  • 13