Tips for writing readable tests

Following is list of best practices I try to follow when writing tests.

Test method name should describe the tested behavior

//not so good
void searchPosts(){...}

//better
void searchIncludesAllUsersIfNoUserIdsProvided(){...}

Test method name should be as specific as possible

//not so good
void invalidLogin(){...}

//better
void loginFailsWhenPasswordDoesNotMatch(){...}

Test should be short (avoid tests which assert different scenarios)

//not so good
void findHolidaysByUser() {
  ....
  holidays = holidayRepository.findByUser(userId, today, tomorrow)
  assertEquals(...)

  holidays = holidayRepository.findByUser(userId, tomorrow, nextYear)
  assertEquals(...)
}

//better
void findHolidaysWhereStartingDateIsBetweenProvidedDates() {
  holidays = holidayRepository.findByUser(userId, today, tomorrow)
  assertEquals(...)
}

void findHolidaysWhereEndingDateIsBetweenProvidedDates() {
  holidays = holidayRepository.findByUser(userId, tomorrow, nextYear);
  assertEquals(...);
}

Note that these test methods actually tell what is the desired behavior while single test method gives no information about the tested behavior.

It’s OK to have long test names

Although sometimes when test names get too long it is good indicator that test class should be split into multiple smaller test classes (and possibly class under test should be split too :))

Avoid ANY logic inside tests

//bad sample
void searchOrdersResultsByStartingDate() {
  results = repository.findHolidays(...)

  for (Holiday holiday : holidays) {
    if (last != null) {
      assertTrue(last.getStartingDate().after(activity.getStartingDate()))
    }
    last = activity;
  }
}  

//better sample
void searchOrdersResultsByStartingDate() {
  save(holiday1, holiday2, holiday3)

  results = repository.findHolidays(...)

  assertEquals(holiday1, results.get(0))
  assertEquals(holiday3, results.get(1))
  assertEquals(holiday2, results.get(2))
}

In this sample we actually know the correct order of holidays since we inserted them in the same test.

Never assume database state

//bad sample
testProject = projectRepository.findById(6);

Try to minimize database traffic when testing DAOs/Repositories

//bad sample
void setUpTestData() {
  testUser = userRepository.findAllUsers().get(0);
}

//better
void setUpTestData() {
  User testUser = new User();
  save(testUser);
}

Clean database before running your test (in setUp)

Although well written tests should clean up system state after their are executed it is still possible that some legacy tests don’t do this or someone manually inserted some data.

Avoid mocking simple objects

Mocks created by some special mocking library like EasyMock or Mockito make test code more complicated than setting up simple objects. I have seen that people who are new to using mocking libraries start mocking every little dependency they have. In general I find that such dynamic mocks always make the test more complex.

Avoid tests with extensive mocking

If your test looks something like below then you should probably either rewrite it as integration test without mocks or simply delete it. Sooner or later you will do something that causes this test to fail and you will spend lot of time understanding what exactly does it means that this test fails now.

...
EasyMock.expect(projectService.getCurrentUserProjects()).andReturn(projects);
EasyMock.expect(projectModelConverter.convertModel(p1)).andReturn(pdto1);
EasyMock.expect(projectModelConverter.convertModel(p2)).andReturn(pdto2);
EasyMock.expect(session.getUser()).andStubReturn(user);
EasyMock.expect(userProjectGroupService.getByUser(user.getId())).andReturn(projectGroups);
EasyMock.expect(projectService.getJiraInstances()).andReturn(jiras);
EasyMock.expect(userService.getAllUsers(true)).andReturn(userList);
EasyMock.expect(listItemDtoConverter.convert(u1)).andReturn(userItem);

...
    
InitialDataRpcFacade facade = ...
InitialDataDto data = facade.loadInitialData();
  
//assertions

EasyMock.verify(session, userService, projectService);
EasyMock.verify(userProjectGroupService, listItemDtoConverter, projectModelConverter);

Always mock DAOs/Repositories when testing higher layers e.g services or facades

Unless you are doing higher level testing of course.

Avoid using dependency injection container (e.g Spring) for unit tests

Spring should only be used if we for example want to test Spring XML bean definitions.
For testing DAOs we only need minimal set of beans that can be set up manually.

When asserting if collection returned contains the right items use the toString

This trick I learned from my colleague Risto.

List result = service.doSomething();
assertEquals("[product1, product2]", result.toString());

Implement toString() so that it will be easy to use in tests (also see previous tip)

//bad sample}
String toString() {
  return new ToStringBuilder(this).append(field1).append(field2).append(id).toString();
}

//better
String toString() {
  return id;
}

Now in some cases you may want to use toString also for debugging info in logs. I suggest to use something else for this purpose. Probably you don’t want this to be turned on all the time in production anyway so one option is to use some reflection based solution e.g BeanUtils.

Don’t implement extensive equals for extensive testing

//not so good
boolean equals(Object other) {
  ...
  User user = (User) other
  return new EqualsBuilder()
        .append(getName(), other.getName())
        .append(getPersonalCode(), other.getPersonalCode())
        .append(getDateOfBirth(), other.getDateOfBirth())
        ...
        .isEquals()
}

Sometimes this is done to test if persistence works correctly. However, I find that this kind of changes in production code are not justified even if it helps testing certain cases. Better alternative is to implement custom assert that checks all fields and leave equals to check only the business identity.

//better
boolean equals(Object other) {
  ...
  User user = (User) other
  return new EqualsBuilder()
        .append(getPersonalCode(), other.getPersonalCode()).isEquals()
}


void assertEquals(User expected, User actual) {
  assertEquals(expected.getName(), actual.getName())
  assertEquals(expected.getPersonalCode(), actual.getPersonalCode())
  assertEquals(expected.getDateOfBirth(), actual.getDateOfBirth())
}

Advertisements