22

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.

  • 1
    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
12

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.

| improve this answer | |
  • 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
16

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).

| improve this answer | |
  • +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
3

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!

| improve this answer | |
2

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.

| improve this answer | |
1

s. robins answer is very thoughtful and worth keeping in mind.

particularly struggle with 'just wait and see', as frequently when a project goes over schedule, it's often because a legacy library had poor test coverage that caused extra software updates to learn and fix problem.

so simply waiting to fix these problems is already less than ideal. totally agree that finding every untested bug would not be more efficient, but believe a smart way of tackling this could yield much better results.

We've focused on improving tooling to better identify high roi ideas:

  • wrote a series of small tools to slice and dice coverage reports
  • this allows devs to understand patterns in classes/calls missing test coverage
  • lets them identify which classes/calls are used most, as well as those with poorest coverage
  • together that gives some basic ideas about where biggest exposure is
  • when you combine this by business impact of groups of class/calls, it gives more information where most leverage is
  • sometimes there are other patterns that emerge. in one of these exercises, 20% of untested methods were get/set accessors, which weren't tested directly but were one-liners to add even on really old tests. it took about half a day for a developer to double average test coverage.

We're looking at it from the perspective of filtering out everything but high-quality test adds.

A high quality test generally:

  1. does not involve writing a lot of test code
  2. it definately doesn't involve changing implementation code
  3. is executed inside an already defined/meaningful business/service logic example

So at least for us, improving coverage quality AND quantity is best done by finding these small missing elements (often one-liners) from existing test suites.

I will note that our examples are a little different than the methods espoused above- we briefly looked at eliminating less used classes to improve coverage (and potentially reduce some redudancy), but in our case most of our coverage units had some coverage. So simply removing things didn't add a lot of value (especially given long-tail risks to downstream users). It also means that it was easier for us to augment our existing tests to add realism, without having to try to re-understand or re-conceive business logic (which can be bad if done by wrong person, however you can do a lot to de-risk this stuff with not too much effort).

We're also interested in AI-based testing as a sort of stop-gap between test logic improvements and status quo. Most of these approaches work by tagging these classes/methods explicitly, so you can track human test coverage vs human-computer test coverage seperately. No consensus on this yet but there are situations where it's better than no test.

Overall the purpose of tests is that they're supposed to express realistic user situations and scenarios. If this is difficult, it's worth investigating because it's teaching you something about the software. Once you learn the lessons, you can look at costs and determine whether to address with legacy tests or new logic or both as appropriate.

this answer on quora is also pretty good he differentiates coverage from complexity and favors depth of coverage over quantity, identifying complex algos to maximize his test roi

| improve this answer | |

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