Test-Commit-Revert: A Useful Workflow for Testing Legacy Code in Ruby

This article was originally written by José M. Gilgado on the Honeybadger Developer Blog.

It happens to all of us. As software projects grow, parts of the codebase end up in production without a comprehensive test suite. When you take another look at the same area of code after a few months, it may be difficult to understand; even worse, there might be a bug, and we don't know where to begin fixing it.

Modifying code without tests is a major challenge. We can't be sure if we'll break anything in the process, and checking everything manually is, at best, prone to mistakes; usually, it's impossible.

Dealing with this kind of code is one of the most common tasks we perform as developers, and many techniques have focused on this issue over the years, such as characterization tests, which we discussed in a previous article.

Today, we'll cover another technique based on characterization tests and introduced by Kent Beck, who also introduced TDD to the modern programming world many years ago.

What's TCR?

TCR stands for "test, commit, revert", but it's more accurate to call it "test && commit || revert". Let's see why.

This technique describes a workflow to test legacy code. We'll use a script that will run the tests every time we save our project files. The process is as follows:

• First, we create an empty unit test for the part of the legacy code we want to test.
• We then add a single assertation and save the test.
• Since we have our script set up, the test is automatically run. If it succeeds, the change is committed. If it fails, the change is deleted (reverted), and we need to try again.

Once the test passes, we can then add a new test case.

Essentially, TCR is about keeping your code in a "green" state instead of writing a failing test first (red) and then make it pass (green), as we do with test-driven development. If we write a failing test, it'll just vanish, and we'll be brought back to the "green" state again.

Purpose

The main goal of this technique is to understand the code a bit better each time you add a test case. This will naturally increase the test coverage and unblock many refactorings that, otherwise, wouldn't be possible.

One of the advantages of TCR is that it's useful in many scenarios. We can use it with code that has no tests at all or with code that's partially tested. If the test doesn't pass, we just revert the change and try again.

How can we use it?

Kent Beck shows, in different articles and videos (linked at the end), that a good approach is using a script that runs after certain files in the project are saved.

This will depend heavily on the project you're trying to test. Something like the following script, which is executed every time we save files with a plugin in the editor, is a good start:

(rspec && git commit -am "WIP") || git reset --hard

If you're using Visual Studio Code, a good plugin to execute on every save is "runonsave". You can include the above command or a similar one for your project. In this case, the whole config file would be

{
  "folders": [{ "path": "." }],
  "settings": {
    "emeraldwalk.runonsave": {
      "commands": [
        {
          "match": "*.rb",
          "cmd": "cd ${workspaceRoot} && rspec && git commit -am WIP || git reset --hard"
        }
      ]
    }
  }
}

Remember that later, you can squash the commit with Git directly in the command line or when merging the PR if you're using Github:

.

This means we'll only get one commit in the main branch for all the commits we did on the branch we're working on. This diagram from Github explains it well:

.

Writing our first test with TCR

We'll use a simple example to illustrate the technique. We have a class that we know is working, but we need to modify it.

We could just make a change and deploy the changes. However, we want to be sure that we don't break anything in the process, which is always a good idea.

# worker.rb
class Worker
  def initialize(age, active_years, veteran)
    @age = age
    @active_years = active_years
    @veteran = veteran
  end

  def can_retire?
    return true if @age >= 67
    return true if @active_years >= 30
    return true if @age >= 60 && @active_years >= 25
    return true if @veteran && @active_years > 25

    false
  end
end

The first step would be to create a new file for the tests, so we can start adding them there. We've seen the first line in the can_retire? method with

  def can_retire?
    return true if @age >= 67
    ...
    ...
  end

Thus, we can test this case first:

# specs/worker_spec.rb
require_relative './../worker'

describe Worker do
  describe 'can_retire?' do
    it "should return true if age is higher than 67" do

    end
  end
end

Here's a quick tip: when you're working with TCR, every time you save, the latest changes will disappear if the tests don't pass. Therefore, we want to have as much code as possible to "set up" the test before actually writing and saving the line or lines with the assertion.

If we save the above file like that, we can then add a line for the test.

require_relative './../worker'

