In Progress
Unit 1, Lesson 21
In Progress

Mocking Smells 3

Video transcript & code

Today I thought we might look at a few more cases of mock and stub objects being used in less than optimal ways.

First up, let's talk about the difference between a mocked method, and a stubbed method. In this code, we're testing a thermostat class. Periodically, it will receive a check_temperature message. When it does, it should read the current temperature from a thermometer object. If the temperature is cooler than a certain level, it should tell the furnace to light itself.

In order to test this, we have two test double objects: one for the thermometer, and one for the furnace. In a production scenario these would be proxies for real-world devices, so it makes sense to mock them out when we're in a unit testing environment.

We inject our doubles as dependencies into a new thermostat object. Then we set an expectation that the thermometer will receive a temp_f message, which will return a value of 67. Next, we set an expectation that that the furnace will receive a #turn_on message. Then we send the #check_temperature message.

This test is presently passing. And it looks like it is testing every last detail of the method. But is it really?

require "rspec/autorun"

class Thermostat
  def initialize(thermometer:,furnace:)
    @thermometer = thermometer
    @furnace     = furnace
  end

  def check_temperature
    temp = @thermometer.temp_f
    if temp = 67
      @furnace.turn_on
    end
  end
end

RSpec.describe Thermostat do
  it "turns on the furnace when the temperature is too low" do
    thermometer = double
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(thermometer).to receive(:temp_f).and_return(67)
    expect(furnace).to receive(:turn_on)
    thermostat.check_temperature
  end
end

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

Let's change the test so that the thermometer returns a nice comfortable temperature instead. Then let's run the test again. We expect to see a failure, as the thermostat no longer activates the furnace. But instead, it still passes. What's going on here?

require "rspec/autorun"

RSpec.describe Thermostat do
  it "turns on the furnace when the temperature is too low" do
    thermometer = double
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(thermometer).to receive(:temp_f).and_return(72)
    expect(furnace).to receive(:turn_on)
    thermostat.check_temperature
  end
end

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

Before we get to the root of the misbehavior, let's take another look at the RSpec example. This example looks like it tests a lot. And in fact, I'd say it tests too much. And yet, as we've just seen, it plainly also doesn't test enough.

expect(thermometer).to receive(:temp_f).and_return(72)

This example has a smell that is very common in specs I've seen in the wild. It sets an expectation, when a stub would have been more appropriate.

On this line, we tell the thermometer double that it must receive the #temp_f message, and we give it a value to return when it does. This is using the thermometer double as a mock object.

But this object is only being used as a source of input for the method under test. The important thing here is not that the #temp_f method is called; the important thing is that the #check_temperature method sees a temperature of 67 degrees Fahrenheit. Setting an expectation here binds the test tightly to the implementation of the method under test, without actually adding any value.

The thermometer would be more appropriately set up as a stub object, not as a mock object. To do this, we use allow instead of expect. The test will no longer assert that the method is called; but it will return the value of 67 if the method is called.

allow(thermometer).to receive(:temp_f).and_return(72)

But we're not going to leave it at that. An advantage to switching this double to behave as a stub instead of as a mock is that RSpec enables us to use a much more concise syntax to stub out methods. We remove the whole line, and instead set up the return value for the #temp_f method when we create the test double.

thermometer = double(temp_f: 67)

We now have a shorter, humbler example.

require "rspec/autorun"

class Thermostat
  def initialize(thermometer:,furnace:)
    @thermometer = thermometer
    @furnace     = furnace
  end

  def check_temperature
    temp = @thermometer.temp_f
    if temp = 67
      @furnace.turn_on
    end
  end
end

RSpec.describe Thermostat do
  it "turns on the furnace when the temperature is too low" do
    thermometer = double(temp_f: 67)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to receive(:turn_on)
    thermostat.check_temperature
  end
end

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

By simplifying the test, we've also made it clearer what are the inputs and what are the expected outputs. Let's now add a new test that changes the input and the expected output.

require "rspec/autorun"

class Thermostat
  def initialize(thermometer:,furnace:)
    @thermometer = thermometer
    @furnace     = furnace
  end

  def check_temperature
    temp = @thermometer.temp_f
    if temp = 67
      @furnace.turn_on
    end
  end
end

RSpec.describe Thermostat do

  it "turns on the furnace when the temperature is too low" do
    thermometer = double(temp_f: 67)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to receive(:turn_on)
    thermostat.check_temperature
  end

  it "leaves the furnace when the temperature is comfortable" do
    thermometer = double(temp_f: 72)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to_not receive(:turn_on)
    thermostat.check_temperature
  end

end

