Brittle Tests

Tammer Saleh

A friend and I were talking about the scaffold mailer tests that are generated by rails, and it got me on a bit of a rant. Here’s what you start out with:

class MailerTest < Test::Unit::TestCase
  # ...stuff...
  def setup
    # ...more stuff...
    @expected = TMail::Mail.new
    @expected.set_content_type "text", "plain", { "charset" => CHARSET }
    @expected.mime_version = '1.0'
  end

  def test_invitation
    @expected.subject = 'Some subject'
    @expected.body    = read_fixture('invitation')

    assert_equal @expected.encoded, Mailer.create_invitation(args).encoded
  end

  private
  # ...even more stuff for reading the fixture and doing the encoding...
end

completely pointless image

Basically, you’re generating the email, and then comparing it to an exact copy in your fixtures directory. Not only will this fail whenever you’ve got anything interesting like the current time in body of your mail, but it’s also just a super brittle way of doing things.

Would you write a controller test that compared the rendered view to a pre-generated version in your fixtures directory. Any changes made by the designer, any typos found, anything at all would have to also be made to that fixture.

A much better way of testing emails is like such:

class MailerTest < Test::Unit::TestCase
  def setup
    # ActionMailer settings
  end

  def test_invitation
    user = User.find_first
    email = Mailer.create_invitation(user)

    assert_equal "admin@politiquotes.com", email.from.first
    assert email.to.include?(user.email)
    assert_match(/welcome to politiquotes/i, email.subject)
    assert_match(/please follow the link below/i, email.body)
    assert_match('http://politiquotes.com/tour', email.body)
  end
end

another pointless image

Test only those things which are necessary. You should not be testing that there are two spaces in between each sentence, but you should be testing that you have included the tour url.

This is a great rule to follow in all testing scenarios. Don’t test that the rendered view has a div with class “person” (unless your rjs depends on it), but do test that the rendered form will post to /users. Don’t test that your flash has a :notice key with “Permission denied! Please login as an administrator and try again!”, but do test that any flash key matches against /denied/. Always strive to make your tests more focused and less brittle. I guarantee, your coworkers will thank you for it.

Update: Looks like I’m not the first person to notice the ActionMailer default tests. In fact, I guess this argument is made in the Agile Web Development with Rails book (page 420-421).