describe Worker do
  describe 'can_retire?' do
    it "should return true if age is higher than 67" do
      expect(Worker.new(70, 10, false).can_retire?).to be_true ## This line can disappear when we save now
    end
  end
end

When we save, if the new line doesn't vanish, we've done a good job; the test passes!

Adding more tests

Once we have our first test, we can keep adding more cases while taking into account false cases. After some work, we have something like this:

# frozen_string_literal: true

require_relative './../worker'

describe Worker do
  describe 'can_retire?' do
    it 'should return true if age is higher than 67' do
      expect(Worker.new(70, 10, false).can_retire?).to be true
    end

    it 'should return true if age is 67' do
      expect(Worker.new(67, 10, false).can_retire?).to be true
    end

    it 'should return true if age is less than 67' do
      expect(Worker.new(50, 10, false).can_retire?).to be false
    end

    it 'should return true if active years is higher than 30' do
      expect(Worker.new(60, 31, false).can_retire?).to be true
    end

    it 'should return true if active years is 30' do
      expect(Worker.new(60, 30, false).can_retire?).to be true
    end
  end
end

In every case, we write the "it" block first, save, and then add the assertion with expect(...).

As usual, we can add as many tests as possible, but it makes sense to avoid adding too many once we're relatively sure that everything is covered.

There are still a few cases to cover, so we should add them just for completeness.

Final tests

Here's the spec file in its final form. As you can see, we could still add more cases, but I think this is enough to illustrate the process of TCR.

# frozen_string_literal: true

require_relative './../worker'

describe Worker do
  describe 'can_retire?' do
    it 'should return true if age is higher than 67' do
      expect(Worker.new(70, 10, false).can_retire?).to be true
    end

    it 'should return true if age is 67' do
      expect(Worker.new(67, 10, false).can_retire?).to be true
    end

    it 'should return true if age is less than 67' do
      expect(Worker.new(50, 10, false).can_retire?).to be false
    end

    it 'should return true if active years is higher than 30' do
      expect(Worker.new(60, 31, false).can_retire?).to be true
    end

    it 'should return true if active years is 30' do
      expect(Worker.new(20, 30, false).can_retire?).to be true
    end

    it 'should return true if age is higher than 60 and active years is higher than 25' do
      expect(Worker.new(60, 30, false).can_retire?).to be true
    end

    it 'should return true if age is higher than 60 and active years is higher than 25' do
      expect(Worker.new(61, 30, false).can_retire?).to be true
    end

    it 'should return true if age is 60 and active years is higher than 25' do
      expect(Worker.new(60, 30, false).can_retire?).to be true
    end

    it 'should return true if age is higher than 60 and active years is 25' do
      expect(Worker.new(61, 25, false).can_retire?).to be true
    end

    it 'should return true if age is 60 and active years is 25' do
      expect(Worker.new(60, 25, false).can_retire?).to be true
    end

    it 'should return true if is veteran and active years is higher than 25' do
      expect(Worker.new(60, 25, false).can_retire?).to be true
    end
  end
end

Ways to Refactor

If you've read this far, there's probably something that feels a bit off with the code. We have many "magical numbers" that should be extracted into constants, both in the test and in the Worker class.

We could also create private methods for each case in the main can_retire? public method.

I'll leave both potential refactorings as exercises for you. However, we have tests now, so if we make a mistake in any step, they will tell us.

Conclusions

I encourage you to try TCR with your projects. It's a very cheap experiment because you don't need any fancy continuous integration in an external server or a dependency with a new library. All you need is a way to execute a command every time you save certain files on your computer.

It'll also give you a "gaming" experience when adding tests, which is always fun and interesting. Additionally, the discipline of having failing tests removed from your editor will give you an extra safety net by confirming that the tests you're pushing to the repository are passing.

I hope you find this new technique useful when dealing with legacy code. I've used multiple times in the last few months, and it's always been a pleasure.

Extra Resources

• Good video as an introduction.
• Kent Beck's take on how to use TCR in VS Code.
• VS Code's plugin to run a script on save.

About Honeybadger

Honeybadger has your back when it counts. We're the only error tracker that combines exception monitoring, uptime monitoring, and cron monitoring into a single, simple to use platform.
 Our mission: to tame production and make you a better, more productive developer. Learn more