views:

249

answers:

5

Lately I had to change some code on older systems where not all of the code has unit tests.
Before making the changes I want to write tests, but each class created a lot of dependencies and other anti-patterns which made testing quite hard.
Obviously, I wanted to refactor the code to make it easier to test, write the tests and then change it.
Is this the way you'd do it? Or would you spend a lot of time writing the hard-to-write tests that would be mostly removed after the refactoring will be completed?

Any insights are welcome!

+4  A: 

First of all, here's a great article with tips on unit testing. Secondly, I found a great way to avoid making tons of changes in old code is to just refactor it a little until you can test it. One easy way to do this is to make private members protected, and then override the protected field.

For example, let's say you have a class that loads some stuff from the database during the constructor. In this case, you can't just override a protected method, but you can extract the DB logic to a protected field and then override it in the test.

public class MyClass {
    public MyClass() {
        // undesirable DB logic
    }
}

becomes

public class MyClass {
    public MyClass() {
        loadFromDB();
    }

    protected void loadFromDB() {
        // undesirable DB logic
    }
}

and then your test looks something like this:

public class MyClassTest {
    public void testSomething() {
        MyClass myClass = new MyClassWrapper();
        // test it
    }

    private static class MyClassWrapper extends MyClass {
        @Override
        protected void loadFromDB() {
            // some mock logic
        }
    }
}

This is somewhat of a bad example, because you could use DBUnit in this case, but I actually did this in a similar case recently because I wanted to test some functionality totally unrelated to the data being loaded, so it was very effective. I've also found such exposing of members to be useful in other similar cases where I need to get rid of some dependency that has been in a class for a long time.

I would recommend against this solution if you are writing a framework though, unless you really don't mind exposing the members to users of your framework.

It's a bit of a hack, but I've found it quite useful.

Mike Stone
A: 

I am not sure why would you say that unit tests are going be removed once refactoring is completed. Actually your unit-test suite should run after main build (you can create a separate "tests" build, that just runs the unit tests after the main product is built). Then you will immediately see if changes in one piece break the tests in other subsystem. Note it's a bit different than running tests during build (as some may advocate) - some limited testing is useful during build, but usually it's unproductive to "crash" the build just because some unit test happens to fail.

If you are writing Java (chances are), check out http://www.easymock.org/ - may be useful for reducing coupling for the test purposes.

Valters Vingolds
+3  A: 

@valters

I disagree with your statement that tests shouldn't break the build. The tests should be an indication that the application doesn't have new bugs introduced for the functionality that is tested (and a found bug is an indication of a missing test).

If tests don't break the build, then you can easily run into the situation where new code breaks the build and it isn't known for a while, even though a test covered it. A failing test should be a red flag that either the test or the code has to be fixed.

Furthermore, allowing the tests to not break the build will cause the failure rate to slowly creep up, to the point where you no longer have a reliable set of regression tests.

If there is a problem with tests breaking too often, it may be an indication that the tests are being written in too fragile a manner (dependence on resources that could change, such as the database without using DB Unit properly, or an external web service that should be mocked), or it may be an indication that there are developers in the team that don't give the tests proper attention.

I firmly believe that a failing test should be fixed ASAP, just as you would fix code that fails to compile ASAP.

Mike Stone
A: 

This book may help (I have seen it recommended by people I trust, but I have not read it): http://www.amazon.com/Working-Effectively-Legacy-Robert-Martin/dp/0131177052

Michael Neale
A: 

I have read Working Effectively With Legacy Code, and I agree it is very useful for dealing with "untestable" code.

Some techniques only apply to compiled languages (I'm working on "old" PHP apps), but I would say most of the book is applicable to any language.

Refactoring books sometimes assume the code is in semi-ideal or "maintenance aware" state before refactoring, but the systems I work on are less than ideal and were developed as "learn as you go" apps, or as first apps for some technologies used (and I don't blame the initial developers for that, since I'm one of them), so there are no tests at all, and code is sometimes messy. This book addresses this kind of situation, whereas other refactoring books usually don't (well, not to this extent).

I should mention that I haven't received any money from the editor nor author of this book ;), but I found it very interesting, since resources are lacking in the field of legacy code (and particularly in my language, French, but that's another story).

Ced-le-pingouin