27 July 2013

404 error with spring mvc testing

We have a standard rest application built with spring mvc (v3.2.3). Let's add another controller (MyController) with one method that does some computations and returns an empty response with 200 OK. Test first:
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;

import org.junit.*;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;

public class MyControllerTest {

 private MockMvc mockMvc;

 @Test
 public void test() throws Exception {
  mockMvc.perform(get("/my/test"))

   .andExpect(status().isOk())
   .andExpect(content().string(""));
 }

 @Before
 public void setUp() throws Exception {
  mockMvc = MockMvcBuilders.standaloneSetup(new MyController()).build();
 }
}
Nothing new, everything just as in spring's tutorial. So now let's write the code:
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.RequestMapping;

@Controller
public class MyController {

 @RequestMapping("/my/test")
 public void test() {
  System.out.println("test method executed"); //yes, yes, I know
 }
}
Test is green, that was easy. Let's see our code in action. Start jetty and check the address /my/test. Method is executed and we get... 404 not found. Wtf?!

Probably the quickest way to find the problem is to compare the method with other working controllers. Of course, I forgot about @ResponseBody. After adding it, jetty displays the page correctly and test still passes. But the purpose of writing tests is to have protection against such mistakes. So why the test was green?

For void methods without @ResponseBody spring forwards request processing to DispatcherServlet which, in this case, fails trying to resolve a view for the specified url. But for some reason mockMvc reports empty response and status 200. I reported it as a bug but it got status 'Works as Designed'. So how can we eliminate the false positive? We need to explicitly check if no forwarding is done. And there is an existing ResultMatcher for this:
forwardedUrl(null)
It can be added with another andExpect inside each test. But turning it on globally will save you from such mistakes in future:
@Before
public void setUp() throws Exception {
 mockMvc = MockMvcBuilders.standaloneSetup(new MyController())
                             .alwaysExpect(forwardedUrl(null))
                             .build();
}
After adding this matcher, the test without @ResponseBody fails:
java.lang.AssertionError: Forwarded URL expected:<null> but was:<my/test>

8 July 2013

Testing time-dependent code

Often people do something like that
public List<Notification> findNotificationsToSend() {

    List<Notification> notifications = repository.findDailyNotifications();
    int today = Calendar.getInstance().get(DAY_OF_WEEK);
    if (today == SUNDAY) 
        notifications.addAll(repository.findAllWeeklyNotifications());
    return notifications;
}
and then you can be sure they don't use TDD. Because there is absolutely no way you can create such a bad code when you have tests. But what can we do about the calendar? Same as always. Have you noticed that when there is a call to a database or other external system, people immediately say: 'extract and mock'? But when there is a call to jvm's infrastructure they have no idea what to do. And the answer is simple: 'extract and mock'. Does it mean people just repeat previously seen schemes without thinking?
We can start refactoring with:
public class TimeProvider {

    public int dayOfWeek() {
        return Calendar.getInstance().get(DAY_OF_WEEK);
    }
}
That's a good start. Now it's easy to test the findNotificationsToSend but TimeProvider can still contain some complicated time calculations which are not testable. And it will grow with calendar-dependent methods. How to clean it up?
  • Switch to joda time. It has much better api that protects TimeProvider from uncontrolled growing.
  • TimeProvider should contain only often used, calendar-like, parameterless methods dependent on current time. And nothing else. 'isSunday' and 'beginningOfQuarter' are fine but 'shouldIncludeWeeklyNotification' is not.
  • Completely separate jvm's infrastructure access from time calculations. In this case I usually choose inheritance over composition because TimeProvider won't ever grow in any additional dependencies. After all, even business guys don't change the definition of Sunday.
The following code usually works for me.
abstract class TimeProvider {

    protected abstract long currentMillis();

    public final DateTime now() {
        return new DateTime(currentMillis());
    }

    public final boolean isSunday() {...} //if often used

    // other common business methods. all final.
}

public final class RealTimeProvider extends TimeProvider {

    protected long currentMillis() {
        return System.currentTimeMillis();
    }
}

public class TestTimeProvider extends TimeProvider {

    private long currentMillis; 

    public TestTimeProvider() {
        this("2013-05-17"); // preset time; handy for tests
    }

    public TestTimeProvider(String currentTime) {
        setTime(currentTime);
    }

    public void setTime(String currentTime) {
        currentMillis = parseTime(currentTime);
    }

    protected long currentMillis() {
        return currentMillis;
    }

    private static long parseTime(String time) {...}
}
Of course, we use TestTimeProvider in unit and spring-context tests. Often it's more handy than mocks. If needed, add similar support for timezone.
Now we can test findNotificationsToSend, control time during integration tests and test isSunday method:
@RunWith(ZohhakRunner.class)
public class TimeProviderTest {

    TestTimeProvider timeProvider = new TestTimeProvider();

    @TestWith({
                 "2013-04-14,  true",
                 "2013-04-15,  false"
    })
    public void should_detect_sunday(String date, boolean shouldBeSunday) {
        timeProvider.setTime(date);

        boolean isSunday = timeProvider.isSunday();

        assertThat(isSunday).isEqualTo(shouldBeSunday);
    }
}
One place where time provider alone is not enough is integration testing, when we start the whole server to imitate production environment and connect to it over http. But that's a story for another post.