I want to test a private method - is there something wrong with my design? [duplicate]

Question

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.

Answer 1

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.