Is it bad practice to use Reflection in Unit testing? [duplicate]

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)

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.

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.

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.

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.