This article compares four different approaches to testing private methods in Java classes.

http://www.artima.com/suiterunner/private.html

Do you test private methods? If not, why not? If so, how do you prefer to test them?

Bill, your parseArgsIntoLists function screamed duplication to me.

Does it indicate that parseArgsIntoLists should be moved into another class to promote reusability, as the JUnit FAQ suggests? Would Dave and Andy say its a warning sign that there's another class in there struggling to get out?

Yes and yes, imnsho.

I don't know Java, so the following probably doesn't work, and you may not care for the brace alignment, but it conveys, to me a much clearer sense of what is happening than the original code (and I'm sure real working Java could be written that looks more like this):

switch args[ i].firstTwoCharacters()
	{
	case "-p": addNextTwoArguments(runpathList, i);
	case "-g",
	     "-o",
	     "-e": reportersList.add(args[ i])
	case "-f",
	     "-r": addNextTwoArguments(reportersList, i);
	case "-s": addRemainingArguments(suitesList, i);
	default:   throw new IllegalArgumentException("Unrecognized argument: "
                        + args[ i]);

And here's a bug (well, undesired behavior at least): in the web formatting code: since the java code includes bracket-i-bracket, it italicizes everything after(I'm adding spaces to prevent that...)
Peace,
--Carl

I agree with the post above. If a generic algorithm can be pulled out into a private method, it can probably be pulled into a seperate class that is used by the currently tested class. Then you can write tests for it. I.e. another class is probably struggling to come out.

If it is only called by one method that does not mean there is a problem with creating a class to hold it and any potential brethren. Rather than worry about a class's client list, worry about how the class's cohesion.

> I don't know Java, so the following probably doesn't work,
> and you may not care for the brace alignment, but it
> conveys, to me a much clearer sense of what is happening
> than the original code (and I'm sure real working Java
> could be written that looks more like this):
>
>

> switch args[ i].firstTwoCharacters()
> 	{
> 	case "-p": addNextTwoArguments(runpathList, i);
> 	case "-g",
> 	     "-o",
> 	     "-e": reportersList.add(args[ i])
> 	case "-f",
> 	     "-r": addNextTwoArguments(reportersList, i);
> 	case "-s": addRemainingArguments(suitesList, i);
> default:   throw new
> w IllegalArgumentException("Unrecognized argument: "
>                         + args[ i]);
> 

>
As you guessed, you can't do that in Java. Java requires that the expression that comes in the parentheses following a switch be an int. The same is true, therefore, of the case statements. When it comes to switching on the semantic value of Strings, which is what this method does, you must in Java use a series of if-else statements.

> I agree with the post above. If a generic algorithm can
> be pulled out into a private method, it can probably be
> pulled into a seperate class that is used by the currently
> tested class. Then you can write tests for it. I.e.
> another class is probably struggling to come out.
>
> If it is only called by one method that does not mean
> there is a problem with creating a class to hold it and
> any potential brethren. Rather than worry about a class's
> client list, worry about how the class's cohesion.

Well, it is always true that anything that can be pulled out into a private method could alternatively be placed in a separate class. If you make that class package access, then you can write tests for it. I refer to this as "approach 2" in the article. But you need not actually pull it into another class to test it that way, you can just delete the word private and make the method is package access where it sits.

As far as cohesion goes, the reason to worry about cohesion is to make the code easier for humans to understand. That's the real goal. And a class with one static method to me does not carry its weight. You have now added another class to the package-level API, and it will be in people's faces to some extent, and they'll have to do a bit of work to realize they don't need to care about it because it is only used in one place. Occasionally, you will see a class that only contains a bunch of static methods. This is fine, but it is rather rare, and a class that contains only one static method and which is needed by only one other class really calls to me to be moved into that class and made into a private method. That's the kind of thing private methods are for in my book (which I should probably write someday.)

There is no perfect design, and different people prefer different things. Some people don't mind turning private methods into package access for the sake of testing. But some people do mind, and for them, they can use any of the other 3 approaches I list in the article. I am curious to hear how other people reading this prefer to deal with the testing private methods issue.

> And here's a bug (well, undesired behavior at least): in
> the web formatting code: since the java code includes
> bracket-i-bracket, it italicizes everything after(I'm
> adding spaces to prevent that...)

Yes I'm aware of that one, and I did fix it once. The problem stems from the interplay between two different filters that come with Jive Forums, which I use underneath for the forums here at Artima. Quite a while back I replaced one of their filters with one I wrote myself (that algorithm was actually the very first thing I did test-first). That solved the problem, but I later upgraded to a new version of Jive that had a bunch of breaking changes to their APIs. One of the things that broke was that filter, so I went back to using their old one which has the problem still. What's worse is that the code markup doesn't work anymore, because that was something I added in my filter. It is on our list, but hasn't bubbled up high enough to get attention yet.

While I haven't actually tried this, I was wondering about the possibility of using AOP to help with this.

The idea would be that you would use aspects to change your private methods to be public, and then your test code would be compiled and run against the "aspectized" version, rather than the original version.

Some potential practical issues:

1. I don't know if any AOP libraries currently support changing accessibility of methods.

2. A private method in a superclass, made public, may clash with an existing method in a subclass.

3. Things like refactoring wouldn't work properly, as you'd be refactoring the original source with private methods.

Anyway, just a thought... don't know if it would work in practice or not.

How about using xdoclet or annotate metadata to help you generate testable code? The code for the method(s) with a specific annotation could be copied to a test-friendly source file with test-friendly access and be incorporated into your test suite.

> How about using xdoclet or annotate metadata to help you
> generate testable code? The code for the method(s) with a
> specific annotation could be copied to a test-friendly
> source file with test-friendly access and be incorporated
> into your test suite.

It sounds like both you and Calum are suggesting ways to autonomously modify the source code, or make a second more testable copy, that converts the private to package access. Perhaps this is a fifth approach. One trouble I forsee with taking such an approach is that it might be hard to get IDEs like IntelliJ and Eclipse to not complain that I'm trying from my test to invoke an inaccessible private method. I could conceivably do it the other way, and have the tool transform marked methods from package access to private for deployment. The problem I have with that is that then my IDE wouldn't be warning me when I invoke an inaccessible private method from a non-test class. Another issue I would have with doing it this way is I think it would be a bit confusing to the newcomer.

None of the four approaches I listed in the article are perfect. They each have advantages and disadvantages. Going in and autonomously modifying the source code (or the binaries for that matter) for the test also has advantages and disadvantages. I wouldn't want to do it this way given my current circumstances, but someone out there may be in a situation for which this is a reasonable approach.

I won't talk about the use and misuse of testing private methods (or fields), I just can't add any better idea.

But let's talk about the example with the argument list parsing. I recently used a PDF library to merge PDF files, as I was in a hurry and didn't want to dive into the APIs, I just called the main method of a "tool" class, passing the arguments as strings instead of calling a java method with the correct arguments.

Based on this experience, I thought we could see "main" methods as beeing a public interface for calling black box services. If you do agree with this point, what do you think about saying that the parse arguments method also could be a public method? So client code could validate the way it calls the public interface (maybe in some test code).

Think of arguments as a "serialized" list of objects...

I am not saying we always should turn private methods we want to test to some public service, it just happened I don't really like the "main(String[] args)" way to startup a program.

Jean Lazarou

PS:
(1) The generic body of a main method could be :
(a) set default startup parameters
(b) parse and validate arguments, produce the startup
parameters
(c) call the actual startup method with the startup
parameters (the startup method is type safe but
could require some kind of MyParameters object)
- the startup method is not to be static .

(2) What about starting programs with a command line like :

> java my.package.MyClass@aMethod 528 "Hello, World!"

with the MyClass class beeing:

> class MyClass {
> public String aMethod(int i, String s) { ... }
> }

Private methods are typically pieces of duplicate code that have been refactored from other methods. Thus, the "indirect testing" concept is going to work fine, and you will not need to directly test private methods in the large majority of situations.

While this is mildly interesting, I think you're solving the wrong problem here. IMO, a much more important problem is that too much ceremony is required to create unit tests. What we need is a simpler way to create and maintain unit tests.

> Private methods are typically pieces of duplicate code
> that have been refactored from other methods. Thus, the
> "indirect testing" concept is going to work fine, and you
> will not need to directly test private methods in the
> large majority of situations.
>
In the article my conclusion is that testing private methods indirectly will likely be the best choice most of the time, but not always. Just this week I had the problem again, while working on the Java app that formats the pages for this website. The problem came up in the usual way, not when I was factoring out duplicate code from multiple methods into a private method, but when I just had a private method for the sake of making the calling method easier to understand. As with the example I used in the article, the calling method was main(). I used my usual approach to test the private method, approach 2 in the article. I made the method package access.

> While this is mildly interesting, I think you're solving
> the wrong problem here. IMO, a much more important problem
> is that too much ceremony is required to create unit
> tests. What we need is a simpler way to create and
> maintain unit tests.

What ceremony is that? The ceremony of adding test methods to test cases or suites doesn't seem overly burdensome to me. I know you prefer to have the test code next to the production code, but you can do that with JUnit or SuiteRunner by putting declaring the test class as a nested class inside the production class it is testing. You could also script the tests with Groovy or Jython, but I don't think that will really reduce much ceremony. Are you perhaps feeling the urge for something more like precondition, postconditions, and invariants a la Eiffel?

> While this is mildly interesting, I think you're solving
> the wrong problem here. IMO, a much more important problem
> is that too much ceremony is required to create unit
> tests. What we need is a simpler way to create and
> maintain unit tests.

Can you expand on this? What do you mean by ceremony? I tend to agree unit tests can be tedious to write, though I've found that extremely granular tests are substantially easier to write and maintain.

> Private methods are typically pieces of duplicate code
> that have been refactored from other methods. Thus, the
> "indirect testing" concept is going to work fine, and you
> will not need to directly test private methods in the
> large majority of situations.

I'm not really convinced that this is really the case. If a piece of code warrants its own method than I feel that it's not unreasonable to say that it warrants its own testing, too.

In particular, any method that is capable of producing different results depending upon a set of parameters or other conditions should have a set of tests that cover all the variations. Those tests should limit themselves to the method being tested and should not be encumbered with the added complexity of having to manipulate the public interface in order to induce specific behaviour in hidden methods.

Further tests may be necessary to ensure that the private methods work as intended via the public interface but that is a separate issue to demonstrating that those methods work in their own right.

Vince.

"... While I haven't actually tried this, I was wondering about the possibility of using AOP to help with this..."

So was I :-)

At first I did think: Well, if I can introduce new methods and parents with aspects then surely I can also introduce a new inner class (Suggestion 3)... But alas, no such luck (in AspectJ, anyway).

But then noticed this:

If I specify an aspect as privileged then I can access private fields and methods.

So here is what I will do in the future:

public class A {
   
   private boolean privateMethod() {
      return true;
   }
   
}

will have its private parts tested by

public privileged aspect ATest extends TestCase {
 
      public void testPrivateMethodInA() {
         A a= new A();
         Assert.assertTrue(a.privateMethod());
      }
 
}

:-) And the best part is.. It works! :-)

(And btw, I did this with JUnit and AspectJ on Eclipse 3.0RC1. And note that for some reason I need to put the aspect (ATest) and the AllTests class in the default package to make it work...)