One common question when performing a Move Method or Pull Up Method refactoring is what to do with the tests. Consider the following simple Order class:

We are interested in extracting #print_receipt and moving it to a Receipt class. Here are the existing specs:

A simple Move Method refactoring

Refactoring to use a Receipt class is trivial. We start by creating a Receipt, passing in the Order object.

Next we copy #print_receipt to Receipt and change the receiver of the #customer, #shipping_address, and #total_price methods to be the Order:

And finally we delegate Order#print_receipt to the new Receipt class:

What about the tests?

This is a typical refactoring, and one of the reasons we have unit tests is to enable us to perform such refactorings. However there’s one question remaining - what should we do about the tests?

There are usually two choices at this point:

• Do nothing. We haven’t changed the system’s behavior, and all the tests are green, so we can just leave things as they are
• Copy the tests into receipt_spec.rb and modify them to work for receipts

Doing nothing is an okay option. We’ve done a pure refactoring, after all, so there’s nothing driving the addition/modification of specs. There currently aren’t any other clients of Receipt, so it doesn’t necessarily need to be fully spec’d out. I’ve read Michael Feathers suggest leaving the tests alone for the time being, and then writing/moving tests any time you use untested code. More concretely, I might leave all the tests alone, and then if I ever wrote more code that used Receipt#print down the road then I’d write specs for it.

While I don’t think that leaving the specs alone is bad, I don’t think it’s particularly nice either. Specs have four major benefits:

• help drive the design of code
• provide regression tests
• enable refactoring
• serve as documentation

The last one is especially important for me because I never comment or document my code. That’s not an exaggeration either, I really mean never.

It would be nice if we had some documentation around Receipt#print in the form of executable specs. The traditional way to do this would be to copy the specs from Order#print_receipt and modify them to work for a Receipt. (You might also move the specs before moving the implementation, or you might change the Order#print_receipt specs to use a mock instead. Those are decisions that are not that interesting in the context of this post)

Introducing shared example groups

The only problem with copying the specs is that doing so introduces duplication. Some amount of duplication in specs is acceptable and sometimes even desirable, but in this case we’d have functional duplication which is almost always bad.

We can take advantage of shared example groups to add coverage to Receipt without introducing bad duplication. To do this, we’ll extract a shared example group out of the existing example group in order_spec.rb.

Because we’re just printing to a buffer, we can reuse nearly all of the specs. The first step is to generalize the receipt printing code in the spec:

All I’ve done in this case is extracted the receipt printing to a method called #print_receipt. Now I’ll create a shared example group and move the first example to it:

Those all run, so now we can move the rest of the specs over:

Now to write the spec for Receipt#print, we just need to write an example group, do a bit of setup and implement #print_receipt, and use the shared example group:

Steps for refactoring to shared example groups

There we go, a nice way to handle refactoring specs along with production code. Briefly, the steps are:

1. Extract behavior to a method in original example group
2. Create a new shared example group
3. Make original EG behave like new SEG
4. Move examples to SEG

Some final thoughts

I didn’t make these specs as clean as I could have, because I just wanted to show you the basics of this refactoring. The two things that concern me about the example code as I’ve left it are that the shared example group uses instance variables defined in the concrete example groups, and that there’s duplication in setup between the Order and Receipt specs.

The first problem is pretty simple. Just do attr_reader :receipt_output in both example groups and change the shared example group to use that method instead. This way it’s relying on an API instead of mixing up API and instance data.

The second problem could be handled a number of ways. You could move some of the setup code into the shared example group itself. You might be using something like fixture_scenarios on your project, or maybe you’re using an object mother. And of course in the case of receipt_spec.rb, you might use a mock Order instead of a real one.

I really like this pattern for certain refactorings, where the code is organized in one place but executed in multiple places (think of pushing an instance method up to a superclass). We can lean on our test suite to do the refactoring, and then easily provide direct coverage for any new methods and classes.

My only gripe about this style is that the tests are a little removed from the code. Instead of just looking at receipt_spec.rb, I have to go look at where the shared example group is defined. However that is much better than before, where Receipt#print was tested, but the specs were very far removed from it, living in order_spec.rb. In this case, the reduction of duplication is a big win that outweights the little bit of indirection.

Special thanks to Nick Kallen and Jonathan Barnes for a good discussion on moving specs when refactoring code