Unit-testing – Does it make sense to write tests for legacy code when there is no time for a complete refactoring

legacy coderefactoringunit testing

I usually try to follow the advice of the book Working Effectively with Legacy Code. I break dependencies, move parts of the code to @VisibleForTesting public static methods and to new classes to make the code (or at least some part of it) testable. And I write tests to make sure that I don't break anything when I'm modifying or adding new functions.

A colleague says that I shouldn't do this. His reasoning:

  • The original code might not work properly in the first place. And writing tests for it makes future fixes and modifications harder since devs have to understand and modify the tests too.
  • If it's GUI code with some logic (~12 lines, 2-3 if/else block, for example), a test isn't worth the trouble since the code is too trivial to begin with.
  • Similar bad patterns could exist in other parts of the codebase, too (which I haven't seen yet, I'm rather new); it will be easier to clean them all up in one big refactoring. Extracting out logic could undermine this future possibility.

Should I avoid extracting out testable parts and writing tests if we don't have time for complete refactoring? Is there any disadvantage to this that I should consider?

Best Answer

Here's my personal unscientific impression: all three reasons sound like widespread but false cognitive illusions.

  1. Sure, the existing code might be wrong. It might also be right. Since the application as a whole seems to have value to you (otherwise you'd simply discard it), in the absence of more specific information you should assume that it is predominantly right. "Writing tests makes things harder because there's more code involved overall" is a simplistic, and very wrong, attitude.
  2. By all means expend your refactoring, testing and improvement efforts in the places where they add the most value with the least effort. Value-formatting GUI subroutines are often not the first priority. But not testing something because "it's simple" is also a very wrong attitude. Virtually all severe errors are committed because people thought they understood something better than they actually did.
  3. "We will do it all in one big swoop in the future" is a nice thought. Usually the big swoop stays firmly in the future, while in the present nothing happens. Me, I'm firmly of the "slow and steady wins the race" conviction.
Related Topic