122

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)

5
  • What kind of access do you practice with reflection, set, get or both?
    – fish
    Commented May 11, 2010 at 14:00
  • @fish: i only use get to verify if some specific values are setted Commented May 11, 2010 at 14:02
  • 8
    It's normal practice in Java to put the tests in the same package so that you can test "package private" (default access) classes. Commented May 11, 2010 at 14:17
  • 9
    One addition to the previous comment (on putting tests in the same package) is that you put the tests in a separate source tree that isn't deployed with the application.
    – deterb
    Commented May 11, 2010 at 16:19
  • Adding getters for those variables should not be that big a deal then, especially if you can convince your supervisor to accept the tests in different source tree but in the same package (which is the common practice as already mentioned). IMHO, it's always a bad idea to access the fields directly, whether using reflection or just having loose visibility.
    – fish
    Commented May 12, 2010 at 6:24

7 Answers 7

98

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.

3
  • 12
    Reflection is obviously useful within the test framework for calling tests and mocking interfaces, as an alternative to compile-time annotations processors. But hat should be clearly noted as a separate type of usage. Commented May 11, 2010 at 14:19
  • 3
    @tackline, this is what I really meant, thanks for pointing it out. I added a clarification to my answer. Commented May 11, 2010 at 14:34
  • 1
    You don't need a lot of qualifiers. Reflection should mostly be used as a last resort, period. It makes reasoning about the program effectively impossible.
    – Ira Baxter
    Commented Apr 14, 2011 at 6:40
74

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.

6
  • 6
    There's two sides to the production API changing. Changing it just for the tests is bad, while updating your API/design so that your code is more testable (and thus you're not changing your visibility just for the tests) is a good practice.
    – deterb
    Commented May 11, 2010 at 16:21
  • 2
    if you are sure that what you are doing doesn't have side-effects - yes. ;)
    – Bozho
    Commented May 11, 2010 at 16:25
  • If you don't have tests to test your api changes that you have made so you can write tests... Bozho has a very valid point and answer.
    – Justin
    Commented May 12, 2010 at 6:40
  • I faced this problem: I had to test the invariant of a class without exposing all its details to clients. Reflection was useful in that case. Commented Jul 19, 2013 at 13:39
  • 2
    But note that changing the visibility of a member from private to package-private has nil consequence for consumers of your package, unless they are writing code in your package's namespace. This particular change to scope may be tolerable. Commented Nov 6, 2015 at 3:10
36

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.

3
  • 3
    Agreed. Treat the object being tested as a black box - make sure the outputs match the inputs. If they don't you know you have an problem with the internals of the implementation.
    – AngerClown
    Commented May 12, 2010 at 17:39
  • 1
    But what about testing object state? Commented Jan 22, 2015 at 11:38
  • 8
    The object's internal state doesn't matter, as long as its external behavior is correct. Commented Jan 22, 2015 at 13:23
7

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.

2
  • +1 For changing production code visibility not being a good solution Commented May 11, 2010 at 14:03
  • @Yishai But what if we have same class name issue in local as well as in dependent jars ? How can the protected methods be tested in Java gradle ? Need help on this. Commented Apr 2, 2021 at 6:32
6

I second the thought of Bozho:

It is really bad to modify the visibility of a production API just for the sake of testing.

But if you try to do the simplest thing that could possibly work then it might be preferred to writing Reflection API boilerplate, at least while your code is still new/changing. I mean, the burden of manually changing the reflected calls from your test every time you change the method name, or params, is IMHO too much burden and the wrong focus.

Having fallen in the trap of relaxing accessibility just for testing and then inadvertently accessing the was-private method from other production code i thought of dp4j.jar: it injects the Reflection API code (Lombok-style) so that you don't change the production code AND don't write the Reflection API yourself; dp4j replaces your direct access in the unit test with the equivalent reflection API at compile-time. Here is an example of dp4j with JUnit.

5

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.

4

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.

Not the answer you're looking for? Browse other questions tagged or ask your own question.