Testing private members

[Eusebio Floriano]

Hi folks,

I have some private methods and attributes and i would like to test them.
But i don�t want to expose them to public just for test purpose.

How can i test this members ?

Regards,

[Alexandru Popescu]

Check:
this forum [1]
this forum [2]

./pope

[Eusebio Floriano]

This is a interesting discution about testing private members in this thread.
Here are my questions:
In Lasse�s opinion, one private member should not be tested because it�s just a part of an whole behavior that is exposed by the public member and this one that should be test.
I would like to give you an example and debating what is the best way (or a good way) to do.
Let�s supose that it�s necessary to validate an student�s school registration.

 
public void validateRegistration(Student student) {
   // do something
 
   validateGrade(student.getSomeData());
 
   validateFinancialPayment(student.getOtherData());
 
   // do other things
 
} 
 
private void validateGrade(SomeData sd) {
  //complex code
}
 
private void validateFinancialPayment(OtherData od) {
  //complex code
}

validateXXX is a part of the whole validateRegistragion. They don�t have a real meaning by themselves but they are complex enought to deserve to be tested.
I wouldn�t like to expose them JUST because i want to test them.
What do you suggest me to do ?

Regards,

[Lasse Koskela]

You could make the private method protected and in your TestCase, subclass the class under test in order to access the protected method. Beyond that, you could use Reflection to bypass the private restriction (the JUnit Yahoo! group's "files" section includes a class named PrivilegedAccessor.java which you can use).

[Ilja Preuss]

Originally posted by Vinicius Boson:

I wouldn�t like to expose them JUST because i want to test them.

Why not? What harm would be done?

[Lasse Koskela]

Note that I'm still of the opinion that it is generally the best option to make the functionality public, i.e. something like this:


Instead of doing:

public class RegistrationValidator {
 
    public void validateRegistration(Student student) {
        // do something
 
        validateGrade(student.getSomeData());
 
        validateFinancialPayment(student.getOtherData());
 
        // do other things
    } 
 
    private void validateGrade(SomeData sd) {
        //complex code
    }
 
    private void validateFinancialPayment(OtherData od) {
        //complex code
    }
}

...you could do this:

public class RegistrationValidator {
 
    private GradeValidator gradeValidator;
    private PaymentValidator paymentValidator;
 
    public RegistrationValidator(GradeValidator gv, PaymentValidator pv) {
        this.gradeValidator = gv;
        this.paymentValidator = pv;
    }
 
    public void validateRegistration(Student student) {
        // do something
 
        gradeValidator.validateGrade(student.getSomeData());
 
        paymentValidator.validateFinancialPayment(student.getOtherData());
 
        // do other things
    }
}
 
public class GradeValidator {
 
    public void validateGrade(SomeData sd) {
        //complex code
    }
}
 
public class PaymentValidator {
    public void validateFinancialPayment(OtherData od) {
        //complex code
    }
}

This has the beneficial side effect of making your code both easier to maintain (avoiding a monolithic do-all class) and more testable (avoiding severe forehead trauma when you need to add functionality and need to edit a brittle, untested 200-line method somewhere instead of one of many tested, short methods.

Again, if you still don't like to make validateFinancialPayment() etc. public, you can make the package-private (the default access scope) and your tests (and the top-level validator object) can still access them as long as they're located in the same package.

[Alexandru Popescu]

I liked the solution proposed with extension, but i wouldn't go for making public functionality that is not required. Defining an API is a very important task. Maybe the solution is good for a system development, but for a library/framework I wouldn't do this. Moreover exposing an API what is not functional without the context is not a good idea.

pope's 2c

[Terry Mullett]

Vinicius,

A lot of time the best value I get out of testing is the way it makes me question my design. The original code you posted had the two public methods looking like a functional decomposition of the public method that called them. In this case those two private members were only marginally "members" -- members by convenience of structure, rather than by design intent. Lasse offered a solution that would look like a strategy pattern with a bit more tweaking, and if the classes that replaced those methods had package visibility I'd say you could be done. But then comes the question: are those two methods really top-level abstractions in your mind (in which case promotion to classes could make sense) or are they just pulled out into separate methods to control ugliness (in which case promotion to classes is departing from your desing intent)? The answer to this question should also tell you whether to bother testing those methods directly, or to cover them as side effects of exercising the public method.

[Eusebio Floriano]

Originally posted by Ali Pope:
I liked the solution proposed with extension, but i wouldn't go for making public functionality that is not required. Defining an API is a very important task. Maybe the solution is good for a system development, but for a library/framework I wouldn't do this. Moreover exposing an API what is not functional without the context is not a good idea.

pope's 2c

I agree with you. Expose an API that is not fucntional without the context is not a good ideia.
The another suggestion, turn the method protected and extends the class. How could you do this if you must extends junit�s TestCase ?

[Alexandru Popescu]

I think the suggestion is to extend the original class and write a separate test unit class for extended one.

./pope

[Eusebio Floriano]

Originally posted by Terry Mullett:
Lasse offered a solution that would look like a strategy pattern with a bit more tweaking, and if the classes that replaced those methods had package visibility I'd say you could be done.

Turning the visibility to "packaged" could solve the problem.
The solution proposed by Lasse helps to maintain the methods clearer, shorter, more maintainable but isn�t it the same ideia of expose the methods to public (creating public helper classes).

Originally posted by Terry Mullett:

But then comes the question: are those two methods really top-level abstractions in your mind (in which case promotion to classes could make sense) or are they just pulled out into separate methods to control ugliness (in which case promotion to classes is departing from your desing intent)? The answer to this question should also tell you whether to bother testing those methods directly, or to cover them as side effects of exercising the public method.

The porpose of these methods is to make them smaller making it more clear and decreasing the method�s complexity.
But these private mathods are still complex enough to be tested.

Regards,

[Eusebio Floriano]

Originally posted by Ali Pope:
I think the suggestion is to extend the original class and write a separate test unit class for extended one.

./pope

So, what is the difference in test a super class or the sub class ?
Their methods will still be protected and not accessible for unit test classes.

[Alexandru Popescu]

By extending you can enlarge the visibility and so to make them public ;-).

[Eusebio Floriano]

Have you ever know about junitx ?
It seems to be an extention of junit for testing private members.

[Ilja Preuss]

Originally posted by Vinicius Boson:
Have you ever know about junitx ?
It seems to be an extention of junit for testing private members.

Yes. And it's more complicated than I like it to be. I also still think that complex private methods are a code smell - an indication that a class is doing too much.

[Alexandru Popescu]

Nice one (didn't know about this). Thanks for the tip.
The link: JUnitX.

./pope

[Ilja Preuss]

Originally posted by Terry Mullett:
But then comes the question: are those two methods really top-level abstractions in your mind (in which case promotion to classes could make sense) or are they just pulled out into separate methods to control ugliness (in which case promotion to classes is departing from your desing intent)? The answer to this question should also tell you whether to bother testing those methods directly, or to cover them as side effects of exercising the public method.

Well personally I find that testing helps me discover when my initial ideas weren't as good as I'd like. Perhaps the methods should be "top level abstractions", even if I didn't initially think of them that way...

[Ilja Preuss]

Originally posted by Ali Pope:
I liked the solution proposed with extension, but i wouldn't go for making public functionality that is not required. Defining an API is a very important task. Maybe the solution is good for a system development, but for a library/framework I wouldn't do this. Moreover exposing an API what is not functional without the context is not a good idea.

Don't mix exposing a public API of a framework (publishing a member) with making a member public. There are many public, but non-published classes coming with the JDK - for example in the com.sun.* packages.

If you don't think the two extracted classes should be published, put them into a xxx.xxx.internal package, for example.

At work, we've created a @published javadoc tag for defining the published API.

[Alexandru Popescu]

I do make a big difference between frameworks and systems, but I really do not understand how the packaging may affect the public available API. Even if SUN packaged the unwanted functionality in com.sun I have seen a few weeks ago an interesting usage of a class contained in this package. So I do not think that package naming will solve the public API issue.

again my 2c
./pope

[Terry Mullett]

Ilja,

Well personally I find that testing helps me discover when my initial ideas weren't as good as I'd like. Perhaps the methods should be "top level abstractions", even if I didn't initially think of them that way...

Indeed. Unit testing really makes you take a hard look at your design, and I will definitely tweak designs based on insights gained during test writing. I do try to be careful not to take it too far because changing a design to get around a hard testing problem could make the design unnecessarilly complex, or compromise it's conceptual integrity by introducing distractingly incohesive public classes/methods into the api. It's a tradeoff like anything else. Still, the exercise is valuable, because even if I don't or can't change the design, working through the question sharpens my abilities.

Terry
SCEA/SCBCD/SCWCD/SCJP

[Lasse Koskela]

Originally posted by Ali Pope:
Even if SUN packaged the unwanted functionality in com.sun I have seen a few weeks ago an interesting usage of a class contained in this package. So I do not think that package naming will solve the public API issue.

But you aren't afraid of people using your private methods through Reflection or after running them through a bytecode modifier at classloading time? If it's the technical restrictions that you're looking for, in the end, they're not that strong.

What I'm proposing is to let the users use your API to the fullest but make it clear when they're using non-supported features (for example, by leaving those classes/methods out from the javadocs you distribute with the API or publish on your website).

[Alexandru Popescu]

In fact I was not talking about the impossibility of running the code, but rather about the consistency of the exposed API. I am sorry to disagree on this point. I believe that OO is about encapsulation and exposed APIs, and I do not consider a solution to offer an API to unsupported functionality. I agree that leaving this outside the Javadoc will help a little, but will not conform to OO.

./pope

[Eusebio Floriano]

Originally posted by Ilja Preuss:


Yes. And it's more complicated than I like it to be.

Yeah ... At first sight it seems not be so trivial.
I think it is a good idea because you don�t need to expose your member just because test issues.
But i think it should solve part of the problem because continues because in a ejb container, you are not supposed to use reflection, right ?

Regards,

[Alexandru Popescu]

But i think it should solve part of the problem because continues because in a ejb container, you are not supposed to use reflection, right ?

Why not? Everything that is related to transaction, security, etc offered by containers is related to public API and not to private, so invoking private API using reflection will not break the container offerings. Am i completely wrong?

./pope

[Eusebio Floriano]

Originally posted by Ali Pope:
In fact I was not talking about the impossibility of running the code, but rather about the consistency of the exposed API. I am sorry to disagree on this point. I believe that OO is about encapsulation and exposed APIs, and I do not consider a solution to offer an API to unsupported functionality. I agree that leaving this outside the Javadoc will help a little, but will not conform to OO.

./pope

So do I. I agree 100% with you.

I think this subject about expose or not a member it�s very personal ideia.
If you don�t mind in expose it just for test purpose (like Lasse and Ilja do), Lase gave us a good solution.
If you do, using package visibility would be a nice try (IMHO the best one).
An alternative would be use a protected visibility and extends it.

The only one that i knew was to expose the members.
Thank you all for the suggestions ..

Regards,

[Eusebio Floriano]

Originally posted by Ali Pope:


Why not? Everything that is related to transaction, security, etc offered by containers is related to public API and not to private, so invoking private API using reflection will not break the container offerings. Am i completely wrong?

./pope

According with ejb specification, you must not use Reflection API and the compliant container is not suposed to let you do it.
Allowing it could compromise the security.

Regards,

[Jeanne Boyarsky]

"I agree with you. Expose an API that is not fucntional without the context is not a good idea. The another suggestion, turn the method protected and extends the class. How could you do this if you must extends junit�s TestCase ?"
Protected members are also package-private (which is another issue altogether.) So assuming the tests are in the same package, they can access the protected member. I prefer using package-private access because I have more control over the package though.

[Jeanne Boyarsky]

Originally posted by Vinicius Boson:


According with ejb specification, you must not use Reflection API and the compliant container is not suposed to let you do it.
Allowing it could compromise the security.

Regards,

I see no conflict with this. The tests aren't being run in the ejb container. They are just being run in the JRE/JVM. If the code is being deployed to an ejb container, the real code won't be compromised in this way. The tests don't get run in production.

Also if someone was able to use reflection on my code, they could do it regardless of whether I had tests.

[Eusebio Floriano]

Originally posted by Jeanne Boyarsky:
Protected members are also package-private (which is another issue altogether.) So assuming the tests are in the same package, they can access the protected member. I prefer using package-private access because I have more control over the package though.

I didn�t know that ..
Thx ..

[Eusebio Floriano]

Originally posted by Jeanne Boyarsky:

I see no conflict with this. The tests aren't being run in the ejb container. They are just being run in the JRE/JVM. If the code is being deployed to an ejb container, the real code won't be compromised in this way. The tests don't get run in production.

Also if someone was able to use reflection on my code, they could do it regardless of whether I had tests.

Jeanne,

I **think** that if the tests aren�t being running in the ejb container they never could access private members, even using reflection. The only thing that an remote client is the RemoteObject�s stub (local clients are in the same JVM, so it doesn�t apply) and it does not contain protected members.
Otherwise, if the tests are being running in the ejb container, the container are not supposed to allow you to use a reflection api.
Am i completely wrong ?

Regards,

[Alexandru Popescu]

Afaik the reference to Reflection inside the EJB spec refers to not trying to overpass the Java security and so brake the container security.
Moreover, a container cannot disallow Reflection.

./pope

[Lasse Koskela]

Originally posted by Ali Pope:
a container cannot disallow Reflection.

In theory, it could. EJB containers are free to check that the EJB classes it loads do not (directly or indirectly) utilise "illegal" classes like java.lang.reflect.*. However, none of the EJB containers out there is actually doing this and choose to simply state that developers shouldn't use Reflection instead of implementing a hard restriction.

[Alexandru Popescu]

Originally posted by Lasse Koskela:

In theory, it could. EJB containers are free to check that the EJB classes it loads do not (directly or indirectly) utilise "illegal" classes like java.lang.reflect.*. However, none of the EJB containers out there is actually doing this and choose to simply state that developers shouldn't use Reflection instead of implementing a hard restriction.

If you have bad intentions you can trick this mechanism ;-).

./pope

[Ilja Preuss]

Originally posted by Ali Pope:

Moreover, a container cannot disallow Reflection.

It can at least disallow accessing private members through reflection.

[Eusebio Floriano]

Originally posted by Lasse Koskela:

In theory, it could. EJB containers are free to check that the EJB classes it loads do not (directly or indirectly) utilise "illegal" classes like java.lang.reflect.*. However, none of the EJB containers out there is actually doing this and choose to simply state that developers shouldn't use Reflection instead of implementing a hard restriction.

yeah ... According with the spec 2.0 on page 498:

Some Containers may allow the Deployer to grant more, or fewer, permissions to the enterprise bean
instances than specified in Table 19. Support for this is not required by the EJB specification. Enterprise
beans that rely on more or fewer permissions will not be portable across all EJB Containers.

So, it�s up to the container provider to allow or not you to use Reflection but it�s very bad ideia use it.
So, back to the road, the only way i see to test private members (specific for ejb) is exposing it (at lest for package visibility).

Regards,
[ October 13, 2004: Message edited by: Vinicius Boson ]

[Ilja Preuss]

Originally posted by Ali Pope:
I am sorry to disagree on this point. I believe that OO is about encapsulation and exposed APIs, and I do not consider a solution to offer an API to unsupported functionality.

I don't consider making a method public being equivalent to exposing an API. I don't even consider encapsulation being equivalent to making methods non-public.

Smalltalk, one of the first OO languages, doesn't even have non-public methods!

I agree that leaving this outside the Javadoc will help a little, but will not conform to OO.

I'm not sure what you mean by "conform to OO". What value does your "conformance to OO" have?

[Alexandru Popescu]

IMO exposed API is the interface offered by an object for allowing interaction/collaboration with other objects. While in Java - a not pure OO language - the exposed API is not always public, due to access modifiers and packaging, I do consider thatt public methods are part of the exposed API.
I cannot talk about Smalltalk as I have not working knowledge of it.
Going to encapsulation:

Encapsulation hides detailed internal specification of an object, and publishes only its external interfaces. Thus, users of an object only need to adhere to these interfaces. By encapsulation, the internal data and methods of an object can be changed without changing the way of how to use the object.

I am not sure what will be the usage of an unusable object interface (as we remember the methods in cause where considered unusable without a context).

./pope

[Lasse Koskela]

Originally posted by Ali Pope:
I am not sure what will be the usage of an unusable object interface (as we remember the methods in cause where considered unusable without a context).

Ah, but how do we define "context"? If you're writing a compiler, surely you won't object to splitting the lexer and the parser to different classes based on them being unusable without, well, whatever context you consider the whole compiler to have?

My point is that while the method might not provide any meaningful value to the end user as an independent piece of code, it may be meaningful to some other class/method in your code base.

Maybe we're thinking about two very different scenarios? Could you present an example where you wouldn't want to make a method public based on this lack of context? (maybe in a new discussion thread?)

[J. B. Rainsberger]

Let�s supose that it�s necessary to validate an student�s school registration.

 
public void validateRegistration(Student student) {
   // do something
 
   validateGrade(student.getSomeData());
 
   validateFinancialPayment(student.getOtherData());
 
   // do other things
 
} 
 
private void validateGrade(SomeData sd) {
  //complex code
}
 
private void validateFinancialPayment(OtherData od) {
  //complex code
}

validateXXX is a part of the whole validateRegistragion. They don�t have a real meaning by themselves but they are complex enought to deserve to be tested.
I wouldn�t like to expose them JUST because i want to test them.
What do you suggest me to do ?

Regards,[/QB]

Here is what I have done in the past, and will do again.

Validating input (whether semantically or syntactically, the approach is the same) is based on a sequence of rules. Each rule defines an aspect of the input that makes it correct or incorrect. Generally speaking, input is valid if and only if the input passes all the validation rules involved.

interface Rule {
    // If a rule does not pass, then add some failures
    // to the list to indicate why.
    boolean passes(List failures);
}
 
class GradeRule implements Rule {
    private final Grade grade;
 
    public GradeRule(Grade grade) {
        this.grade = grade;
    }
 
    boolean passes(List failures) {
        // add failure objects describing why it's invalid, if any
        // return true if valid, false if invalid
    }
}
 
...
 
class ShortCircuitRuleset implements Rule {
    private List rules = new ArrayList();
 
    void addRule(Rule rule) { rules.add(rule); }
 
    boolean passes(List failures) {
        for (Iterator i = rules.iterator(); i.hasNext(); ) {
            Rule each = (Rule) i.next();
            if (each.passes(failures) == false) { return false; }
        }
        return true;
    }
}
 
...
 
public void validateRegistration(Student student) {
    ShortCircuitRuleset rules = new ShortCircuitRuleset();
    rules.add(new GradeRule(student.getData());
    ...
 
    List failures = new ArrayList();
    boolean valid = rules.passes(failures);
 
    if (!valid) {
        displayFailures(failures);    // Or whatever...
    }
 
    ...
}

I can reuse ShortCircuitRuleset all over the place, but I only need to test it once for an arbitrary set of rules that might pass or fail. It might require six tests. It is very easy (by comparison) to test each part of the validation rule in isolation. By testing each part of the rule in isolation, you will find it easier to make sure you cover more and better failure conditions.

Future refactorings, if appropriate, are easier: you could potentially externalize the validation rules as data in files. For syntactic validation, I have in past made these rules simple regular expression matches that were stored outside the system in easy-to-change text files. I could even put them directly into web page templates next to the widget the validation rules belong to!

This is an example of taking private methods that really don't want to be private and changing the design in such a way to make the whole thing easier to test and better suited for change. The duplication in all those private methods wasn't obvious before, and now it is: each method looks at data, decides whether it's valid and registers some kind of failure message if it isn't. Now that duplicate behavior is explicit and easy to see. We have even leveraged it by using the Composite pattern!

Take care.

[Jeff Langr]

Excellent answer JB!

Other than writing tests for everything in the first place (using TDD is the easiest way to accomplish this), eliminating all duplication is what will provide you with a system that can be maintained and extended at low cost. That of course means you learn how to spot and eliminate duplication whereever it exists, in even the most subtle of places.

http://www.xprogramming.com/xpmag/expEmergentDesign.htm

Following the simple rules of emergent design often eliminates the angst over the side concerns (e.g. the need to break encapsulation as in the above discussion).

-Jeff-
[ October 13, 2004: Message edited by: Jeff Langr ]