We're in a project that has quite a lot of code and while in general we have a good test coverage and things work out pretty well, I noticed that our tests grew more and more complex over time up to a point where the tests are more complex than the code they test.
As a bit of background we're using Spock for testing and make ample use of it's mocking and interaction testing features - which may be part of the problem. In general we opted for white-box testing, so our test code mostly knows about the internal workings of the classes under test (which may be another contributing factor to the problem).
Let me give you a simplified example of a less than stellar test on this method:
class SomeServiceImpl {
SomeDao someDao;
public void store(SomeInputForm form) {
// some checks of the form here.
SomeEntity entity = new SomeEntity();
// copy data from form to entity here.
someDao.store(someEntity);
}
}
And the test:
SomeServiceImpl underTest
SomeDao someDao
def setup() {
underTest = new SomeServiceImpl()
someDao = Mock(SomeDao)
underTest.someDao = someDao
}
def "storing something yields that stuff ends up in the database"() {
setup:
def form = new SomeInputForm(... with some data ...)
SomeEntity result = null
when:
underTest.store( form )
then:
1 * someDao.store( { result = it } _ as SomeEntity )
// additional checks to verify that the entity has the same data as the form
result.someProperty == form
}
The intention of this test is to verify that some data that comes in via a form is stored inside the database. We mock out the DAO so we don't need a real DB here. While the test certainly works it has several problems:
- It knows very well how the service works internally. As soon as you change the implementation of the service you have to change the implementation of the test, even if the end result would be the same.
- It's hard to detect the side effect of the store method (see the complicated code for extracting the generated entity).
- If you have multiple collaborators you will have to do a lot of mocking and stacking them all together.
- The test basically repeats the code of the service in the
thensection.
I'm currently experimenting with a more functional way of things where you have just functions getting input and producing output, e.g. like this:
class SomeServiceImpl {
void verify(SomeInputForm form) {
// verify and throw exception if a problem is there
}
SomeEntity createEntityFromForm(SomeInputForm form) {
// do the conversion here
}
void store(SomeEntity entity) {
dao.store(entity);
}
}
While this solves part of the testing problem, because now I can test the verify and createEntityFromForm methods without any collaborators and without knowing about the actual implementation, there still has to be a place where these three methods are called in the correct order:
service.verify(form);
Entity en = service.createEntityFromForm(form);
service.store(form);
Which effectively just moves the problem somewhere else (and introduces a lot of margin for error as you need to call methods in the correct order now).
So I wonder whether there is a better way of organising the code differently to make the tests less brittle and less whitebox while still detecting the side effects (or even a way where you wouldn't need to detect the side effect but still know stuff is properly saved to the database).
Which effectively just moves the problem somewhere else-- Sure, but that "somewhere else" is very easy to reason about.