9

Some times I found myself in situations in which unit tests would be easier if I change the visibility of some methods from private to package private in order to either facilitate unit test mocking, assertions...

One example would be this

Say I have an object A that contains 4 attributes X, Y, Z and R where X, Y and Z are sets and R are relations among different elements of each set, for example a relation will be composed of an element of X , an element of Y and an element of Z. The object A does not allow direct access to X, Y, Z or R, instead it provides a rich API which allows you to create new elements on X, Y and Z, also allows you to mix those elements into new R elements. For a unit testing it would be highly convenient to have a public getX(), public getY(), public getZ() and public getR() methods, so I can make exact assertions about the internals of the object each time I call the object API. However, exposing X, Y and Z is something I want to prevent, which is why from the very beggining the object make those elements private and only provided indirect access to them by using its API. Would it however make sense to provide package private methods getX(), getY(), getZ() and getR() so that at least form the unit test I can easily check if the internal state of the object is the expected one?

The drawback is of course that the visibility of the method is increased and given that such method was private for a good reason, It feels a little bit weird.

Of course I could use reflection to achieve the same, but that feels even dirtier.

So the question is, is this a good or a bad practice? is it a code smell? does it happen to someone else? is there a better technique for this?

Juan Antonio Gomez Moriano
  • 13,103
  • 10
  • 47
  • 65
  • Can you provide a specific example? Generally speaking, the preferred approach is to rework the API so that you can directly mock dependencies of the object under test. – chrylis -cautiouslyoptimistic- Mar 05 '15 at 00:10
  • IMO increasing visibility for the sake of unit tests is wrong. The method has that visibility for a reason. – JamesB Mar 05 '15 at 00:16
  • See http://stackoverflow.com/questions/8599023/visiblefortesting-is-a-smell-that-suggests-which-refactoring – dnault Mar 05 '15 at 01:22
  • possible duplicate of [How to test a class that has private methods, fields or inner classes](http://stackoverflow.com/questions/34571/how-to-test-a-class-that-has-private-methods-fields-or-inner-classes) – NamshubWriter Mar 05 '15 at 04:48

5 Answers5

4

If you want to test a private method you have a few options:

  1. Dependency Injection

If you really need to test a private method it's very possible that the method should actually be a public one of another class (code smell). This means that if you have the private method 'm' in the class 'A', you might consider moving 'm' as a public method in a class 'B' and then inject 'B' into 'A'.

This technique is quite common and most of the times it does the trick. Nevertheless, it's only meaningful when you move a significant part of your code from the original class ('A') to the new class ('B').

This solution is Mockito friendly.

  1. Test Double Inheritance

Change the visibility of the private method 'm' in the class 'A' to protected. Then create a class 'B' in the test folder that extends 'A'. Finally, create a public method 'pm' in 'B' with the same signature as 'm' that just calls 'm'. This way you can "make" the non-visible 'm' method in 'A' accessible though 'pm' in 'B'.

This solution is also Mockito friendly.

  1. Groovy

Use a JVM dynamic language such as Groovy that allows you to call private methods.

  1. Reflection

Get the private method using reflection and then change its visiblity. See this.

  1. Package Visibility

Declare the method as package (no visibility constraint) and declare the test class in the same package. I personally don't like using this one because I don't find elegant Java's package visibility.

Community
  • 1
  • 1
Pau Carre
  • 471
  • 2
  • 5
  • +1 for "If you really need to test a private method it's very possible that the method should actually be a public one of another class" – Adam Johns Aug 01 '19 at 16:34
3

Generally the good practice is to not expose the internal logics. Instead you have to make your classes configurable. For example if your class needs other components like let's say HttpComponent or etc. try to use different dependency injection techniques to provide those dependencies. Then in tests, you can mock those dependencies and verify those mocks.

In your case, it depends on the context. Most of the time, you test the private functions as part of the testing the public functions. So you test the public behaviour for different cases and if all pass it means the private function which has been invoked via that public function works too.

Nami
  • 1,215
  • 11
  • 21
1

General suggestion would be to test only public API of the class (that invokes private API and being tested as well). Otherwise in case of refactoring your internal API you will need to refactor most of the tests.

David stands with Ukraine
  • 2,267
  • 2
  • 18
  • 23
0

It sounds like A is doing too many things if it's managing X,Y,Z and R.

Perhaps the a good idea would be to refactor your code to make R a separate class that takes X,Y and Z as input parameters. The class could be made package private if you wish. Eitherway, you would be able to test R directly by providing different X,Y and Zs.

dkatzel
  • 31,188
  • 3
  • 63
  • 67
-1

For unit test, you can have some problems with testing and mocking private, static methods and constructors. So you have the choice to use one of the two solutions :

  1. Don't use private method, and make them package visible. Also don't use static methods and make them in a singleton class.

  2. Use a mock library that perform such tests like powermock. It can also mock constructor and static method.

Bad or good solution, I guess there is not, it depend on the strategies of the project, and on the decision to make to test them.

Bouhanef
  • 72
  • 5