I'm tasked with getting a legacy application under unit test. First some background about the application: It's a 600k LOC Java RCP code base with these major problems

  • massive code duplication
  • no encapsulation, most private data is accessible from outside, some of the business data also made singletons so it's not just changeable from outside but also from everywhere.
  • no abstractions (e.g. no business model, business data is stored in Object[] and double[][]), so no OO.

There is a good regression test suite and an efficient QA team is testing and finding bugs. I know the techniques how to get it under test from classic books, e.g. Michael Feathers, but that's too slow. As there is a working regression test system I'm not afraid to aggressively refactor the system to allow unit tests to be written.

How should I start to attack the problem to get some coverage quickly, so I'm able to show progress to management (and in fact to start earning from safety net of JUnit tests)? I do not want to employ tools to generate regression test suites, e.g. AgitarOne, because these tests do not test if something is correct.

  • Why not create the regression tests automatically, and verify each one individually? Gotta be faster than writing them all by hand. – Robert Harvey Nov 20 '11 at 23:14
  • It sounds a bit funny to be calling anything written in Java legacy, but agreed, it certainly is legacy. You mention you aren't afraid to refactor the system to allow unit tests to be written, but shouldn't you write the unit tests on the system as is, before any refactoring is attempted? Then your refactoring can be run through the same units tests to ensure nothing is broken? – dodgy_coder Nov 21 '11 at 2:38
  • 1
    @dodgy_coder Usually agree, but I hope the traditional QA which works efficiently would safe me some time. – Peter Kofler Nov 21 '11 at 10:36
  • 1
    @dodgy_coder Michael C. Feathers, author of Working Effectively with Legacy code defines Legacy code as "code without tests." It serves as a useful definition. – StuperUser Nov 21 '11 at 12:00

I believe there are two main axes along which code can be placed when it comes to introducing unit tests: A) how testable is the code? and B) how stable is it (i.e. how urgently does it need tests)? Looking only at the extremes, this yields 4 categories:

  1. Code that is easy to test and brittle
  2. Code that is easy to test and stable
  3. Code that is hard to test and brittle
  4. Code that is hard to test and stable

Category 1 is the obvious place to start, where you can get much benefit with relatively little work. Category 2 allows you to quickly improve your coverage statistic (good for morale) and get more experience with the codebase, while category 3 is more (often frustrating) work but also yields more benefit. Which you should do first depends on how important morale and coverage statistics are for you. Category 4 is probably not worth bothering with.

  • Excellent. I have an idea how to determine if it's easy to check by static analysis, e.g. dependency count/Testability Explorer. But how could I determine if code is brittle? I can't match defects to specific units (e.g. classes), and if it's number 3 of course (god classes/singletons). So maybe number of checkins (the hotspots)? – Peter Kofler Nov 21 '11 at 10:32
  • 1
    @Peter Kofler: commit hotspots are a good idea, but the most valuable source of this kind of knowledge would be developers who have worked with the code. – Michael Borgwardt Nov 21 '11 at 10:43
  • 1
    @Peter - like Michael said, the developers who have worked with the code. Anyone who has worked with a large codebase for a fair amount of time will know which parts of it smell. Or, if the whole thing smells, which parts of it really reek. – Carson63000 Nov 21 '11 at 23:44

I have a lot of experience working on legacy systems (not Java though), much larger than this. I hate to be the bearer of bad news, your problem is the size of you problem. I suspect you have underestimated it.

Adding regression tests to legacy code is a slow, expensive process. Many requirements are not well documented - a bug fix here, a patch there, and before you know it, the software defines it's own behavior. Not having tests means that the implementation is all there is to go by, no tests to "challenge" the implicit requirements implemented in the code.

If you try to get coverage quickly, it is likely you will rush the job, half bake it, and fail. The tests will give partial coverage of the obvious stuff, and poor to no coverage of the real issues. You will convince the very managers you are trying to sell to that Unit Testing is not worth it, that it's just another silver bullet that does not work.

IMHO, The best approach is to target you testing. Use metrics, gut feeling and defect log reports to identify the 1% or 10% of code that produces the most problems. Hit these modules hard, and ignore the rest. Don't try to do too much, less is more.

