This question already has answers here:
Closed 2 years ago.
So I'm extremely new to software testing, and am looking at adding a couple of tests to one of my applications. I have a public method addKeywords() which along the way calls a private method removeInvalidOperations(). This private method makes calls to an external API and is ~50 lines of code. As I consider this to be a somewhat complex method I would like to test this without having to do so through calling the addKeyword() method. Yet this does not seem to be possible (at least not though JUnit).
The information I have looked at is suggesting that the desire to test ones private methods could be a code smell. Some people suggest that it may be a sign that this should be refactored into a separate class and made public. Also, there are the suggestions that if you really need to then you can make edits to your production code e.g. change the visibility of the private method.
I don't really see why I have a problem with my current code design, but also don't like the idea of editing my production code to suit my tests needs.
As I say - I'm rather new to testing, so any help is much appreciated. Also, please let me know if there is any further info I can provide to help with the answers.
I suggest refactoring it.
The information I have looked at is suggesting that the desire to test
ones private methods could be a code smell. Some people suggest that
it may be a sign that this should be refactored into a separate class
and made public.
You have covered the various reasons for and against it in your own question, you seem to be well aware of the arguments. But you have a method that seems rather complicated and involves an external API. That is worth testing on its own. removeInvalidOperations() can still be a private method on the class it is in, but it would basically delegate to another dependency.
class YourClass
{
private OperationRemover remover;
public void addKeywords() {
// whatever
removeInvalidOperations();
}
private void removeInvalidOperations() {
remover.remove();
}
}
This gives you the added benefit of being able to replace this dependency at some point, including being able to test your addKeywords() method without actually placing an external API call, which would make testing that method easier. OperationRemover could be an interface, for example, and for testing purposes, you simply pass in a stub in its place instead of the concrete version used in production. As for your concrete version, you can write tests for it independently of what is happening with your existing class.
I don't really see why I have a problem with my current code design,
but also don't like the idea of editing my production code to suit my
tests needs.
Easier testability is a side-benefit. Look at it another way: What you are actually doing is making the code loosely-coupled and extensible. Above, we have separated the call to the external API from the code that might need to use the result. The external API could change. You might go from one service to another, but the code that uses the result does not have to care. That class can stay the same, only the class that actually places the calls needs to be modfied (or replaced).
Real world example: The year is 2007 and you work for a bank in a large financial center in the United States. Your application needs to use account information. Your code reaches out to a web service of some sort inside the bank and gets the information it needs, in the shape it needs, then goes on about its processing. In 2008, the US financial sector implodes, and your bank (which is on the verge of collapse) is gobbled up by another bank. Your application is spared, except now you have to reach out to a different API that already exists within the surviving bank to get account information from there. Should the code that consumes this account information need to change? Not necessarily. It's the same account information as before, just from a different source. No, all that needs to change is the implementation invoking the APIs. Consuming code never has to know.
The fact that such loose coupling also promotes and facilitates testing is a bonus.
If it's private, it can't be considered part of your application's API, so testing it is indeed a code smell - when the test breaks, is that OK or not?
Unit tests are supposed to be functionality oriented, not code oriented. You test units of functionality, not units of code.
Regardless of philosophy, a class outside of your implementation cannot access the private method without hacking the JVM, so you're out of luck - you either have to change the method's visibility, make a protected testing API the unit-test class extends, or test the function indirectly by calling the public methods making use of it.
Usually you wouldn't want to test a private method, but there are exceptions.
You might be tempted to test a private method if:
You haven't thought carefully about how to test the private method
indirectly by calling the existing public methods.
Your class's API is too inflexible. The public methods need more
parameters, or some of the private methods need to be made public.
Your class's API is flexible enough, but underneath the public
methods it has some pretty complicated private methods going on
underneath.
Based on your question, you could be in any of those cases.
For (1), obviously you should first try to find a way to test your private method with the existing public methods.
For (2) and (3), unit tests won't tell you which case you're in. What you need to do is write some example code. As Josh Bloch recommends, code some use cases for your API. Your API should be the minimal set of public methods required to satisfy your use cases.
(3) is the case where it's OK to test private methods. There are various tricks for that. For production code, these are better than exposing your method to the API user (making it public) just so you can test it. Or splitting related functionality into 2 classes just so you can test it.
Instead of thinking in terms of "code smells", which is imprecise and subjective, you can think in terms of information hiding. Design decisions that are likely to change should not be exposed in your public API. Preferably, design decisions that are likely to change should not be exposed to your unit tests either -- that's why people recommend against testing private methods.
But if you really think it's important to unit-test your private method, and if you can't do it adequately via the public methods, then don't sacrifice the correctness of your code! Test the private method. Worst case is your test code is messier, and you have to rewrite the tests when the private method changes.
If you don't want to call addKeywords(), perhaps you should just add another public method testRemoveInvalidOperations(), which just calls the private removeInvalidOperations(). You can remove the test at a later date.
