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()) }
Very good tips!
Regarding toString() and equals()/hashCode(), I’d argue that always implement them keeping in mind production code only. In tests, assert the results as you need. This may make asserting in tests a little longer, but I think it’s worth it – production code is free of any test related stuff and tests are more explicit.
The trick shown by Risto regarding toString() is a really good one.
Sven
Thanks!
I agree on equals and hashCode part. Regarding to toString I would in most cases use it for testing only. For extensive debugging and logging in production I would use something that uses reflection or implement other method specially for that purpose. For example Groovy has built-in support for that via Object.dump()
Don’t refuse to sharpen the saw and take a look for ScalaTest.org
Reblogged this on Ürgo Ringo's blob and commented:
I wrote this post some time ago. Decided to update it primarily due to inspiration got from Growing Object-Oriented Software, Guided by Tests book
Pingback: Ignite OÜ — Tips for writing readable tests