1

I have a public method that calls group of private methods.

I would like to test each of the private method with unit test as it is too complicated to test everything through the public method ,

Is think it will be a bad practice to change method accessibility only for testing purposes.

But I dont see any other way to test it (maybe reflection , but it is ugly)

Michael Lloyd Lee mlk
  • 14,561
  • 3
  • 44
  • 81
MoShe
  • 6,197
  • 17
  • 51
  • 77
  • 1
    I was really hoping the suggested edit would change `pubic method` to `public method` - was disappointed. – Idos Feb 22 '16 at 11:39
  • Possible duplicate of http://stackoverflow.com/questions/34571/how-to-test-a-class-that-has-private-methods-fields-or-inner-classes – Alex Feb 22 '16 at 11:41
  • You could write your tests in groovy then you can access private methods. Groovy brings also the advantage that you can easily create mocks. – eztam Feb 22 '16 at 11:42
  • You can either make the package local, or question whether you should be testing private methods at all, try testing the public methods which call these methods. – Peter Lawrey Feb 22 '16 at 11:42
  • 2
    Your public method is the thing you care about. Testing the private methods in isolation will NOT guarantee ANYTHING about the public one that calls them. If things are "too complicated" to be tested through the public method, you are in high time for a refactoring. If you post some code we can maybe help out more. – Diego Martinoia Feb 22 '16 at 11:45
  • I'm quite new to TDD, but i'm getting quite angry at the responses, in general, regarding that issue. Under what logic is it wrong not to test **public** methods, but completely ok not to test **private** methods? As if there was a fundamental difference between the two. And saying "you test the privates methods by testing the public methods that call them" is a nonsense to me. If i have a public method, which calls 2 private methods, which in turn call 2 private methods each, what do i do? That's 6 private methods for 1 public method. When the test fails, where do i look? – Loïc N. May 07 '17 at 01:29

4 Answers4

2

Private methods should only exist as a consequence of refactoring a public method, that you've developed using TDD.

If you create a class with public methods and plan to add private methods to it, then your architecture will fail.

I know it's harsh, but what you're asking for is really, really bad software design.

I suggest you buy Uncle Bob's book "Clean Code"

http://www.amazon.co.uk/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882

Which basically gives you a great foundation for getting it right and saving you a lot of grief in your future as a developer.

Pedro G. Dias
  • 3,162
  • 1
  • 18
  • 30
2

There is IMO only one correct answer to this question; If the the class is too complex it means it's doing too much and has too many responsibilities. You need to extract those responsibilities into other classes that can be tested separately.

So the answer to your question is NO!

What you have is a code smell. You're seeing the symptoms of a problem, but you're not curing it. What you need to do is to use refactoring techniques like extract class or extract subclass. Try to see if you can extract one of those private methods (or parts of it) into a class of itself. Then you can add unit test to that new class. Divide and conquer untill you have the code under control.

Kjetil Klaussen
  • 6,266
  • 1
  • 35
  • 29
1

You could, as has been mentioned, change the visibility from private to package, and then ensure that the unit-tests are in the same package (which should normally be the case anyway).

This can be an acceptable solution to your testing problem, given that the interfaces of the (now) private functions are sufficiently stable and that you also do some integration testing (that is, checking that the public methods call the private ones in the correct way).

There are, however, some other options you might want to consider:

  • If the private functions are interface-stable but sufficiently complex, you might consider creating separate classes for them - it is likely that some of them might benefit from being split into several smaller functions themselves.
  • If testing the private functions via the public interface is inconvenient (maybe because of the need for a complex setup), this can sometimes be solved by the use of helper functions that simplify the setup and allow different tests to share common setup code.
Dirk Herrmann
  • 5,550
  • 1
  • 21
  • 47
1

You are right, changing the visibility of methods just so you are able to test them is a bad thing to do. Here are the options you have:

Test it through existing public methods. You really shouldn't test methods but behavior, which normally needs multiple methods anyway. So stop thinking about testing that method, but figure out the behavior that is not tested. If your class is well designed it should be easily testable.

Move the method into a new class. This is probably the best solution to your problem from a design perspective. If your code is so complex that you can't reach all the paths in that private method, parts of it should probably live in their own class. In that class they will have at least package scope and can easily be tested. Again: you should still test behavior not methods.

Use reflection. You can access private fields and methods using reflection. While this is technical possible it just adds more legacy code to the existing legacy code in order to hide the legacy code. In the general case a rather stupid thing to do. There are exceptions to this. For example is for some reason you are not allowed to make even the smallest change to the production source code. If you really need this, google it.

Just change the visibility Yes it is bad practice. But sometimes the alternatives are: Make large changes without tests or don't test it at all. So sometimes it is ok to just bite the bullet and change the visibility. Especially when it is the first step for writing some tests and then extracting the behavior in its own class.

Jens Schauder
  • 77,657
  • 34
  • 181
  • 348