Is it bad practice to use Reflection in Unit testing?

Question

During the last years I always thought that in Java, Reflection is widely used during Unit testing. Since some of the variables/methods which have to be checked are private, it is somehow necessary to read the values of them. I always thought that the Reflection API is also used for this purpose.

Last week i had to test some packages and therefore write some JUnit tests. As always i used Reflection to access private fields and methods. But my supervisor who checked the code wasn't really happy with that and told me that the Reflection API wasn't meant to use for such "hacking". Instead he suggested to modifiy the visibility in the production code.

Is it really bad practice to use Reflection? I can't really believe that-

Edit: I should have mentioned that i was required that all tests are in a separate package called test (so using protected visibilty e.g. wasn't a possible solution too)

Answer 1

IMHO Reflection should really only be a last resort, reserved for the special case of unit testing legacy code or an API you can't change. If you are testing your own code, the fact that you need to use Reflection means your design is not testable, so you should fix that instead of resorting to Reflection.

If you need to access private members in your unit tests, it usually means the class in question has an unsuitable interface, and/or tries to do too much. So either its interface should be revised, or some code should be extracted into a separate class, where those problematic methods / field accessors can be made public.

Note that using Reflection in general results in code which, apart from being harder to understand and maintain, is also more fragile. There are a whole set of errors which in the normal case would be detected by the compiler, but with Reflection they crop up as runtime exceptions only.

Update: as @tackline noted, this concerns only using Reflection within one's own test code, not the internals of the testing framework. JUnit (and probably all other similar frameworks) uses reflection to identify and call your test methods - this is a justified and localized use of reflection. It would be difficult or impossible to provide the same features and convenience without using Reflection. OTOH it is completely encapsulated within the framework implementation, so it does not complicate or compromise our own testing code.

Answer 2

It is really bad to modify the visibility of a production API just for the sake of testing. That visibility is likely to be set to its current value for valid reasons and is not to be changed.

Using reflection for unit testing is mostly fine. Of course, you should design your classes for testability, so that less reflection is required.

Spring for example has ReflectionTestUtils. But its purpose is set mocks of dependencies, where spring was supposed to inject them.

The topic is deeper than "do & don't", and regards what should be tested - whether the internal state of the objects needs to be tested or not; whether we should afford questioning the design of the class under test; etc.

Answer 3

I would regard it as a bad practice, but just changing visibility in the production code isn't a good solution, you have to look at the cause. Either you have an untestable API (that is the API doesn't expose enough for a test to get a handle on) so you are looking to test private state instead, or your tests are too coupled with your implementation, which will make them of only marginal use when you refactor.

Without knowing more about your case, I can't really say more, but it is indeed regarded as a poor practice to use reflection. Personally I would rather make the test a static inner class of the class under test than resort to reflection (if say the untestable part of the API was not under my control), but some places will have a larger issue with the test code being in the same package as the production code than with using reflection.

EDIT: In response to your edit, that is at least as poor a practice as using Reflection, probably worse. The way it is typically handled is to use the same package, but keep the tests in a separate directory structure. If unit tests don't belong in the same package as the class under test, I don't know what does.

Anyway, you can still get around this problem by using protected (unfortunately not package-private which is really ideal for this) by testing a subclass like so:

 public class ClassUnderTest {
      protect void methodExposedForTesting() {}
 }

And inside your unit test

class ClassUnderTestTestable extends ClassUnderTest {
     @Override public void methodExposedForTesting() { super.methodExposedForTesting() }
}

And if you have a protected constructor:

ClassUnderTest test = new ClassUnderTest(){};

I don't necessarily recommend the above for normal situations, but the restrictions you are being asked to work under and not "best practice" already.

Answer 4

I think your code should be tested in two ways. You should test your public methods via Unit Test and that will act as our black box testing. Since your code is broken apart into manageable functions(good design), you'll want to unit test the individual pieces with reflection to make sure they work independent of the process, the only way I can think of doing this would be with reflection since they are private.

At least, this is my thinking in the unit test process.

Answer 5

From the perspective of TDD - Test Driven Design - this is a bad practice. I know you didn't tag this TDD, nor specifically ask about it, but TDD is a good practice and this goes against its grain.

In TDD, we use our tests to define the interface of the class - the public interface - and therefore we're writing tests that interact directly only with the public interface. We are concerned with that interface; appropriate access levels are an important part of design, an important part of good code. If you find yourself needing to test something private, it's usually, in my experience, a design smell.

Answer 6

To add to what others have said, consider the following:

//in your JUnit code...
public void testSquare()
{
   Class classToTest = SomeApplicationClass.class;
   Method privateMethod = classToTest.getMethod("computeSquare", new Class[]{Integer.class});
   String result = (String)privateMethod.invoke(new Integer(3));
   assertEquals("9", result);
}

Here, we are using reflection to execute the private method SomeApplicationClass.computeSquare(), passing in an Integer and returning a String result. This results in a JUnit test which will compile fine but fail during execution if any of the following occur:

• Method name "computeSquare" is renamed
• The method takes in a different parameter type (e.g. changing Integer to Long)
• The number of parameters change (e.g. passing in another parameter)
• The return type of the method changes (perhaps from String to Integer)

Instead, if there is no easy way to prove that the computeSquare has done what it is supposed to do through the public API, then your class is likely trying to do to much. Pull this method into a new class which gives you the following test:

public void testSquare()
{
   String result = new NumberUtils().computeSquare(new Integer(3));
   assertEquals("9", result);
}

Now (especially when you use the refactoring tools available in modern IDE's), changing the name of the method has no effect on your test (as your IDE will also have refactored the JUnit test as well), while changing the parameter type, number of parameters or return type of the method will flag a compile error in your Junit test meaning you won't be checking in a JUnit test which compiles but fails at runtime.

My final point is that sometimes, especially when working in legacy code, and you need to add new functionality, it may not be easy to do so in a separate testable well written class. In this instance my recommendation would be to isolate new code changes to protected visibility methods which you can execute directly in your JUnit test code. This allows you to begin building up a base of test code. Eventually you should refactor the class and extract your added functionality, but in the meantime, protected visibility on your new code can sometimes be your best way forward in terms of testability without major refactoring.