# >> .F
# >>
# >> Failures:
# >>
# >>   1) Thermostat leaves the furnace when the temperature is comfortable
# >>      Failure/Error: $SiB.record_result(12, (@furnace.turn_on))
# >>        (Double).turn_on(no args)
# >>            expected: 0 times with any arguments
# >>            received: 1 time
# >>      # xmptmp-in2122l_g.rb:12:in `check_temperature'
# >>      # xmptmp-in2122l_g.rb:34:in `block (2 levels) in <main>'
# >>
# >> Finished in 0.00522 seconds (files took 0.09003 seconds to load)
# >> 2 examples, 1 failure
# >>
# >> Failed examples:
# >>
# >> rspec  # Thermostat leaves the furnace when the temperature is comfortable
# >>

This test fails. The furnace turns on even when the temperature is 72 degrees Fahrenheit. When we take a look at the method being tested, we can see why: we accidentally put an assignment where we meant to put a less-than-or-equal comparison.

Once we fix this, both examples succeed.

def check_temperature
  temp = @thermometer.temp_f
  if temp = 67
    @furnace.turn_on
  end
end
require "rspec/autorun"

class Thermostat
  def initialize(thermometer:,furnace:)
    @thermometer = thermometer
    @furnace     = furnace
  end

  def check_temperature
    temp = @thermometer.temp_f
    if temp <= 67
      @furnace.turn_on
    end
  end
end

RSpec.describe Thermostat do

  it "turns on the furnace when the temperature is too low" do
    thermometer = double(temp_f: 67)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to receive(:turn_on)
    thermostat.check_temperature
  end

  it "leaves the furnace when the temperature is comfortable" do
    thermometer = double(temp_f: 72)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to_not receive(:turn_on)
    thermostat.check_temperature
  end

end

# >> ..
# >>
# >> Finished in 0.00486 seconds (files took 0.07132 seconds to load)
# >> 2 examples, 0 failures
# >>

Now let's move on to another test. It turns out that the furnace might fail to turn on in some cases. For instance, it might be manually disabled. The #turn_on method on furnace objects will return true if activating the furnace was successful, and false if it failed. As a new requirement, we want the #check_temperature method to raise an exception when the furnace fails to light.

We temporarily disable all other examples, and add a new one to test this exception-raising behavior. This test is mostly a duplicate of the others, but this time we specify a return value of false from the #turn_on method, and we place an expectation that the method under test will raise an exception.

require "rspec/autorun"

class Thermostat
  def initialize(thermometer:,furnace:)
    @thermometer = thermometer
    @furnace     = furnace
  end

  def check_temperature
    temp = @thermometer.temp_f
    if temp <= 67
      @furnace.turn_on
    end
  end
end

RSpec.describe Thermostat do

  xit "turns on the furnace when the temperature is too low" do
    thermometer = double(temp_f: 67)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to receive(:turn_on)
    thermostat.check_temperature
  end

  xit "leaves the furnace when the temperature is comfortable" do
    thermometer = double(temp_f: 72)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to_not receive(:turn_on)
    thermostat.check_temperature
  end

  it "raises an exception when the furnace fails to light" do
    thermometer = double(temp_f: 67)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to receive(:turn_on).and_return(false)
    expect { thermostat.check_temperature }.to raise_error
  end

end

# >> **F
# >>
# >> Pending: (Failures listed here are expected and do not affect your suite...
# >>
# >>   1) Thermostat turns on the furnace when the temperature is too low
# >>      # Temporarily skipped with xit
# >>      # xmptmp-in2122AV0.rb:19
# >>
# >>   2) Thermostat leaves the furnace when the temperature is comfortable
# >>      # Temporarily skipped with xit
# >>      # xmptmp-in2122AV0.rb:28
# >>
# >> Failures:
# >>
# >>   1) Thermostat raises an exception when the furnace fails to light
# >>      Failure/Error: $SiB.record_result(43, (expect { thermostat.check_te...
# >>        expected Exception but nothing was raised
# >>      # xmptmp-in2122AV0.rb:43:in `block (2 levels) in <main>'
# >>
# >> Finished in 0.00582 seconds (files took 0.07806 seconds to load)
# >> 3 examples, 1 failure, 2 pending
# >>
# >> Failed examples:
# >>
# >> rspec  # Thermostat raises an exception when the furnace fails to light
# >>

This fails, since we haven't yet added the behavior being tests. Let's go ahead and add it now. We use the control-flow or that we learned about in episode #125, and the fail method we learned about in episode #188.

require "rspec/autorun"

