I'm currently looking at a code base that could do with some refactoring, overall it's a pretty good code base, and the test coverage is pretty good too. But looking through it gave me some ideas on what makes a code base tricky to safely refactor. Here are some tips to make sure your code is in a good state to be refactored with safety and confidence.
Provide Good Test Names
Test methods should have good, meaningful names; this is of course exactly the same as all code - naming is very important to give context and meaning. I am looking at a lot of tests that make sense to the original developer, and within reason make sense to me, but they do not properly describe what the test tries to do and what it expects.
A test named "ValidHardware()" is less than ideal for telling me the intent of the test. If the method had been correctly named it would have been called:
ensure_that_hardware_detector_returns_true_when_valid_hardware_is_present()
Now you tell me, which is easier to understand? And which is going to give me more confidence in refactoring that hardware detector?
Make Sure Variables Are Well Named
Probably blindingly obvious given the above section, but if you use poor variable names, I am probably going to waste a lot of time trying to figure out exactly which thing is which, and worse still, I am probably going to miss subtle bugs.
Cover The Right Things
Beware of high coverage that does not actually test either all the aspects of a component, or that makes a lot of assumptions.
Getting 100% coverage is likely to be impossible, and probably your return on investment over 75% is going to be low, but don't achieve the 75% by testing the trivial and easy to test things only - it's going to be those tricky to test components that will cause most bugs when refactoring.
If something is tricky to test, perhaps you have the wrong level of abstraction applied, or perhaps the test is trying to tell you something. Skipping it and going for some simpler tests may pay off in the short term, but in the long term will hurt you.
Avoid Fragile Tests
There is a lot of debate between the "classicist and mockist schools" about what to test, and personally I veer towards state based testing for most components - but regardless of where your personal impulses lie, avoid creating fragile tests. Any test that is likely to break with trivial refactoring could be considered fragile, and those tests are going to cost you a lot of time and cause many potential bugs when refactoring.
One of the main causes of fragile tests are an overreliance on mocks, and specifically using mocks over stubs. By all means mock where necessary, but try to limit mock usage to very specific test cases, and try to favour stubs over mocks and favour "Setup.ResultFor" over "Expect.Call" type semantics.
Avoid [Setup] and [Teardown]
Yes, I know they make it easier to create lots of similar tests, but they make your tests fragile and interdependent.
James Newkirk (who was on the original NUnit team) made a good blog post on this when he moved to writing xUnit, where Setup and Teardown were removed.
They make it hard to read your tests (you have to read code in two or three locations to understand what the test does) and can cause Setup/Teardown getting too many responsibilities, leading to fragility or code bloat.
Either switch to setup within the individual test, or favour a Context based testing approach such as with BDD
Conclusion
Tests are code, and code has to be maintained too. Make sure your tests are of a similar or higher standard than the rest of your codebase, make sure they test the right things, in the right way, and you will make your life much easier, and that of the guy who comes after you much easier too
Posted
09-21-2009 4:17 PM
by
Jak Charlton