In Progress
Unit 1, Lesson 21
In Progress

Mocking Smells 2

Video transcript & code

Today I thought we'd talk about a few more code smells to be found in tests that make use of mock or stub objects.

First, here's one I run across pretty often. We have a Purchase class, which is the class we want to test. In order to calculate totals, it uses a <>TaxPolicy to determine the <>appropriate tax rate. The TaxPolicy is a simple value-holder object, with no special behavior or dependencies.

Our test creates a policy as an RSpec test-double object, which will return a rate of 0.05. Then we create a new Purchase, passing in a subtotal and our fake TaxPolicy. Finally, we assert that the calculated purchase total includes the appropriate amount of tax.

Tax Policy = Struct.new(:name, :rate)

class Purchase
  def initialize(subtotal:, tax_policy:)
    @subtotal = subtotal
    @tax_policy = tax_policy
  end

  def total
    @subtotal + @subtotal * @tax_policy.rate
  end
end

require "rspec/autorun"

RSpec.describe Purchase do
  it "can calculate a total with tax" do
    policy = double(rate: 0.05)
    purchase = Purchase.new(subtotal: 10.0, tax_policy: policy)
    expect(purchase.total).to eq(10.50)
  end
end
# >> .
# >>
# >> Finished in 0.00132 seconds (files took 0.11178 seconds to load)
# >> 1 example, 0 failures

The smell here is a double used in place of a value object. Ordinarily, we use test doubles when we want to isolate the test and the object under test from dependencies on other objects. We want to make sure our tests don't become coupled to the structure of relationships between objects, or to complex object setup. We may be writing the test before the collaborator object even exists yet. And more pragmatically, we don't want class dependencies dragging in lots of extra gem requirements.

But in this case, the dependency is on a simple value object class. This class already exists. It is so simple it might even be included in the same file as the Purchase class that we're testing. And even if it isn't, requiring it isn't likely to load a lot of other files, since it is a basic Struct.

In situations like this, it doesn't make a lot of sense to use a test double. A double always adds a little bit of comprehension overhead to a test. If we can use the actual value object without adding dependencies or extra test setup, we should.

To fix this smell, we get rid of the double and replace it with an actual TaxPolicy object.

Now, before we go on to fill in my preferred version of this code, I want to quickly show another, related smell I frequently see. We have a real TaxPolicy object… but then we use RSpec's stubbing mechanism to fake out its rate message to return the test rate.

TaxPolicy = Struct.new(:name, :rate)

class Purchase
  def initialize(subtotal:, tax_policy:)
    @subtotal = subtotal
    @tax_policy = tax_policy
  end

  def total
    @subtotal + @subtotal * @tax_policy.rate
  end
end

require "rspec/autorun"

RSpec.describe Purchase do
  it "can calculate a total with tax" do
    policy = TaxPolicy.new
    allow(policy).to receive(:rate).and_return(0.05)
    purchase = Purchase.new(subtotal: 10.0, tax_policy: policy)
    expect(purchase.total).to eq(10.50)
  end
end

# >> .
# >>
# >> Finished in 0.00797 seconds (files took 0.08121 seconds to load)
# >> 1 example, 0 failures

This is the worst of both worlds: we are including a dependency on the real TaxPolicy class in the test, but we are also introducing the syntactic complexity of a stubbed-out method.

None of this is necessary. We don't need mocking or stubbing at all in a test like this. Instead, we can simply create a real TaxPolicy value object with its fields filled in with test values.

TaxPolicy = Struct.new(:name, :rate)

class Purchase
  def initialize(subtotal:, tax_policy:)
    @subtotal = subtotal
    @tax_policy = tax_policy
  end

  def total
    @subtotal + @subtotal * @tax_policy.rate
  end
end

require "rspec/autorun"

RSpec.describe Purchase do
  it "can calculate a total with tax" do
    policy = TaxPolicy.new("Test Policy", 0.05)
    purchase = Purchase.new(subtotal: 10.0, tax_policy: policy)
    expect(purchase.total).to eq(10.50)
  end
end

# >> .
# >>
# >> Finished in 0.00135 seconds (files took 0.09494 seconds to load)
# >> 1 example, 0 failures

Let's now switch things up a bit and say that instead of individual tax policies, our program is even simpler and includes a single, global tax policy. We have a Settings module, with a tax_rate attribute defined using the fattr gem we introduced in episode #276. The Purchase class uses this attribute when calculating totals.

Our test forces the tax_rate to a test-defined rate by stubbing out the method on the module before asserting that the right total emerges.

require "fattr"

module Settings
  Fattr :tax_rate, 0.06
end

class Purchase
  def initialize(subtotal:)
    @subtotal = subtotal
  end

  def total
    @subtotal + @subtotal * Settings.tax_rate
  end
end

require "rspec/autorun"

RSpec.describe Purchase do
  it "can calculate a total with tax" do
    allow(Settings).to receive(:tax_rate).and_return(0.05)
    purchase = Purchase.new(subtotal: 10.0)
    expect(purchase.total).to eq(10.50)
  end
end

# >> .
# >>
# >> Finished in 0.0046 seconds (files took 0.09675 seconds to load)
# >> 1 example, 0 failures

This is more justifiable than the stubbing we saw before. Still, there is an alternative that's worth considering. Instead of using stubs, we can simply update the actual settings attribute to the desired value for the test.

"But wait!" you might be yelling right about now. "Doesn't this mean the value will leak across tests???" This a very valid objection, and an important consideration when designing robust tests.

However, if we have a reliable way to reset the settings before each test, this might not be quite as alarming. As it turns out, since we used fattr to define the attribute, we can set up a before handler to reset the value to its default simply by using the bang version.

require "fattr"

module Settings
  Fattr :tax_rate, 0.06
end

class Purchase
  def initialize(subtotal:)
    @subtotal = subtotal
  end

  def total
    @subtotal + @subtotal * Settings.tax_rate
  end
end

require "rspec/autorun"

RSpec.describe Purchase do
  before do
    Settings.tax_rate!
  end

  it "can calculate a total with tax" do
    Settings.tax_rate = 0.05
    purchase = Purchase.new(subtotal: 10.0)
    expect(purchase.total).to eq(10.50)
  end
end

# >> .
# >>
# >> Finished in 0.00076 seconds (files took 0.09011 seconds to load)
# >> 1 example, 0 failures

With more than a few attributes to reset this could become unwieldy and couple the test unnecessarily closely to the Settings module. One way to address this problem can be seen in episode #172, where we came up with a way to confine seemingly-global variables to a resettable scope.

We've now seen three more smells that can occur in conjunction with test doubles. This isn't the end of my list of mocking smells, but it's enough for today. Happy hacking!

Responses