Unit Testing – How to Handle Mocking When Function Arguments Change

refactoringunit testing

I'm learning how to write unit tests on a project I'm doing with my friends. One thing we tried was to mock the return values of function calls if the functions are declared in a different "layer". For example, if I have a function in the routes/ directory that calls another function in the services/ directory, my unit tests in the routes directory will mock the return values of function calls where the functions are declared in the services directory. I thought this would speed up the execution of the tests and make them easier to maintain. However, we noticed that this approach makes the tests less reliable at capturing changes caused by refactoring.

I'll try to give a demonstration of my problem with pseudocode. Here's pseudocode for two functions in two different layers where one calls another:

// routes/calendar.js

function expiredDriversLicense(httpRequest, response) {
   const result = services.dateDiff(httpRequest.get.dateA, httpRequest.get.dateB);
   const isExpired = result > 3600*24*365;
   response.status(200).send({isExpired:isExpired});
}
// services/calendar.js

function timeDiff(dateA, dateB) {
  return dateA.getUnixTime() - dateB.getUnixTime();
}

My unit tests for routes/calendar.js mocks results from the services.dateDiff() function, so it looks like this:

function testExpiredDriversLicense() {
   expiredDriversLicense.mock.returnResult(100);
   const httpRequest = {get:{dateA:"some date", dateB:"some date"}}
   expiredDriversLicense(httpRequest);
   
   assert response.send.called[0].arguments[0] === {isExpired:false};
}

The above worked fine initially.

But later, my friends refactored services/calendar.js:timeDiff() by changing the order of incoming arguments. For example, he changed the function to this:

// services/calendar.js

function timeDiff(dateB, dateA) {
  return dateA.getUnixTime() - dateB.getUnixTime();
}

Notice the dateB and dateA changed places. There were a lot of unit tests for services/calendar.js and my friend updated all of them while refactoring the services layer. However, my friend did not refactor routes/calendar.js:expiredDriversLicense() because he didn't realize it depends on services/calendar.js:timeDiff(). And because my unit tests for routes/calendar.js:expiredDriversLicense() mocks all calls to and from the services layer, none of router unit tests failed.

Is scenario above an example of bad implementation of unit testing? Or inappropriate use of mocks? Did I completely misunderstood the way unit tests are supposed to be implemented?

Best Answer

Is scenario above an example of bad implementation of unit testing? Or inappropriate use of mocks? Did I completely misunderstood the way unit tests are supposed to be implemented?

Not necessarily any of those - it may be that you were missing other checks that could have detected the problem.

Martin Fowler 2002 introduces the concept of a "published interface"; very roughly: the important indicator that a method is published is that you cannot easily find and change all of the client code.

By this definition, what your friend did is make a backwards incompatible change to a published interface. That's a really bad idea.

(Not to blame your friend: it may not have been obvious that this interface was published - you didn't necessarily publish the interface on purpose, but rather just got to a place where it was hard to find all of the callers).

So, the question I see is "are unit tests the right tool to use to catch this mistake?". And that question in turn is going to depend on which definition of "unit test" you are using....

Rainsberger 2017 offers the suggestion that test doubles (mocks, stubs, etc) should be paired with tests that evaluate the assumptions of the double. This isn't necessarily protection (after all, your friend refactored his tests successfully), but discovering the test in another place might have helped discover the mock, and its implications.

You also might have caught this problem with tests at a larger grain - an "integration" test or an "end-to-end" test.

Related Topic