How can I refactor and unit test complex legacy Java EE5 EJB methods?

My colleagues and I are currently introducing unit tests to our legacy Java EE5 codebase. We use mostly JUnit and Mockito. In the process of writing tests, we have noticed that several methods in our EJBs were hard to test because they did a lot of things at once.

I'm fairly new to the whole testing business, and so I'm looking for insight in how to better structure the code or the tests. My goal is to write good tests without a headache.

This is an example of one of our methods and its logical steps in a service that manages a message queue:

consumeMessages
- acknowledgePreviouslyDownloadedMessages
- getNewUnreadMessages
- addExtraMessages (depending on somewhat complex conditions)
- markMessagesAsDownloaded
- serializeMessageObjects

The top-level method is currently exposed in the interface, while all sub-methods are private. As far as I understand it, it would be bad practice to just start testing private methods, as only the public interface should matter.

My first reaction was to just make all the sub-methods public and test them in isolation, then in the top-level method just make sure that it calls the sub-methods. But then a colleague mentioned that it might not be a good idea to expose all those low-level methods at the same level as the other one, as it might cause confusion and other developers might start using when they should be using the top-level one. I can't fault his argument.

So here I am.

How do you reconcile exposing easily testable low-level methods versus avoiding to clutter the interfaces? In our case, the EJB interfaces.

I've read in other unit test questions that one should use dependency injection or follow the single responsibility principle, but I'm having trouble applying it in practice. Would anyone have pointers on how to apply that kind of pattern to the example method above?

Would you recommend other general OO patterns or Java EE patterns?

Solution

At first glance, I would say that we probably need to introduce a new class, which would 1) expose public methods that can be unit tested but 2) not be exposed in the public interface of your API.

As an example, let's imagine that you are designing an API for a car. To implement the API, you will need an engine (with complex behavior). You want to fully test your engine, but you don't want to expose details to the clients of the car API (all I know about my car is how to push the start button and how to switch the radio channel).

In that case, what I would do is something like that:

public class Engine {
  public void doActionOnEngine() {}
  public void doOtherActionOnEngine() {}
}


public class Car {
  private Engine engine;

  // the setter is used for dependency injection
  public void setEngine(Engine engine) {
    this.engine = engine;
  }

  // notice that there is no getter for engine

  public void doActionOnCar() {
    engine.doActionOnEngine();
  }

  public void doOtherActionOnCar() {
    engine.doActionOnEngine();
    engine.doOtherActionOnEngine(),
  }
}

For the people using the Car API, there is no way to access the engine directly, so there is no risk to do harm. On the other hand, it is possible to fully unit test the engine.