Do not use magic in unit tests

Greetings!

Working in legacy system is both fun and hard. It becomes harder due to lack of tests. Not only that, it is difficult to write tests due to the ways code is written.
As our legacy system doesn't have a DI framework like Spring, we are maintaining object lifecycle manually. We have a policy to create singletons (I do not know why, I plan to remove them anyway) which made them harder for unit tests.
The way to write unit tests for these classes is using reflection and mocking frameworks (scary isn't).
Let's see a basic example of such a class.

public final class AppointmentRepository {
  private AppointmentRepository() {
  }

  private static class LazyHolder {
    private static final AppointmentRepository INSTANCE = new AppointmentRepository();
  }

  public static AppointmentRepository getInstance() {
    return LazyHolder.INSTANCE;
  }
}

public final class AppointmentService {
  private final AppointmentRepository appointmentRepository;

  private AppointmentService() {
    appointmentRepository = AppointmentRepository.getInstance();
  }

  private static class LazyHolder {
    private static final AppointmentService INSTANCE = new AppointmentService();
  }

  public static AppointmentService getInstance() {
    return LazyHolder.INSTANCE;
  }
}

How do you write unit tests for AppointmentService?

import org.fest.reflect.core.Reflection;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.testng.Assert;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

public class AppointmentServiceTest {
  @Mock
  private AppointmentRepository appointmentRepository;
  private AppointmentService appointmentService;

  @BeforeMethod
  public void setUp() {
    appointmentService = AppointmentService.getInstance();
    MockitoAnnotations.initMocks(this);
    Reflection.field("appointmentRepository")
      .ofType(AppointmentRepository.class)
      .in(appointmentService)
      .set(appointmentRepository);
  }

  @Test
  public void testHello() {
    Assert.assertTrue(true);
  }
}

With that setup Unit test is working. However, there is a big problem in this unit test. Can you spot it!
AppointmentService has hidden dependencies which are initialized in the constructor. When we call the getInstance, it will create the objects with original classes!! With reflection we are setting mocks but too late.
This unit test;

• Wastes resources by creating objects unnecessarily
• It will be slow as it creates actual objects

Understand the problem first

When you write a class, you should be able to unit test it purely without any framework magic. If you have to rely on frameworks, you are doing it wrong. You need to write testable code. Unit test always trump.

• Hidden dependencies.
• We don't have a way to inject dependencies as we wish.
• No proper interfaces.
• Cannot extend to create stubs (if interface is not necessary) because the usage of final

How can we solve this?

Existing code

If it is an existing code, you might not be able refactor it fully. In such a cases, introduce test method which accepts dependencies. You can use that method to create objects in unit tests, use existing getInstance in production code. It looks like this breaks the single but remember you are working as a team. Who is going to break it?

New code

I have no idea why do people write new codes in this way. I always prefer to use parameterized constructors which is always safe and easy to use.

public class AppointmentService {
  private AppointmentRepository appointmentRepository;

  public AppointmentService(AppointmentRepository appointmentRepository) {
    this.appointmentRepository = appointmentRepository;
  }
}

Look at this now. It is very clear what it depends on. You can easily send a dummy objects (with or wihout using mock framework)

How can I create object in production then?

We have inverted the dependencies, however due to the lack of DI container, we need to create/maintain objects ourserlves. One technique I would like to use is a factory. More precisely a ServiceLocator which is an old pattern but useful in this situation. We can combine Flyweight to maintain Singleton behaviour as well.
There should be an entry point anyway, an ejb in my case. What I do is, I use my Factory there by making other classes pure.

import static java.util.Objects.isNull;

public class AppointmentObjectAssembler {
  private AppointmentRepository appointmentRepository;
  private AppointmentService appointmentService;

  public AppointmentRepository getAppointmentRepository() {
    if (isNull(appointmentRepository)) {
      appointmentRepository = new AppointmentRepository();
    }
    return appointmentRepository;
  }

  public AppointmentService getAppointmentService() {
    if (isNull(appointmentService)) {
      appointmentService = new AppointmentService(getAppointmentRepository());
    }
    return appointmentService;
  }
}

Eventhough this looks little ugly, when we move to a DI framework oneday, this is the only place we need to change. That change also will be minimum as for example this looks like a Spring configuration class.

Summary

Write unit testable code always. Trust your team to follow few guidelines.