class Thermostat
  def initialize(thermometer:,furnace:)
    @thermometer = thermometer
    @furnace     = furnace
  end

  def check_temperature
    temp = @thermometer.temp_f
    if temp <= 67
      @furnace.turn_on or fail "Furnace could not be lit"
    end
  end
end

RSpec.describe Thermostat do

  xit "turns on the furnace when the temperature is too low" do
    thermometer = double(temp_f: 67)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to receive(:turn_on)
    thermostat.check_temperature
  end

  xit "leaves the furnace when the temperature is comfortable" do
    thermometer = double(temp_f: 72)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to_not receive(:turn_on)
    thermostat.check_temperature
  end

  it "raises an exception when the furnace fails to light" do
    thermometer = double(temp_f: 67)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to receive(:turn_on).and_return(false)
    expect { thermostat.check_temperature }.to raise_error
  end

end

# >> **.
# >>
# >> Pending: (Failures listed here are expected and do not affect your suite...
# >>
# >>   1) Thermostat turns on the furnace when the temperature is too low
# >>      # Temporarily skipped with xit
# >>      # xmptmp-in2122Z2h.rb:19
# >>
# >>   2) Thermostat leaves the furnace when the temperature is comfortable
# >>      # Temporarily skipped with xit
# >>      # xmptmp-in2122Z2h.rb:28
# >>
# >> Finished in 0.00529 seconds (files took 0.07257 seconds to load)
# >> 3 examples, 0 failures, 2 pending
# >>

Our new test now passes. But is it written in an ideal way? I don't think it is. Once again, we are asserting more than we actually stated in the spec string. The important thing here is what happens when the Furnace#turn_on message returns false, but we are also testing that the message is sent, something we already confirmed in another test. Every time we assert more than we set out to test in a unit test, we set ourselves up for more test maintenance when things change. Let's change from using the furnace double as a mock, to using it as a /stub.

require "rspec/autorun"

class Thermostat
  def initialize(thermometer:,furnace:)
    @thermometer = thermometer
    @furnace     = furnace
  end

  def check_temperature
    temp = @thermometer.temp_f
    if temp <= 67
      @furnace.turn_on or fail "Furnace could not be lit"
    end
  end
end

RSpec.describe Thermostat do

  xit "turns on the furnace when the temperature is too low" do
    thermometer = double(temp_f: 67)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to receive(:turn_on)
    thermostat.check_temperature
  end

  xit "leaves the furnace when the temperature is comfortable" do
    thermometer = double(temp_f: 72)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to_not receive(:turn_on)
    thermostat.check_temperature
  end

  it "raises an exception when the furnace fails to light" do
    thermometer = double(temp_f: 67)
    furnace     = double(turn_on: false)
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect { thermostat.check_temperature }.to raise_error
  end

end

# >> **.
# >>
# >> Pending: (Failures listed here are expected and do not affect your suite...
# >>
# >>   1) Thermostat turns on the furnace when the temperature is too low
# >>      # Temporarily skipped with xit
# >>      # xmptmp-in2122MzP.rb:19
# >>
# >>   2) Thermostat leaves the furnace when the temperature is comfortable
# >>      # Temporarily skipped with xit
# >>      # xmptmp-in2122MzP.rb:28
# >>
# >> Finished in 0.00117 seconds (files took 0.07764 seconds to load)
# >> 3 examples, 0 failures, 2 pending
# >>

This version gets at the heart of the aspect we wanted to test, without any extra effort.

Now that we have this test in good shape, let's re-enable our original tests, and make sure they still pass.

require "rspec/autorun"

class Thermostat
  def initialize(thermometer:,furnace:)
    @thermometer = thermometer
    @furnace     = furnace
  end

  def check_temperature
    temp = @thermometer.temp_f
    if temp <= 67
      @furnace.turn_on or fail "Furnace could not be lit"
    end
  end
end

RSpec.describe Thermostat do

  it "turns on the furnace when the temperature is too low" do
    thermometer = double(temp_f: 67)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to receive(:turn_on)
    thermostat.check_temperature
  end

  it "leaves the furnace when the temperature is comfortable" do
    thermometer = double(temp_f: 72)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to_not receive(:turn_on)
    thermostat.check_temperature
  end

  it "raises an exception when the furnace fails to light" do
    thermometer = double(temp_f: 67)
    furnace     = double(turn_on: false)
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect { thermostat.check_temperature }.to raise_error
  end

end

