Best practice - testing private methods

[Alan Livie]

I have been refactoring some methods and applying 'compose method' pattern to some of them ie breaking up the big public method into one public method at a higher level that calls lots of small private methods to do the work. This makes the code more readable and easier to test.

I am also aware of a 'best practice', especially among the JUnit crowd that says you shouldn't unit test private methods (a Google search shows lots of opinions on the subject)

As testing private methods in mxUnit is easy and its helping me debug and refactor code more easily I will continue to do it anyway.

I just thought its an interesting debate and whether cf'ers had the same polarised opinions on the subject.

I would argue it should be best practice TO test private methods and that if we don't unit test private methods and just the public one that uses them we're not doing 'true' unit tests as we're testing things I would say were at a higher level than a unit.
I still have a unit test for the public method and don't mind seeing it fail alongside the small private method that caused it to fail. Having the private method test makes it easy to debug and correct the failure.

Alan

[Marc Esher]

I'm coming to develop a sense of smell about testing private methods
that I didn't have before. And also, sometimes, of ignoring that
smell. I think in general, I try not to do it, and when I do find
myself "needing" to do it, I at least try to ask myself why I'm doing
it.... try to figure out what about my design is potentially bad that
it's causing me to do so. Sometimes, I accept it; sometimes, I will
change the design.

Here's an example of where I have a private method being tested in
mxunit itself when, really, it'd be more appropriate to refactor. In
directorytestsuite.cfc, there's a private method named "accept". it
looks at the "excludes" argument, and also at some metadata for the
testcase in question, and decides whether that test should be included
in the derived suite. this is precisely the kind of functionality that
I'd want to test easily, i.e. I want to know whether
"com.something.LibraryTest" will be pass through accept() if "Library"
is in the excludes argument. And I sure as heck don't want to test
that indirectly through the public interface (i.e. through
directorytestsuite.run()). So naturally, you just test the private
method, and that seems perfectly reasonable I suppose.

But probably the better solution would be to take that accept()
functionality out of the cfc itself and into a new
"TestAcceptanceStrategy" or "TestAcceptanceFilter" type of component
that has a public accept() function. This not only gets rid of the
problem in question, i.e. "should i test private functions", because
now the function is public, but it also then enables easy swapping out
of strategies, which it's now apparent is an issue with MXUnit. Folk
want more flexibility in naming conventions, and this kind of thing
would provide that. So for example, by default, directorytestsuite
would just use some "DefaultTestAcceptanceStrategy" component, and if
you wanted additional flexibility, you'd create your own component,
implement "accept", and set that into the directorytestsuite object
using setAcceptanceStrategy(your.component) or some such thing. This
is along the lines of FileFilters or Comparators in java.

Now, does that mean I want a gazillion little components sitting
around for all the things that used to be private? surely not. But it
does mean at least I'm trying to be on the lookout for these kinds of
things.

In general, I'm not a big fan of opening up access just to make
something testable. That there though is another holy war.

I came across this and thought it was a good read. particularly the
comments because they get into a lot of different scenarios:
http://michaelfeathers.typepad.com/michael_feathers_blog/2007/09/the-deep-synerg.html

I'll pose the same challenge that michael feathers posted in his: can
you think of a scenario in your code right now where it's appropriate
to test the private method rather than refactor into another
component?

best,

marc

[Alan Livie]

Hi Marc.

I agree in some situations maybe this is a design issue and another class is the way forward.

However, I still disagree in other situations :-)

After reading 'Refactoring to Patterns' by Joshua Kerievsky ( http://www.amazon.co.uk/Refactoring-Patterns-Addison-Wesley-Signature-Kerievsky/dp/0321213351 ) I started applying Compose Method to some of my 'difficult to understand' methods. As some of the new private methods were only 2-3 lines of code and the main purpose for them existing was to make the large public method smaller and more readable they were not good cases for a separate class.

I could just ignore them in my unit tests and just test the public method as the code did previously. I just found it easier to test all the private ones too as it made things easier to debug and know *EXACTLY* where the problems occurred.

These private methods could easily be public but I like the idea of keeping things private and opening them up as I need to. As nothing outside the cfc needed to use the new composed methods it makes the intent clearer if they are private. I also have most of my DAO's and Gateway methods as 'package' access so only my Service objects can use them (as they sit in the same dir). This is a subtle hint to other developers (and myself) that we shouldn't be running these methods from anywhere else.

Alan

________________________________________
From: mxu...@googlegroups.com [mxu...@googlegroups.com] On Behalf Of Marc Esher [marc....@gmail.com]
Sent: 06 May 2008 12:50
To: mxu...@googlegroups.com
Subject: [mxunit:395] Re: Best practice - testing private methods