OmbuLabs Blog

The Lean Software Boutique

DRY your tests

I'm a big fan of having small classes. I'm not a big fan of having huge specs for a small class/object. Every time I see an opportunity to DRY my specs, I take it.

Today I wrote a spec to make sure that we gracefully ignore SPAMmy contact requests in the Ombu Labs contact page. It initially looked like this:

test "gracefully ignores spammy requests with valid attributes" do
  @valid_contact = contacts(:two)
  attributes = @valid_contact.attributes
                             .merge(email_confirmation: @valid_contact.email)

  assert_no_difference("Contact.count") do
    post :create, contact: attributes, format: 'js'
  end

  assert_response :success
end

The new behavior adds a simple SPAM trap field that bots will usually fall for. If a bot is submitting the email_confirmation field (which is hidden by a CSS class), then it is SPAM and it gracefully ignores the request.

The test only tests the scenario where the bot is performing an AJAX request. Then I thought that SPAM bots might try to submit a non-AJAX html request.

So I wrote some more:

test "gracefully ignores spammy requests with valid attributes (AJAX)" do
  @valid_contact = contacts(:two)
  attributes = @valid_contact.attributes
  attributes.merge!(email_confirmation: @valid_contact.email)

  assert_no_difference("Contact.count") do
    post :create, contact: attributes, format: 'js'
  end

  assert_response :success
end

test "gracefully ignores spammy requests with valid attributes (HTML)" do
  @valid_contact = contacts(:two)
  attributes = @valid_contact.attributes
  attributes.merge!(email_confirmation: @valid_contact.email)

  assert_no_difference("Contact.count") do
    post :create, contact: attributes, format: 'html'
  end

  assert_response :success
end

Now that is ridiculous, why should I copy/paste this? There is only one parameter (format) that varies between them.

So, I refactored the code to look like this:

["js", "html"].each do |format|
  test "gracefully ignores spammy requests with valid attributes" do
    @valid_contact = contacts(:two)
    attributes = @valid_contact.attributes
    attributes.merge!(email_confirmation: @valid_contact.email)

    assert_no_difference("Contact.count") do
      post :create, contact: attributes, format: format
    end

    assert_response :success
  end
end

The code above doesn't really work. It raises this exception:

/Users/etagwerker/.rvm/gems/ruby-2.2.1@ombulabs-com/gems/activesupport-3.2.17/lib/active_support/testing/declarative.rb:28:in `test': test_should_gracefully_ignore_spammy_requests_with_valid_attributes is already defined in ContactsControllerTest (RuntimeError)
    from /Users/etagwerker/Projects/ombulabs.com/test/functional/contacts_controller_test.rb:29:in `block in <class:ContactsControllerTest>'
    from /Users/etagwerker/Projects/ombulabs.com/test/functional/contacts_controller_test.rb:28:in `each'
    from /Users/etagwerker/Projects/ombulabs.com/test/functional/contacts_controller_test.rb:28:in `<class:ContactsControllerTest>'
    from /Users/etagwerker/Projects/ombulabs.com/test/functional/contacts_controller_test.rb:3:in `<top (required)>'
    from /Users/etagwerker/.rvm/gems/ruby-2.2.1@ombulabs-com/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:10:in `require'
    from /Users/etagwerker/.rvm/gems/ruby-2.2.1@ombulabs-com/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:10:in `block (2 levels) in <main>'
    from /Users/etagwerker/.rvm/gems/ruby-2.2.1@ombulabs-com/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:9:in `each'
    from /Users/etagwerker/.rvm/gems/ruby-2.2.1@ombulabs-com/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:9:in `block in <main>'
    from /Users/etagwerker/.rvm/gems/ruby-2.2.1@ombulabs-com/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:4:in `select'
    from /Users/etagwerker/.rvm/gems/ruby-2.2.1@ombulabs-com/gems/rake-10.5.0/lib/rake/rake_test_loader.rb:4:in `<main>'
Errors running test:functionals! #<RuntimeError: Command failed with status (1): [ruby -I"lib:test" -I"/Users/etagwerker/.rvm/gems/ruby-2.2.1@ombulabs-com/gems/rake-10.5.0/lib" "/Users/etagwerker/.rvm/gems/ruby-2.2.1@ombulabs-com/gems/rake-10.5.0/lib/rake/rake_test_loader.rb" "test/functional/**/*_test.rb" ]>

Basically test-unit doesn't want me to define a test with the same description more than once.

So, I decided to interpolate the format variable in the test description:

["js", "html"].each do |format|
  test "gracefully ignores spammy requests in #{format} format" do
    @valid_contact = contacts(:two)
    attributes = @valid_contact.attributes
    attributes.merge!(email_confirmation: @valid_contact.email)

    assert_no_difference("Contact.count") do
      post :create, contact: attributes, format: format
    end

    assert_response :success
  end
end

Test Unit is happy with this, the tests pass and my spec code is as concise as possible.

If you prefer RSpec, it would look like this:

["js", "html"].each do |format|
  it "gracefully ignores spammy requests with valid attributes" do
    @valid_contact = contacts(:two)
    attributes = @valid_contact.attributes
    attributes.merge!(email_confirmation: @valid_contact.email)

    expect do
      post :create, contact: attributes, format: format
    end.not_to change(Contact, :count)

    expect(response).to be_success
  end
end

RSpec is a little smarter than Test Unit and it doesn't require you to interpolate a variable (format) in the description.

Either way, always look for ways to keep your tests as DRY as your classes. It will improve maintenance in the long run.