TDD: Where Did I Get It Wrong?

If you are developing using Test-Driven Development (TDD), there is a chance that you are doing it the wrong way. The symptoms of this are very clear. If you have a huge test suite that breaks every time you insert a new feature or make a change to the code, and you end up having to rewrite tests to make it pass again, you are there. We've all been there, and sometimes we still are.

I was blown away by this presentation from Ian Cooper on DevTernity asking where did all go wrong with TDD. The catch was that at some point, we started to focus our tests on methods instead of focusing on features, on scenarios. To return the TDD to its track, Behavior-Driven Devlopment (BDD) was created. Actually, according to Ian, BDD is how TDD was supposed to be. All of these GIVEN, WHEN, THEN help us think about scenarios and stop thinking about methods.



So let's try an example, developing using the correct TDD way. Let's suppose we have a part of our system that is responsible for keeping historical records of products and services sold. We want to save a record and retrieve a list of all the historical records. Given the "HistoricalRecord" below, let's build our "HystoricalRecordController."

public abstract class HistoricalRecord {
//bunch of properties here
}

public class ProductHistoricalRecord extends HistoricalRecord {
//bunch of properties here
}

public class ServiceHistoricalRecord extends HistoricalRecord{
//bunch of properties here
}

The first scenario must be the most simple. Given that no records were added, when we retrieve a list of records, the list must be empty. This test may appear as a useless requirement. However, how many times did you have to check for null because someone thought it was a good idea to retrieve to return null when there are no records to return?

There are many practices for TDD. One of them being the RED-GREEN: first, you write the test, so it fails. Then, you write the most straightforward code to make it pass.

@Test
public void testRetrieveEmptyList() throws Exception {
//GIVEN
final HistoricalRecordController recordController = new HistoricalRecordController();
//WHEN
final List<HistoricalRecord> historicalRecords = recordController.listAll();
//THEN
assertEquals(0, historicalRecords.size());
}

public class HistoricalRecordController {

public List<HistoricalRecord> listAll() {
return Collections.EMPTY_LIST;
}
}

The next scenario is: Given a historical record, when we save this record, the list retrieved must contain only one record, and they must be equal. So let's do it.

@Test
public void testRetrieveOneRecord() throws Exception {
//GIVEN
final ProductHistoricalRecord historicalRecord = new ProductHistoricalRecord();
final HistoricalRecordController recordController = new HistoricalRecordController();
recordController.saveRecord(historicalRecord);
//WHEN 
final List<HistoricalRecord> historicalRecords = recordController.listAll();
//THEN
assertEquals(1, historicalRecords.size());
assertEquals(historicalRecord, historicalRecords.get(0));
}

Now we do the most straight forward implementation which satisfies both tests:

public class HistoricalRecordController {

  List<HistoricalRecord> historicalRecords = new ArrayList<>();

public List<HistoricalRecord> listAll() {
return Collections.unmodifiableList(historicalRecords);
}

public void saveRecord(final ProductHistoricalRecord historicalRecord) {
historicalRecords.add(historicalRecord);
}
}

However, this implementation persists the records in memory. We should implement a HistoricalRecordRepository  for our historical records. Our HistoricalRecordController  should save and retrieve the list of HistoricalRecordsfrom this repository.

public interface HistoricalRecordRepository {
List<HistoricalRecord> listAll();
void create(ProductHistoricalRecord historicalRecord);
}

public class HistoricalRecordController {

private final HistoricalRecordRepository repository;

public HistoricalRecordController(final HistoricalRecordRepository repository) {
this.repository = repository;
}

public List<HistoricalRecord> listAll() {
return repository.listAll();
}

public void saveRecord(final ProductHistoricalRecord historicalRecord) {
repository.create(historicalRecord);
}
}

Now everything in red again. Before watching Ian's presentation, I would be tempted to rewrite the tests to see them pass. So thinking about my implementation, I would mock my HistoricalRecordRepository, inject it into my HistoricalRecordController  and check if the method create() is called exactly one time:

@Test
public void testSaveRecord() throws Exception {
final HistoricalRecordRepository repository = Mockito.mock(HistoricalRecordRepository.class);
final ProductHistoricalRecord historicalRecord = new ProductHistoricalRecord();
final HistoricalRecordController recordController = new HistoricalRecordController(repository);
recordController.saveRecord(historicalRecord);
Mockito.verify(repository, times(1)).create(historicalRecord);
}

For the time being, this looks okay. However, we know what happens in the future. More features come along, we change our implementation, and suddenly all brakes lose. All the tests we made using our mocks broke. Also, some tests were coupled with the implementation of logic from other classes. Those broke too. We will have to redo everything because we tested our implementation, rather than testing the scenarios, and now the implementation changed. Raise OUR hand who faced this sometimes?

However, if we build our tests against the scenarios involved, just minor changes have to be made to satisfy the business requirements.

We now build a stub for the HistoricalRecordRepository to satisfy the dependency and rerun our tests.

public class HistoricalRecordRepositoryStub implements HistoricalRecordRepository {

private final List<HistoricalRecord> records = new ArrayList<>();

@Override
public List<HistoricalRecord> listAll() {
return Collections.unmodifiableList(records);
}

@Override
public void create(final ProductHistoricalRecord historicalRecord) {
records.add(historicalRecord);
}
}

public class HistoricalRecordControllerTest {

@Test
public void testRetrieveEmptyList() throws Exception {
//GIVEN
final HistoricalRecordController recordController = new HistoricalRecordController(new HistoricalRecordRepositoryStub());
//WHEN
final List<HistoricalRecord> historicalRecords = recordController.listAll();
//THEN
assertEquals(0, historicalRecords.size());
}

@Test
public void testRetrieveOneRecord() throws Exception {
//GIVEN
final ProductHistoricalRecord historicalRecord = new ProductHistoricalRecord();
final HistoricalRecordController recordController = new HistoricalRecordController(new HistoricalRecordRepositoryStub());
recordController.saveRecord(historicalRecord);
//WHEN 
final List<HistoricalRecord> historicalRecords = recordController.listAll();
//THEN
assertEquals(1, historicalRecords.size());
assertEquals(historicalRecord, historicalRecords.get(0));
}
}

What we can take from this is that as long as the business requirements we tested remains the truth, our tests won't change. Besides, we can clearly understand the scenario involved in a particular test and what it is actually testing. Just compare the testRetrieveOneRecord() against testSaveRecord(). After months of a growing code base, for which one would you recognize the requirement being tested? Which one would you accept as a needed test? Which one could be potentially be broken?