Categories
Agile Rails Ruby

DRYing out model specs

[Updated 14/3/07: corrected specify_attributes as per Paul’s comment]
[Updated 18/12/07: modified to avoid crazy RSpec errors]

A week or so ago I wrote about writing specs for simple pieces of functionality (particularly those that are arguably just configuration, like Rails validations). I argued that it’s important to test-drive even the simple things – however, the amount of test code can get out of hand.

Here’s a spec for the single validation from the previous post, rewritten using one expectation per example:

def valid_attrs
  {
    :username => 'fred'
  }
end

describe "A user without a username" do
  setup do
    @user = User.new
    @user.attributes = valid_user_attributes.except(:username)
  end
 
  it "should be invalid" do
    @user.should_not_be_valid
  end
 
  it "should have an error on the username attribute" do
    @user.errors.on(:username).size.should == 1
  end
end

And here’s the massive amount of code that satisfies that spec:

class User 

Clearly once you have a few more attributes you end up with a lot of test code, much of which is repetitive.

I decided to try to create a helper method (see below) to reduce the clutter. Here's how to use it to specify that:

  • The username, first_name and last_name attributes are mandatory
  • The age attribute must be numeric
  • The email attribute must be at least three characters long
  • The username, first_name, last_name and email attributes must be no more than 12, 20, 20 and 200 characters long respectively
def valid_attrs
  {
    :username => 'fred',
    :first_name => 'Fred',
    :last_name => 'Bloggs',
    :email => 'fred@bloggs.com',
    :age => 21
  }
end

specify_attributes User, valid_attrs,
{
  :mandatory => [:username, :first_name, :last_name],
  :numeric => [:age],
  :min_lengths => {:email => 3},
  :max_lengths => {:username => 12, :first_name => 20, :last_name => 20, :email => 200}
}

This automatically executes the following specs:

A User with all attributes set
- should be valid

A User with no username
- should be invalid
- should not allow saving
- should cause exactly one error when save attempted
- should set an error on the username field

A User with no first_name
- should be invalid
- should not allow saving
- should cause exactly one error when save attempted
- should set an error on the first_name field

A User with no last_name
- should be invalid
- should not allow saving
- should cause exactly one error when save attempted
- should set an error on the last_name field

A User with non-numeric age
- should be invalid
- should not allow saving
- should cause exactly one error when save attempted
- should set an error on the age field

A User with email of length 3
- should be valid

A User with email of length 2
- should be invalid
- should not allow saving
- should cause exactly one error when save attempted
- should set an error on the email field

A User with last_name of length 20
- should be valid

A User with last_name of length 21
- should be invalid
- should not allow saving
- should cause exactly one error when save attempted
- should set an error on the last_name field

A User with username of length 12
- should be valid

A User with username of length 13
- should be invalid
- should not allow saving
- should cause exactly one error when save attempted
- should set an error on the username field

A User with first_name of length 20
- should be valid

A User with first_name of length 21
- should be invalid
- should not allow saving
- should cause exactly one error when save attempted
- should set an error on the first_name field

A User with email of length 200
- should be valid

A User with email of length 201
- should be invalid
- should not allow saving
- should cause exactly one error when save attempted
- should set an error on the email field

Here's the code that makes it possible (I added it to spec_helper.rb).

First the ubiquitous except Hash extension:

class Hash
  # Usage { :a => 1, :b => 2, :c => 3}.except(:a) -> { :b => 2, :c => 3}
  def except(*keys)
    self.reject { |k,v|
      keys.include? k.to_sym
    }
  end
end

A couple of local helper methods (I factored out should_cause_error_on from the repetitive specs for validation failure):

def string_of_length(length)
  (1..length).collect {'a'}.join
end

def should_cause_error_on(attr)
  it "should be invalid" do
    @obj.should_not be_valid
  end

  it "should not allow saving" do
    @obj.save.should be_false
  end

  it "should cause exactly one error when save attempted" do
    @obj.save
    @obj.errors.size.should == 1
  end

  it "should set an error on the #{attr} field" do
    @obj.errors_on(attr).size.should == 1
  end
end

And finally the actual specify_attributes method:

