Tuesday, June 22, 2010

unit test and new Date()

A lot of code which use new Date() or System.currentTimeMillis() is quite impossible to unit test.

A too-current-in-every-day-life example method :

public class SomeClass {
    public String currentDateAsString() {
        return new SimpleDateFormat().format(new Date(), "yyyyMMdd");
    }
}

The corresponding unit test will not work for so long :

@Test
public void testCurrentDateAsString() {
    Assert.assertEquals("20100622", new SomeClass().currentDateAsString());
}

It will fail tomorrow ;-)

If you own the code, you should use a Date provider. Using library such as google collection (now guava), you can simply use a Supplier.

public class SomeClass {
    public SomeClass(String pattern, Supplier dateSupplier) {
        this.pattern = pattern;
        this.dateSupplier = dateSupplier;
    }

    public String currentDateAsString() {
        new SimpleDateFormat().format(this.dateSupplier.get(), this.pattern);
    }

    private final String pattern;
    private final Supplier dateSupplier;

}

Now the unit test will be much nicer  and more it follows the Law of Demeter :

@Test
public void testCurrentDateAsString() {
    Supplier dateSupplier = Suppliers.ofInstance(newDate(22,6,2010));
    Assert.assertEquals("20100622", new SomeClass("yyyyMMdd", dateSupplier).currentDateAsString());
}

*newDate may be a static date factory method you wrote, it is here for readability.

If the code come from scary legacy code, you should use jmockit or powermock (see Can I mock static methods).

4 comments:

  1. Hi,

    Just tried that and it's not so bad, but you still have to provide a production implementation of the Date Supplier. Obviously, that cannot just be Suppliers.ofInstance(getCurrentDate()), since the date changes all the time...

    So somehow a class is necessary for production, such as
    class DateSupplier implements Supplier() {
      public Date get() {
        return Calendar.getInstance().getTime();
      }
    }

    The alternative that I had been using so far was to provide my own DateSupplier, with no relation to Guava's Supplier. I would mock its behavior in the test class (easy to do with Mockito, but other mock packages do not necessarily support class mocking).

    @Test
    public void testCurrentDateAsString() {
      DateSupplier dateSupplier = mock(DateSupplier.class);   when(dateSupplier.get()).thenReturn(newDate(22,6,2010));
      Assert.assertEquals("20100622", new SomeClass("yyyyMMdd", dateSupplier).currentDateAsString());
    }

    I don't quite see how things are much easier by using Guava's Supplier here.
    It even seems to me that an explicit DateSupplier class is desirable, to make the requirements clearer.

    What do you think?

    ReplyDelete
    Replies
    1. There is no big deal in using Guava Supplier. It is just that this interface is straightforward and can be used by other guava utils classes such as Multimaps. Your DateSupplier may implements Supplier if required. The only important thing is to decorrelate the creation of date and their use.

      Delete
  2. The thing I don't like about the above is having to pass in the supplier to every class that needs "new Date". The solution we came up with was to implement DateProvider as a Singleton with a static getCurrentDate method. The DateProvider has a protected method that allows a caller to override the singleton instance. Then in a test library we have a DateProviderTestImpl that extends DateProvider. When a unit test invoked DateProviderTestImpl it overrides the singleton instance in DateProvider to be a DateProviderTestImpl whose implementation allows for controlling what value is returned from getCurrentDate. We also have a DateController Rule that handles resetting the DateProviderTestImpl to return new Date at the end of each test.

    ReplyDelete
    Replies
    1. As stated by Misko Hevery in his Testable Code Manifesto (http://misko.hevery.com/code-reviewers-guide/), static is global state and so, not very pretty for testing purpose (http://misko.hevery.com/2008/12/15/static-methods-are-death-to-testability/). Of course, it is not always handy to have to pass dependencies to your class by constructor but if there is too much of them it is because that class is doing to much. And when you use some factory tools such as Spring IOC or Guice, it does not matter anymore. With static, you always need to provide some backdoor to reset the state of the dependencies. As always, it is neither black or white, I sometime use static but as it is said, I known why static is flawed.

      Delete