Reader Q&A: Tommy’s question about testing legacy code

by Jason Swett,

Code with Jason subscriber Tommy C. recently wrote in with the following question:

Jason,

So I have found that one of the hurdles to testing beginners face is that the code they are trying to test is not always very testable. This is either because they themselves have written it that way or because they have inherited it. So, this presents a sort of catch 22. You have code with no tests that is hard to test. You can’t refactor the code because there are no tests in place to ensure you have not changed the behavior of the code.

I noticed that you have said that you don’t bother to test controllers or use request specs. I agree that in your situation, since you write really thin controllers that is a good call. However, in my situation, I have inherited controllers that are doing some work that I would like to test. I would like to move that logic out eventually, but right now all I can get away with is adding some test/specs.

These are some of the things that make testing hard for me. When I’m working on a greenfield side project all is good, but you don’t always have clean, testable code to work with.

Thanks,
–Tommy

Chicken/egg problem

Tommy brings up a classic legacy project chicken/egg problem: you can’t add tests until you change the way the code is structured, but you’re afraid to change the structure of the code before you have tests. It’s a seemingly intractable problem but, luckily, there’s a path forward.

The answer is to make use of the Extract Method and Extract Class refactoring techniques, also known as Sprout Method and Sprout Class. The idea is that if you come across a method that’s too long to easily write a test for, you just grab a chunk of lines from the method and move it – completely unmodified – into its own method or class, and then write tests around that new method or class. These techniques are a way to be reasonably sure (not absolutely guaranteed, but reasonably sure) that your small change has not altered the behavior of the application.

I learned about Sprout Method/Sprout Class from Michael Feathers and Martin Fowler. I also wrote a post about using Sprout Method in Ruby here.

Request specs

To address controller tests/request specs: Tommy might be referring to a post I wrote where I said most of the time I don’t use controller specs/request specs. (I also wrote about the same thing in my book, Rails Testing for Beginners.) There are two scenarios where I do use request specs, though: API-only projects and legacy projects that have a bunch of logic in the controllers. I think Tommy is doing the exact right thing by putting tests around the controller code and gradually moving the code out of the controllers over time.

If you, like Tommy, are trying to put tests on a legacy project and finding it difficult, don’t despair. It’s just a genuinely hard thing. That’s why people have written entire books about it!

Do you have a question about testing Rails legacy code, or about anything else to do with testing? Just email me at jason@codewithjason.com or tweet me at @jasonswett. I’m able to, I’ll write an answer to your question just like I did with Tommy’s.

One thought on “Reader Q&A: Tommy’s question about testing legacy code

  1. Dan Barron

    Wouldn’t system tests also be useful here. If you can test the entire piece of functionality the problem code is part of with system tests, then if you alter its behavior, one or more tests should fail. Also not a guarantee of success, but maybe a good place to start.

    Reply

Leave a Reply

Your email address will not be published. Required fields are marked *