# >> F..
# >>
# >> Failures:
# >>
# >>   1) Thermostat turns on the furnace when the temperature is too low
# >>      Failure/Error: $SiB.record_result(12, (@furnace.turn_on or fail "Fu...
# >>      RuntimeError:
# >>        Furnace could not be lit
# >>      # xmptmp-in2122zRi.rb:12:in `check_temperature'
# >>      # xmptmp-in2122zRi.rb:25:in `block (2 levels) in <main>'
# >>
# >> Finished in 0.00569 seconds (files took 0.07032 seconds to load)
# >> 3 examples, 1 failure
# >>
# >> Failed examples:
# >>
# >> rspec  # Thermostat turns on the furnace when the temperature is too low
# >>

Uh oh, looks like we've broken one of them. How? Well, this is the one that tests that the furnace turns on when the temperature is too low. We've set up an expectation that the furnace will be sent the #turn_on message. By default, RSpec will return nil from this mocked method. But we are now treating falsey returns from #turn_on as errors, and raising an exception in response.

It seems like what we'll have to do now is add an and_return to the end of that expectation, just to keep the test happy.

require "rspec/autorun"

class Thermostat
  def initialize(thermometer:,furnace:)
    @thermometer = thermometer
    @furnace     = furnace
  end

  def check_temperature
    temp = @thermometer.temp_f
    if temp <= 67
      @furnace.turn_on or fail "Furnace could not be lit"
    end
  end
end

RSpec.describe Thermostat do

  it "turns on the furnace when the temperature is too low" do
    thermometer = double(temp_f: 67)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to receive(:turn_on).and_return(true)
    thermostat.check_temperature
  end

  it "leaves the furnace when the temperature is comfortable" do
    thermometer = double(temp_f: 72)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to_not receive(:turn_on)
    thermostat.check_temperature
  end

  it "raises an exception when the furnace fails to light" do
    thermometer = double(temp_f: 67)
    furnace     = double(turn_on: false)
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect { thermostat.check_temperature }.to raise_error
  end

end

# >> ...
# >>
# >> Finished in 0.00599 seconds (files took 0.08221 seconds to load)
# >> 3 examples, 0 failures
# >>

But I don't like this version. Once again, we have an expectation that also has a return value, which was the smell that kicked off this episode. And from the perspective of writing tests that communicate intent, this is just noise. The single most important part of this example is the assertion that the furnace will be turned on. Tacking on an and_return just distracts from that assertion.

It turns out, though, that we don't have to do this. Instead of putting the return value on the line that sets the message expectation, we can instead add it to the setup of theĀ furnace double. RSpec will use this return value by default, while still enabling us to assert that the message was, in fact, sent.

require "rspec/autorun"

class Thermostat
  def initialize(thermometer:,furnace:)
    @thermometer = thermometer
    @furnace     = furnace
  end

  def check_temperature
    temp = @thermometer.temp_f
    if temp <= 67
      @furnace.turn_on or fail "Furnace could not be lit"
    end
  end
end

RSpec.describe Thermostat do

  it "turns on the furnace when the temperature is too low" do
    thermometer = double(temp_f: 67)
    furnace     = double(turn_on: true)
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to receive(:turn_on)
    thermostat.check_temperature
  end

  it "leaves the furnace when the temperature is comfortable" do
    thermometer = double(temp_f: 72)
    furnace     = double
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect(furnace).to_not receive(:turn_on)
    thermostat.check_temperature
  end

  it "raises an exception when the furnace fails to light" do
    thermometer = double(temp_f: 67)
    furnace     = double(turn_on: false)
    thermostat  = Thermostat.new(thermometer: thermometer, furnace: furnace)

    expect { thermostat.check_temperature }.to raise_error
  end

end

# >> ...
# >>
# >> Finished in 0.01013 seconds (files took 0.09661 seconds to load)
# >> 3 examples, 0 failures
# >>

I like this version a lot better. We've kept the important part of the test unobstructed by other considerations. And overall, we've minimized the number of mocked methods.

Mocking methods is a powerful tool for driving design using tests. But it's very easy to overuse mocks. It's easy to build a false sense of security with tests that simultaneously assert too much, and yet fail to check the most important aspects of the code under test.

In general, I would suggest taking a very careful look at any RSpec examples that set more than a single message expectation. It occasionally makes sense to assert that a series of messages are sent in a row. But more often, multiple message expectations in a single test are indicative of a test that is asserting too much at once, and is bound too tightly to the implementation of the object under teset. Which may, in turn, obscure the fact that it is also failing to test things that really matter. One message expectation per test, at most, is a good rule of thumb.

I also recommend taking a hard look at any scenario where a message expectation also contains a return value. Usually this indicates a situation where either a stub would be more appropriate, or one where the return value is there just to keep the code from blowing up.

And that's it for now. Happy hacking!

Responses