Getting At Awkward Code, For Great Testing

A couple days ago, I wound up helping out a colleague on a problem he was having coming up with an appropriate test case for a piece of code in a Rails app. It reminded me that I had wanted to talk about how structuring your code affects what you are actually testing, and how that related to what you want to test.

While addressing that is a much more considerable article, here’s a quick look at the issue in terms of the specific problem that came up.

The code in question looked a bit like this:

1
2
3
4
5
6
7
8
9

class SomeController < ApplicationController
  def action_of_interest
    @entity = Rails.cache.fetch("some_prefix_#{params[:id]}") do
      SomeModel.includes(:some_other_model).where(:id => params[:id])
    end
 
    # Then, things are done with @entity here.
  end
end

The problem was that the colleague only wanted to test the caching semantics, not the remaining semantics of the action.

His first thought was to use stubbing to achieve the desired result, but that had been proving awkward due to the comparative complexity of the steps following the cache lookup.

Hoisting For Fun and Profit

I proposed a simple solution that didn’t add a lot of abstractions or otherwise over-generalize things at this stage.

Simply put: Make a helper method that contains the body of the do..end block, call that helper from said block, and call it from the test case. In short, hoist the code that’s making things awkward to test, so that you aren’t fighting the structure of your code when writing your tests.

So we wound up with code like this:

1
2
3
4
5
6
7
8
9
10
11

class SomeController < ApplicationController
  include SomeHelper
 
  def action_of_interest
    @entity = Rails.cache.fetch("some_prefix_#{params[:id]}") do
      fetch_some_model_for_some_purpose(params[:id])
    end
 
    # Then, things are done with @entity here.
  end
end

And:

1
2
3
4
5

module SomeHelper
  def fetch_some_model_for_some_purpose(id)
    return SomeModel.includes(:some_other_model).where(:id => id)
  end
end

My colleague was skeptical of this approach, seeing it as needlessly tying the implementation and the test together until I asked one key question:

If someone changes the application logic – but not the cache semantics – does this test break?

And with that, the idea I was getting at began to hit home: You’ve got the right separation of concerns when a change unrelated to what you want to test won’t break the test.