Unit-testing – Where is the line between unit testing application logic and distrusting language constructs

tddtestingunit testing

Consider a function like this:

function savePeople(dataStore, people) {
    people.forEach(person => dataStore.savePerson(person));
}

It might be used like this:

myDataStore = new Store('some connection string', 'password');
myPeople = ['Joe', 'Maggie', 'John'];
savePeople(myDataStore, myPeople);

Let us assume that Store has its own unit tests, or is vendor-provided. In any case, we trust Store. And let us further assume that error handling — eg, of database disconnection errors — is not the responsibility of savePeople. Indeed, let us assume that the store itself is magical database that cannot possibly error in any way. Given these assumptions, the question is:

Should savePeople() be unit tested, or would such tests amount to testing the built-in forEach language construct?

We could, of course, pass in a mock dataStore and assert that dataStore.savePerson() is called once for each person. You could certainly make the argument that such a test provides security against implementation changes: eg, if we decided to replace forEach with a traditional for loop, or some other method of iteration. So the test is not entirely trivial. And yet it seems awfully close…


Here's another example that may be more fruitful. Consider a function that does nothing but coordinate other objects or functions. For example:

function bakeCookies(dough, pan, oven) {
    panWithRawCookies = pan.add(dough);
    oven.addPan(panWithRawCookies);
    oven.bakeCookies();
    oven.removePan();
}

How should a function like this be unit tested, assuming you think it should? It's hard for me to imagine any kind of unit test that doesn't simply mock dough, pan, and oven, and then assert that methods are called on them. But such a test is doing nothing more than duplicating the exact implementation of the function.

Does this inability to test the function in a meaningful black box way indicate a design flaw with the function itself? If so, how could it be improved?


To give even more clarity to the question motivating the bakeCookies example, I'll add a more realistic scenario, which is one I've encountered when attempting to add tests to and refactor legacy code.

When a user creates a new account, a number of things need to happen: 1) a new user record needs to be created in the database 2) a welcome email needs to be sent 3) the user's IP address needs to be recorded for fraud purposes.

So we want to create a method that ties together all the "new user" steps:

function createNewUser(validatedUserData, emailService, dataStore) {
  userId = dataStore.insertUserRecord(validateduserData);
  emailService.sendWelcomeEmail(validatedUserData);
  dataStore.recordIpAddress(userId, validatedUserData.ip);
}

Note that if any of these methods throws an error, we want the error to bubble up to the calling code, so that it can handle the error as it sees fit. If it's being called by the API code, it may translate the error into an appropriate http response code. If it's being called by a web interface, it may translate the error into an appropriate message to be displayed to the user, and so on. The point is this function doesn't know how to handle the errors that may be thrown.

The essence of my confusion is that to unit test such a function it seems necessary to repeat the exact implementation in the test itself (by specifying that methods are called on mocks in a certain order) and that seems wrong.

Best Answer

Should savePeople() be unit tested? Yes. You aren't testing that dataStore.savePerson works, or that the db connection works, or even that the foreach works. You are testing that savePeople fulfills the promise it makes through its contract.

Imagine this scenario: someone does a big refactor of the code base, and accidentally removes the forEach part of the implementation so that it always only saves the first item. Wouldn't you want a unit test to catch that?

Related Topic