Finding and Fixing Frustrating Tests

Previously we talked about several ways tests could fail indeterminately. Today we'll look at three ways Ruby's flexibility can lead to bizarre test behavior, and some really simple solutions.

Method Clobbering

I have to admit, when I write a new test I often just copy a similar one. This is of course expedient, but it comes with some risk. Pretend we're testing our new Fruit class. The basic rule appears to be that the presence of an a indicates whether a fruit is delicious:

class Fruit
  attr_reader :name

  def initialize(name)
    @name = name
  end

  def delicious?
    name =~ /a/i
  end
end

We write a few tests to exercise our Fruit class:

class FruitTest < Minitest::Test
  def test_fruit_containing_an_a_are_delicious
    fruit = Fruit.new("Apple")

    assert fruit.delicious?
  end

  def test_fruit_without_an_a_are_not_delicious
    fruit = Fruit.new("Plum")

    assert !fruit.delicious?
  end

  # ... dozens of very insightful tests below

end

Now it turns out that persimmons don't contain an a, but are in fact delicious. Our domain expert has explained that it's the double m which makes them so appealing. The required change is so simple it's hardly worth us even typing it out:

class Fruit
  #...

  def delicious?
    name =~ /a|m/i
  end
end

Likewise, the test case couldn't be easier, we just grab one our existing tests and paste it below...

class FruitTest < Minitest::Test
  def test_fruit_containing_an_a_are_delicious
    fruit = Fruit.new("Apple")

    assert fruit.delicious?
  end

  def test_fruit_without_an_a_are_not_delicious
    fruit = Fruit.new("Plum")

    assert !fruit.delicious?
  end

  def test_fruit_without_an_a_are_not_delicious
    fruit = Fruit.new("Persimmon")

    assert fruit.delicious?
  end
end

We run the tests, and everything passes! Except there's a problem. After committing our code, we noticed that plums have suddenly become delicious. This is a serious problem. Looking at our tests, it's obvious that plums aren't delicious:

def test_fruit_without_an_a_are_not_delicious
  fruit = Fruit.new("Plum")

  assert !fruit.delicious?
end

Two things went wrong. First, the rule was supposed to be: /a|mm/i. Fix that, and all is well, except our test suite never caught the error.

In my haste to prove myself, I accidentally defined two methods with the same name, test_fruit_without_an_a_are_not_delicious. This is perfectly legal in Ruby, but perfectly terrible in your tests. The second declaration redefined the first. As you tests grow, it becomes easier to slip up and not notice something so minor.

Fear not, there are a few handy things you can do to prevent this. First, let's do some quick and dirty checks on our code. We can use grep to see if there are any obvious method redefinitions in our tests:

$ grep "def test_" -r --include "*_test.rb" test | sort | uniq --count --repeated
  2 test/fruit_test.rb:  def test_fruit_without_an_a_are_not_delicious
  2 test/basket_test.rb: def test_purchase_subtotals

If you're not in the habit of muttering the words sed, awk, grep, sort whenever you're trying to figure something out, here's a quick explanation. First we used grep to recursively search our test directory for files containing def test_. We sort that, and the print out a --count of --repeated lines. This provides a list of any duplicated test cases.

If you use ActiveSupport, there's an easy remedy for this problem. ActiveSupport::TestCase defines a method called test:

test "fruit without an a are not delicious"
  fruit = Fruit.new("Plum")

  assert !fruit.delicious?
end

Defining the same test twice will raise an exception, and as an added benefit, it's a bit more readable.

Setup For Failure

What if your tests only pass when run as part of your whole test suite, or perhaps only pass when run in isolation? This often means that tests are interacting with each other. Previously we covered some ways that tests could modify state, but what about tests that modify other tests?

Just as you can easily redefine tests, it's also possible to redefine an existing test class. If you're lazy like me, and duplicate an existing test file, you might end up with something like this:

# signup_gate_test.rb
class SignupGateTest < Minitest::Test
  def setup
    feature(:signup).disable!
  end

  def teardown
    feature(:signup).enable!
  end

  #...
end

# account_controller_test.rb
class SignupGateTest < Minitest::Test
  test "users can sign up"
    get :new
    assert_status :ok
  end
end

Embarrassing as it is, each test passes when run in isolation, but when run as part the test suite, it fails saying that signups are disabled. The first file to load will define SignupGateTest, the second will open SignupGateTest and add or redefine methods. In this example, the setup method causes account_controller_test.rb to fail.

Yet again, there's an easy way to find these tests:

$ grep -h -E 'class [A-Za-z]+Test ' -r --include "*_test.rb" test |\
    sed -e 's/^[ \t]*//g' | sort | uniq --count --repeated
      2 class SignupGateTest < ActiveSupport::TestCase
      2 class ImageHelperTest < ActionView::TestCase

This is similar to our previous bit of shell magic. We search for all the lines matching class [A-Za-z]+Test, strip any excess whitespace with sed, and then count any repeated lines.

Fixing these tests is usually pretty straightforward. Simply start by renaming any duplicated test classes to match their file names. Then run the tests independently, and as part of your test suite.

Global Concerns

One last way to drive yourself mad is to let things slip into the global context. This isn't specific to tests, but I find myself making this mistake more often when testing. Consider the following:

require_relative '../test_helper'

include MailTestHelper

class IntegrationMailerTest < Minitest::Test
  #...
end

This seems pretty innocuous, we require some common test helpers, then include the MailTestHelper for these tests. Of course, we didn't just include MailTestHelper in IntegrationMailerTest, we just included it into Kernel. In the worst case, this might redefine some methods you intend to test in other classes. A more benign possibility is that other tests will start to rely on MailTestHelper being available, and will fail when run in isolation.

Luckily assuming your code is reasonably formatted, there's a quick way to track these down:

$ grep -E '^include' -r --include "*.rb" test
test/integration_mailer_test.rb:include MailTestHelper

Similarly, defining a constant in the global scope of a test file can also lead to similar fragility, ie:

require_relative '../test_helper'

TEST_DOMAIN = "example.com"

class IntegrationMailerTest < Minitest::Test
  #...
end

In this case, TEST_DOMAIN is now available in every other test, and should it get used in another file, hilarity will ensue. It's also possible to accidentally redefine constants this way, though Ruby will print warnings unless you've been excessively clever. Hunt these down with:

$ grep -E '^[A-Z]+[a-zA-Z_]+\s*\=' -r --include "*.rb" test
test/integration_mailer_test.rb:TEST_DOMAIN = "example.com"

Both scenarios are easy to fix, just pull those lines down into your test class. There you go, sanity at last.

Recap

There are many ways to introduce subtle and frustrating behavior into your test suite, but a judicious use of grep, sort, and uniq will help you reveal them. These are of course just heuristics, if you want more rigorous analysis, consider using Rubocop to implement some custom rules.

That's all for now, I wish you many passing tests.

blog comments powered by Disqus
Monkey Small Crow Small