Imagine that you need to test the following piece of code:

1. What red flags do you see which will prevent you from writing meaningful unit-tests?
2. How would you rewrite this class to make it easier to test?
3. What are the dependencies of this code?

1. What kind of assumptions can you make about the ease of testing the dependencies?

public class InboxSyncer {

  private static final InboxSyncer instance = new InboxSyncer(Config.get());

  public static getInstance() { return instance; }

  private final Certificate cert;

  private final String username;

  private final String password;

  private long lastSync = -1;

  private InboxSyncer(Config config) {

    this.cert = DESReader.read(config.get(“cert.path”));

    User user = config.getUser();

    this.username = user.getUsername();

    this.password = user.getPassword();

  }

  public sync() {

    long syncFrom = lastSync;

    if (syncFrom == -1) {

      syncFrom = new Date().getTime() - Defaults.INBOX_SYNC_AMOUNT;

    }

    Inbox inbox = Inbox.get(username);

    POPConnector pop = new POPConnector(cert, username, password);

    pop.connect();

    try {

      Iterator<Messages> messages = pop.messagesIterator();

      Message message;

      while ( messages.hasNext() &&

              (message = messages.next()).getTime() > syncFrom ) {

        if (Defaults.FILTER_SPAM

            ? MessageFilter.spam(message)

            : MessageFilter.addressedToMe(message, username)) {

          this.lastSync = Math.max(this.lastSync, message.getTime());

          if (!inbox.contains(message.getId()) {

            inbox.add(message);

          }

        }

      }

    } finally {

      pop.disconnect();

    }

  }

}

Example of a test written after the implementation and which is too intimate with the implementation.

public class InboxSyncerTest extends TestCase {

  PopServer server;

  Certificate cert;

  static InboxSyncer syncer;

  public setUp(){

    if (syncer == null) {

      Config config = Config.get();

      User user = new User();

      user.setUsername(“test@example.com”);

      user.setPassword(“myTestPassword”);

      config.setUser(user);

      cert = DESCert.generateCert();

      File tempCert = File.creteTempFile(“temp”, “.cert”);

      FileOutputStream out = new FileOutputStream(tempCert);

      out.write(cert);

      out.close();

      config.set(“cert.path”, tempCert.toString());

      syncer = InboxSyncer.get();

    }

    // Reset the state of Global Objects

    Inbox inbox = Inbox.get(“test@example.com”);

    Iterator<Message> messages = inbox.getMessages();

    while(messages.hasNext()) {

      messages.remove();

    }

    Reflection.setPrivateField(syncer, “lastSync”, -1);

    POPServer server = new POPServer(cert);

    server.start();

    MessageQueue queue = server.getMessageQueueForUser(“test@example.com”);

    queue.clear();

  }

  public tearDown() {

    server.stop();

  }

  public testSync(){

    Defaults.FILTER_SPAM = true;

    MessageQueue queue = server.getMessageQueueForUser(“test@example.com”);

    Date time = new Date();

    Message message = new Message(“from”, “to”, “subject”, “message”, time);

    queue.addMessage(message);

    syncer.sync();

    Inbox inbox = Inbox.get(“test@example.com”);

    assertTrue(inbox.contains(message.getId());

    assertEquals(1, inbox.getMessages().size());

  }

}

Example of a test which was written before the implementation and using Specs rather than Tests

Class InboxSyncerSpec extends TestCase {

  Date longEgo = new Date(1);

  Date past = new Date(2);

  Date now = new Date(3);

  Date future = new Date(4);

  POPConnector pop = new MockPOPConnector();

  Inbox inbox = new Inbox();

  Filter filter = new NoopFilter();

  Message longEgoMsg = new Message(“from”, “to”, “subject”, “msg”, longEgo);

  Message pastMsg = new Message(“me”, “you”, “hello”, “world”, past);

  long noDefaultSync = -1;

  public @Test itShouldSyncMessagesFromPopSinceLastSync() {

    pop.addMessage(longEgoMsg);

    pop.addMessage(pastMsg);

    new InboxSyncer(pop, inbox, filter, longEgo, noDefaultSync).sync(now);

    assertThat(inbox.getMessages(), is(equalTo(pastMsg)));

    assertThat(pop.isClosed(), is(true));

  }

  public @Test itShouldCloseConnectionEvenWhenExceptionThrown() {

    Exception exception = new Exception();

    Filter filter = new ExceptionThrowingFilter(exception);

    InboxSyncer syncer = new InboxSyncer(pop, null, filter, null, noDefaultSync);

    try {

      syncer.sync(now);

      fail(“Exception expected!”);

    } catch (Exception e) {

      assertThat(e, is(exception));

      assertThat(pop.isClosed(), is(true));

    }

  }

  public @Test itShouldSyncMessagesOnlyWhenNotAlreadyInInbox() {

    pop.addMessage(pastMsg);

    inbox.add(pastMsg);

    new InboxSyncer(pop, inbox, filter, longEgo, noDefaultSync).sync(now);

    assertThat(inbox.getMessages(), is(equalTo(pastMsg)));

  }

  public @Test itShouldIgnoreMessagesOlderThenLastSync() {}

  public @Test itShouldIgnoreMessagesFailingFilter() {}

  public @Test itShouldDefaultToDefaultTimeWhenNeverSynced() {}

}

Refactored InboxSyncer for testability

public class InboxSyncer {

  private final POPConnector pop;

  private final Inbox inbox;

  private final Filter filter;

  private final Date lastSync;

  private final long defaultPastSync

  private InboxSyncer(POPConnector pop, Inbox inbox, Filter filter,

        Date lastSync, long defaultPastSync) {

    this.pop = pop;

    this.inbox = inbox;

    this.filter = filter;

    this.lastSync = lastSync;

    this.defaultPastSync = defaultPastSync;

  }

  public sync(Date now) {

    Date syncFrom = lastSync;

    if (syncFrom == null) {

      syncFrom = new Date(now.getTime() - defaultPastSync);

    }

    pop.connect();

    try {

      Iterator<Messages> messages = pop.messagesIterator();

      Message message;

      while ( messages.hasNext() &&

              (message = messages.next()).getTime() > syncFrom.getTime() ) {

        if (filter.apply(message)) {

          this.lastSync = new Date(Math.max(this.lastSync.getTime(), message.getTime()));

          if (!inbox.contains(message.getId()) {

            inbox.add(message);

          }

        }

      }

    } finally {

      pop.disconnect();

    }

  }

}