def specify_attributes(clazz, attr_list, options = {})
  label = "#{clazz =~ /^aeiouy/i ? 'An' : "A"} #{clazz}"
  describe "#{label} with all attributes set", :type => 'model' do
    setup do
      @obj = clazz.new
      @obj.attributes = attr_list
    end

    it "should be valid" do
      @obj.should be_valid
    end
  end

  if options[:mandatory]
    options[:mandatory].each do |attr|

      describe "#{label} with no #{attr}", :type => 'model' do
        setup do
          @obj = clazz.new
          @obj.attributes = valid_attrs.except attr
        end
        
        should_cause_error_on attr
      end
    end
  end

  if options[:numeric]
    options[:numeric].each do |attr|
      describe "#{label} with non-numeric #{attr}", :type => 'model' do
        setup do
          @obj = clazz.new
          @obj.attributes = valid_attrs.except attr
          eval("@obj.#{attr} = string_of_length valid_attrs[attr].to_s.size")
        end

        should_cause_error_on attr
      end
    end
  end
  
  if options[:min_lengths]
    options[:min_lengths].keys.each do |attr|
      limit = options[:min_lengths][attr]

      describe "#{label} with #{attr} of length #{limit}", :type => 'model' do
        setup do
          @obj = clazz.new
          @obj.attributes = valid_attrs.except attr
          eval("@obj.#{attr} = string_of_length limit")
        end

        it "should be valid" do
          @obj.should be_valid
        end
      end

      describe "#{label} with #{attr} of length #{limit - 1}", :type => 'model' do
        setup do
          @obj = clazz.new
          @obj.attributes = valid_attrs.except attr
          eval("@obj.#{attr} = string_of_length(limit - 1)")
        end

        should_cause_error_on attr
      end
    end
  end

  if options[:max_lengths]
    options[:max_lengths].keys.each do |attr|
      limit = options[:max_lengths][attr]

      describe "#{label} with #{attr} of length #{limit}", :type => 'model' do
        setup do
          @obj = clazz.new
          @obj.attributes = valid_attrs.except attr
          eval("@obj.#{attr} = string_of_length limit")
        end

        it "should be valid" do
          @obj.should be_valid
        end
      end

      describe "#{label} with #{attr} of length #{limit + 1}", :type => 'model' do
        setup do
          @obj = clazz.new
          @obj.attributes = valid_attrs.except attr
          eval("@obj.#{attr} = string_of_length(limit + 1)")
        end

        should_cause_error_on attr
      end
    end
  end
end

It ought to be possible to extend this to other validations too, as well as the various options like :message, :on, :allow_nil and so on, but that's for another day.

[tags]tdd, bdd, agile, rails, rspec, dry[/tags]

5 replies on “DRYing out model specs”

You’ve got an incorrectly named variable in the specify_attributes method: you pass the valid attributes in as attr_list but within the method you refer to valid_attrs.

(The example as it stands it works as externally you’ve got a method defined called valid_attr which contains the valid attributes)

You’ve actually touched upon a controversial issue. Should my specs be DRY or should they be as expressive as possible? The general rule of thumb among the testing community seems to be in favor of expressive as possible.

First off, I appreciate the amount of work you put into this and it seems to be a very powerful helper method. So congrats on that.

However, there are a few things that do worry me about this code. First, the actual ‘spec’ you end up writing is hardly more expressive than the actual implementation code: not that it necessarily has to be, but it does sort of defeat the purpose of spec’ing in the first place.

Second, you’ve initiated a point of failure in the spec, where you spec may fail, not because the implementation of your applications code is wrong, but because there’s a bug in the helper method.

Third, you now have a rather large amount of untested code, unless you proceed to write test code for the code that generates the code for your test… head spinning…

All that aside, this is a very ingenious little helper method, so good job on that front.

Chris, some very good points, most of which I was aware of and all of which I fully agree with.

At the time, I decided that the trade-off between the downsides and having to write potentially hundreds of lines of test code for a single line in the model was worth it. I now think there’s a better way to do this, and I’m going to try re-implementing the helper as a generator, so the test code actually exists as a skeleton and can be changed afterwards.

Watch this space.

Leave a Reply