How do I write a unit test to verify that a function sorts its result?

Question

I have a data source from which I can request a list of people that live in a (any) country, and a method which retrieves the people from that data source and sorts them by their name alphabetically. How should I write my unit test to make sure that the sorting part of my method works properly?

This is what my SUT looks like:

class PeopleStuff {

    public IData data;

    public List<Person> getSortedPeopleForCountry(String countryName) {
        List<Person> people = data.getPeopleForCountry(countryName);

        Comparator nameComparator = new PersonNameComparator();
        Collections.sort(people, nameComparator);

        return people;
    }

}

And this is what my unit test looks like:

@Test public void testGetPeopleSortsByPeopleName() {
    String COUNTRY = "Whatistan";

    // set up test (the 3 lines below are actually in a @Before setup method)
    PeopleStuff peopleStuff = new PeopleStuff();
    IData mockData = createNiceMock(IData.class);
    peopleStuff.data = mockData;

    // set up data
    List<PersonName> mockPeopleList = new ArrayList<PersonName>();
    mockPeopleList.add(new Person(COUNTRY, "A"));
    mockPeopleList.add(new Person(COUNTRY, "D"));
    mockPeopleList.add(new Person(COUNTRY, "B"));
    mockPeopleList.add(new Person(COUNTRY, "C"));

    when(mockData.getPeopleForCountry(COUNTRY)).thenReturn(mockPeopleList);

    // exercise
    List<String> result = peopleStuff.getSortedPeopleForCountry(COUNTRY);

    // assert
    assertEquals("A", result.get(0).name);
    assertEquals("B", result.get(1).name);
    assertEquals("C", result.get(2).name);
    assertEquals("D", result.get(3).name);
}

What I need to know is if the way I am stubbing the data, running the test and making the assertions is correct, or if there are better ways of doing this.

My application has a lot of methods to test and a lot of custom sorting algorithms; I implemented all tests to use some 4 values that I stub like that, in a "random" order which I choose when I write the test.

---

Should I just test if the comparators are called? That doesn't seem right to me, because I don't know if they're called for the right data or at the right time in the algorithm that's inside getSortedPeopleForCountry(). I want to detect situations like this:

public List<Person> getSortedPeopleForCountry(String countryName) {
    List<Person> people = data.getPeopleForCountry(countryName);

    Comparator nameComparator = new PersonNameComparator();
    List<Person> sortedPeople = new ArrayList<Person>(people)
    Collections.sort(sortedPeople, nameComparator);

    return people; // oops!
}

Should I leave it like this and add mock comparators which use the real comparators but also verify that they're being called?

Am I doing it right?

Answer 1

I think your current test is very good - the tests are realistic, exercising all of the code, and you are mocking out the data source & using dependency injection to supply a mock data source. There is a lot of best practice going on in this test.

On the issue of whether you should look to mock the comparators (and therefore make the test on testGetPeopleSortsByPeopleName a pure unit test), you will definitely get two different opinions here:

• A purist would argue that your test is technically an integration test, and that to have proper unit tests you need to adjust your test to use a mock comparator, and then test the comparator separately.
• A pragmatist would argue that your test is already high quality, and that it doesn't matter that it isn't a unit test in the strictest sense. Furthermore, to split this into two separate unit tests may make the test less readable - which I imagine would be the case with the test above if you were to involve mock comparators.

My personal opinion is that you should leave it as it is, the fact that you have a high quality, readable test that exercises all the code and effectively asserts your requirements is far more important than worrying about having strictly pure unit tests.

The only way in which the test looks in need of improvement is the length of the test method - I think a little method extraction could help improve readability and make the test method more expressive. I would aim for something like this:

@Test public void testGetPeopleSortsByPeopleName() {

    peopleStuff.data = buildMockDataSource(COUNTRY, "A", "D", "B", "C")

    List<String> result = peopleStuff.getSortedPeopleForCountry(COUNTRY);

    assertPersonList(result, "A", "B", "C", "D")
}

private IData buildMockDataSource(String country, String ... names) {
    ...
}

private void assertPersonList(List<Person> people, String ... names) {
    ...
}