2

Assuming OldModule.oldFunc, if we want to move oldFunc to NewModule and, for backward compatibility, keep oldFunc there merely calling NewModule.newFunc by passing the exact same arguments and doing no tampering whatsoever -- do we need to still write unit tests for oldFunc? Or will having unit tests for newFunc suffice?

# Are unit tests still needed for oldFunc?
OldModule.oldFunc(args)
{
  return NewModule.newFunc(args)
}
Adelin
  • 281

3 Answers3

6

Luckily for you, since the function is not doing a lot, it would be easy to test it as well. A few seconds you'll spend adding a unit test is worth a potential loss of a few minutes if someone inadvertently modifies the function, or if the function breaks, affected by a change somewhere else.

How could such a simple function possibly break?

An example of a regression

Let's take an example of a dynamically-typed language. You're in the middle of a refactoring for the last two weeks; some calls were already migrated to newFunc, and there are only a very few which still rely on oldFunc. New code is well tested, but oldFunc has no tests at all.

Your colleague starts checking a bug report. It appears that it is related to a narrow piece of code in which, among others, newFunc is called. In order to fix the bug, your colleague decides to add an argument to newFunc: it's now newFunc(args, isSomething) and can be called by specifying either True or False for isSomething.

Your colleague runs unit tests, and, one by one, corrects them by adding the missing parameter to the callers. The test suite is now all green, so he pushes the changes to production.

A few minutes later, the phone starts ringing...

Since you forgot to test oldFunc, the change wasn't caught up by the test suite. Your colleague introduced a regression because the code was untested. By simply looking at the code, your colleague had no clue that he needed to update oldFunc and all the callers of oldFunc.

What to test?

As you may guess, there is no need to write all unit tests of newFunc for oldFunc: tests of oldFunc should test only what the actual function is doing, not what some other function which is called is performing.

This means that you should limit yourself to possibly one test.

Further changes

If the old function is kept only temporarily, you may—if the language has a support for it—decorate the function as obsolete to make the compiler produce a set of warnings for each location in the code which still uses the old function. The next step might be to replace the call to the new function by an exception, to catch a few locations where the function was called, without being caught by the compiler; the test will therefore be modified accordingly to check that the exception was thrown.

4

If you have a function, then you should have tests for it. This recommendation varies in strength according to the probability of defects in a function. For instance, auto-generated getters and setters with only one line are right at the bottom of the priority list, while business-critical, long, complicated functions need the hell tested out of them.

What does this mean for your legacy delegator? It's trivial and therefore unlikely to have defects; however, it does have a job to do, and you should test that it does this job. Depending on the nature of your test suite it might be easier to just adjust a copy the test of the test for the old function, or you might construct a mock and verify that oldFunc does, in fact, call newFunc with the same arguments.

But I would not omit testing just because the current incarnation of a point of functionality is trivial. That is an implementation detail which might easily change - after all, it did change drastically just now! And when it does change, it's unlikely that someone notices that the informal decision "we don't need to test this because it's trivial" will be remembered and revised. So it's better to write a simple test and keep your coverage.

Kilian Foth
  • 110,899
3

Are unit tests needed for a function that only calls another function?

Yes.

Assuming OldModule.oldFunc, if we want to move oldFunc to NewModule and, for backward compatibility, keep oldFunc there merely calling NewModule.newFunc by passing the exact same arguments and doing no tampering whatsoever -- do we need to still write unit tests for oldFunc?

If oldFunc is still part of the supported public API, then there should be tests to ensure that its behavior is correct. The fact that it delegates all of its work is an implementation detail that is subject to refactoring.

The usual sequence for removing tests is:

  • Deprecate the old API, and schedule its removal
  • Wait until the grace period ends
  • Remove the tests
  • Remove the API

If the API hasn't been published, you have a lot more freedom in tearing things down.

VoiceOfUnreason
  • 34,589
  • 2
  • 44
  • 83