A realistic goal is "Since we implemented UT, defect insertion in modules under test has dropped to x% of those not under UT" (ideally x is a number <100).

  • +1, you can't unit test something effectively without a stronger standard to go by than the code. – dan_waterworth Nov 21 '11 at 9:09
  • I know and I agree. The difference is we have test, traditional regression testing by working QA in place, so there is some kind of safety net. Second I am deeply in favor of unit tests, so it will definitely not be another thing that did not work. A good point on what to target first. Thanks. – Peter Kofler Nov 21 '11 at 10:26
  • 1
    and don't forget that merely aiming for "coverage" isn't going to improve quality as you're going to get stuck in a morass of flawed and trivial tests (and tests for trivial code that doesn't need explicit testing, but are added just to increase coverage). You're going to end up creating tests to please the coverage tool, not because they're useful, and possibly going to change the code itself to increase test coverage without writing tests (like cutting out comments and variable definitions, which some coverage tools will call uncovered code). – jwenting Dec 5 '11 at 6:43

I'm reminded of that saying about not worrying about the barn door when the horse has already bolted.

The reality is that there really isn't a cost effective way to get good test coverage for a legacy system, certainly not in a short time frame. As MattNz mentioned, it's going to be a very time consuming process, and ultimately costly in the extreme. My gut tells me that if you attempt to do something to impress management, you'll likely create a new maintenance nightmare because of trying to show too much too quickly, without fully understanding the requirements that you are attempting to test for.

Realistically, once you have already written the code, it's almost to late to write the tests effectively without the risk of missing something vital. On the other hand, you could say that some tests are better than no tests, but if that's the case, the tests themselves need to show that they add value to the system as a whole.

My suggestion would be to look at those key areas where you feel something is "broken". By that I mean it could be terribly inefficient code, or that you can demonstrate has been previously costly to maintain. Document the problems, then use this as a starting point to introduce a level of testing that helps you to improve the system, without embarking on a massive re-engineering effort. The idea here is to avoid playing catch-up with the tests, and instead introduce tests to help you solve specific problems. After a period of time, see if you can measure and distinguish between the previous cost of maintaining that section of code, and the current efforts with the fixes you have applied with their supporting tests.

The thing to remember is that management are more interested in the cost/benefit and how that directly affects their customers, and ultimately their budget. They are never interested in doing something simply because it is the best thing to do unless you can prove that it will provide them with a benefit that is of interest to them. If you're able to show that you are improving the system and getting good test coverage for the work you are presently doing, management are more likely to see this as an efficient application of your efforts. This could possibly allow you to argue the case for extending your efforts to other key areas, without demanding either a complete freeze of the product's development, or even worse the nearly impossible to argue for rewrite!


One way to improve coverage is to write more tests.

Another way is to reduce the redundancy in your code, in such a way that existing tests in effect cover redundant code not presently covered.

Imagine you have 3 code blocks, a, b, and b', where b' is a duplicate (exact or near miss copy) of B, and that you have coverage on a and b but not b' with test T.

If refactor the code base to eliminate b' by extracting the commonality from b and b' as B, the code base now looks like a, b0, B, b'0, with b0 containing the nonshared code with b'0 and vice-versa, and both b0 and b'0 being much smaller than B, and invoking/using B.

Now the functionality of the program hasn't changed, and neither has test T, so we can run T again and expect it to pass. The code now covered is a, b0, and B, but not b'0. The code base has gotten smaller (b'0 is smaller than b'!) and we still cover what we originally covered. Our coverage ratio has gone up.

To do this, you need to find b, b' and ideally B to enable your refactoring. Our CloneDR tool can do this for many languages, especially including Java. You say your code base has lots of duplicated code; this might be a good way to tackle it to your benefit.

Oddly, the act of find b and b' often increases your vocabulary about the abstract ideas the program implements. While the tool has no idea what b and b' do, the very act of isolating them from the code, allowing simple focus on the content of b and b', often gives programmers an very good idea of what abstraction B_abstract the cloned code implements. So this improves your understanding of the code, too. Make sure you give B a good name when you abstact it out, and you'll get better test coverage, and a more maintainable program.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.