[go: up one dir, main page]
More Web Proxy on the site http://driver.im/

My top 7 RSpec best practices

Posted by Dmytro Shteflyuk on under Ruby & Rails

I use RSpec in all my projects. It’s really hard to overemphasize how helpful it is and how much easier becomes your life if you have good specs coverage. But its outstanding flexibility enables many ways to make your specs awful: horribly slow, over-bloated, even non-readable sometimes. I do not want to teach you BDD and RSpec here, but instead I will give you some ideas how to improve your specs quality and increase efficiency of your BDD workflow.

1. Use before :all block carefully

Sometimes it looks like a good idea to create a test data in before :all block. But be careful — these blocks are not wrapped in a transaction, so the data will not be rolled back after the test. In this case you should clear your data in the after :all block manually.

1
2
3
4
5
6
7
8
9
10
11
12
13
describe Friendship do
  before :all do
    @users = (1..5).collect { Factory(:user) }
  end

  after :all do
    @users.each { |user| user.destroy! }
  end

  it 'should do something' do
    # Something interesting with @users
  end
end

Another option is to move your before :all blocks to before :each to make them rolled back automatically.

2. For each test create exactly what it needs

Fixtures are cool when you start working on a project. But they quickly become painful while project grows: you add a new field to a fixture and break a half of your tests. There are tons of plugins which could simplify test data creation, I personally recommend factory_girl: it’s pretty slick and easy to use.

1
2
3
4
5
6
7
8
Factory.define :user do |f|
  f.sequence(:login) { |n| "user#{n}" }
  f.email { |a| "#{a.login}@example.com" }
  f.description "Ruby on Rails Developer"
end

# Somewhere in specs
@user = Factory(:user, :admin => true)

3. Do not create hundreds of records for a particular spec

Sometimes you want to test a method which operates on large set of records (filtering, trimming, etc). For example, this method returns 50 most popular videos (and no more). The straight approach is to create 51 record and make sure, that the size of the returned array is 50. When I saw a code snippet like this in our project first time, I was surprised. There was a few more pieces sharing this behavior, so here is my advice: add a parameter to the method, which will limit the number of records to return. In this case you can create 3 records, and pass 2 as a parameter.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
describe User do
  it 'should return top users in User.top method' do
    @users = (1..3).collect { Factory(:user) }
    top_users = User.top(2).all
    top_users.should have(2).entries
  end
end

class User < ActiveRecord::Base
  # Select N top users. Returns 10 entries when called without arguments.
  #   User.top.all.size    # => 10
  #   User.top(2).all.size # => 2
  #
  named_scope :top, lambda { |*args| { :limit => (args.size > 0 ? args[0] : 10) } }
end

4. Do not over-mock

Mocking is interesting and sometimes very useful technology. You may mock just everything so you spec will not hit the database. But there is a catch: your model code may be changed some day causing callers to break. Since you mock everything, you will never get failing specs. So now you should update all your mocks to fit a new interface. Also you would not be able to find SQL queries errors if you have mocked them. Instead of this I use integration approach: controller should talk to models, which have to hit the database. Real database with real data (OK, not so real). The practice 2 can help you in test data creation.

Bad:

1
2
3
4
5
6
7
8
9
10
11
12
describe VideosController do
  describe '.create action' do
    it 'should assign top videos' do
      params = { :title => 'new video', :description => 'video description' }
      @video = mock_model(Video)
      Video.should_receive(:new).and_return(@video)
      @video.should_receive(:update_attributes).with(params).and_return(true)
      get :index, :video => params
      assigns[:video].should be(@video)
    end
  end
end

Good:

1
2
3
4
5
6
7
8
9
10
11
describe VideosController do
  describe '.create action' do
    it 'should assign top videos' do
      params = { :title => 'new video', :description => 'video description' }
      get :index, :video => params
      assigns[:video].should_not be_new_record
      assigns[:video].title.should == params[:title]
      assigns[:video].description.should == params[:description]
    end
  end
end

But you can use mocks to skip records retrieving from the database (make sure you have specs covering corresponding model code). Let me explain this. For example, you need to render 20 entries in an RSS feed. You could create 21 record in the database using a factory, and then ensure only 20 of them were retrieved, or you could mock your finder method and check its parameter. You may not like magic numbers like 20 in this particular case, and this is a good point. Just move this magic number to the config and ensure it was used to do the retrieval.

1
2
3
4
5
6
7
8
9
10
describe VideosController do
  describe '.index action' do
    it 'should assign top videos' do
      @videos = [mock_model(Video), mock_model(Video)]
      Video.should_receive(:top).with(50).and_return(@videos)
      get :index
      assigns[:top_videos].should be(@videos)
    end
  end
end

5. Use contexts

RSpec spec specifies how particular code should work. Usually, in the beginning you tell what you are going to describe in this spec, and inside describe block you specify what the code should do:

1
2
3
4
5
describe Video do
  it 'should return 5 records in Video.top method' do
    Video.top.should have(5).items
  end
end

Usually you have more than one it block for each method. To group related specs I recommend to use nested describe blocks. Since describe is aliased to context when placed inside another describe, I think it’s a good idea to use it for specs grouping. Each context may have its own before and after blocks (in this case parent blocks will be called right before child ones).

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
describe Video do
  describe '.top' do
    it 'should return 5 records' do
      Video.top.should have(5).items
    end
  end

  context 'when just created' do
    before :each do
      @video = Video.new
    end
   
    # ...
  end
end

6. Create several test suites to speed up your workflow

There are many things you can do to make your BDD more efficient. We will take a look at two of them: creating a separate test suites and running recently modified specs.

There are several standard test suites configured in RSpec by default: spec:controllers, spec:views, spec:helpers, spec:lib. Check the rake -T spec output to get a list of available RSpec tasks. Let’s create a simple Rake tasks generator for spec suites:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
SPEC_SUITES = [
  { :id => :acl, :title => 'access control', :files => %w(spec/controllers/**/acl_spec.rb) },
  { :id => :amazon, :title => 'Amazon libraries', :dirs => %w(spec/lib/amazon) }
]

namespace :spec do
  namespace :suite do
    SPEC_SUITES.each do |suite|
      desc "Run all specs in #{suite[:title]} spec suite"
      Spec::Rake::SpecTask.new(suite[:id]) do |t|
        spec_files = []
        if suite[:files]
          suite[:files].each { |glob| spec_files += Dir[glob] }
        end

        if suite[:dirs]
          suite[:dirs].each { |glob| spec_files += Dir["#{glob}/**/*_spec.rb"] }
        end

        t.spec_opts = ['--options', ""#{Rails.root}/spec/spec.opts""]
        t.spec_files = spec_files
      end
    end
  end
end

Check what tasks are available now:

1
2
3
4
5
~/test$ rake -T spec:suite

(in /Users/kpumuk/test)
rake spec:suite:acl     # Run all specs in access control spec suite
rake spec:suite:amazon  # Run all specs in Amazon libraries spec suite

It was easy! And now let’s take a look at the Rake task for running the recently touched specs (last 10 minutes).

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
# Grab recently touched specs
def recent_specs(touched_since)
  recent_specs = Dir['app/**/*'].map do |path|
    if File.mtime(path) > touched_since
      spec = File.join('spec', File.dirname(path).split("/")[1..-1].join('/'),
      "#{File.basename(path, ".*")}_spec.rb")
      spec if File.exists?(spec)
    end
  end.compact

  recent_specs += Dir['spec/**/*_spec.rb'].select do |path|
    File.mtime(path) > touched_since
  end.uniq
end

namespace :spec do
  desc 'Run all recent specs in spec directory touched in last 10 minutes'
  Spec::Rake::SpecTask.new(:recent) do |t|
    t.spec_opts = ['--options', ""#{RAILS_ROOT}/spec/spec.opts""]
    t.spec_files = recent_specs(Time.now - 10.minutes)
  end
end

And don’t forget to check autospec and watchr gems.

7. Stop spec_helper from being loaded multiple times

Just don’t do that. If you got a big project, there is a chance that the spec_helper will be required in a many different ways: File.expand_path, File.join, etc.,— which results in it being loaded several times and it slows down your test suite!

To avoid this, add the following code at the top of your spec_helper.rb:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
# figure out where we are being loaded from
if $LOADED_FEATURES.grep(/spec\/spec_helper\.rb/).any?
  begin
    raise "foo"
  rescue => e
    puts <<-MSG
  ===================================================
  It looks like spec_helper.rb has been loaded
  multiple times. Normalize the require to:

    require "spec/spec_helper"

  Things like File.join and File.expand_path will
  cause it to be loaded multiple times.

  Loaded this time from:

    #{e.backtrace.join("\n    ")}
  ===================================================
    MSG
  end
end

It will show where you’ve tried to load spec_helper from so you will be able to fix it immediately. Also there is an interesting snippet of code here, which will find and replace all wrong includes.

Conclusion

RSpec is not a silver bullet. You can have 100% coverage and fine-grained specs, but it does not mean your application is completely bug-free. Refactor your specs, increase your programming level, and refactor again. Write specs for any issue that you, your QAs or users have faced. And remember: do not over-mock.

Credits

This is my first article written completely in Google Wave in collaboration with several good Russian rubyists. Thank you all, guys. I want to acknowledge the editorial help of Roman Dmytrenko and Alexey Kovyrin. Robby Russell created a great picture illustrating how sexy is RSpec. And thank you all my readers for your attention.

Did you like this article? You should follow me in Twitter here.

27 Responses to this entry

Subscribe to comments with RSS or TrackBack to 'My top 7 RSpec best practices'.

said on November 25th, 2009 at 19:33 · Permalink · Reply

Nice practices, thanks for an interesting article. I think that’s a kind of knowledge that Rails community needs to share, as we have too much practices that are very dogmatic and unrealistic.

Unfortunately, RSpec has a documentation that tries to enforce mocking and stubbing for all controller specs. Not only it could bloat specs code with mocks/stubs, but also create a sense of false security, so you think your code works when it’s not.

I also consider over-mocking a big evil, especially when it comes to chaining several scopes, or some branching logic in controller, or… well, it messes up with a lot of use cases. I learned that hard way! Ideally I would recommend to use mocks in the following situations:

  1. Stubbing out the external service interaction, unless it’s the whole point of a given test;

  2. Skipping complex activity irrelevant to a given test (like post-processing some data in :after_create block, which is already tested on its own);

  3. Defaulting some complex queries to a manually crafted data (similar to what you described with top videos).

There are always exceptions though. But the good rule of a thumb would be to rely on spec robustness: how likely is that the spec fails when the process it tests no longer works for some reasons? Do mocks help to fail earlier, or delay the time required to react to application errors?

said on November 26th, 2009 at 02:54 · Permalink · Reply

This is a good collection of tips. Over mocking is usually the biggest problem I see in rspec users.

said on November 26th, 2009 at 15:40 · Permalink · Reply

Great rspec tips. Keep them comming!

Ray
said on November 26th, 2009 at 19:29 · Permalink · Reply

Those are good points. I did have some specs so far mocked out that the didn’t produce any relevant failures. Usually I try to cover that with cucumber features.
But I will try your approach on my next controller and see what it leads to.

Ray

Mark Wilden
said on November 26th, 2009 at 19:38 · Permalink · Reply

My approach is this:

  • In unit tests, mock out classes that are not under test.
  • In integration tests, mock as little as possible (e.g., Time.now and expensive external services).
  • Make your controller code so simple that it doesn’t need unit testing.
  • Use Cucumber for integration testing.
said on November 28th, 2009 at 23:29 · Permalink · Reply

I tend to agree with what Mark just said. I prefer very simple controllers, and if I really have to test them I go for mocking then.

said on November 29th, 2009 at 18:48 · Permalink · Reply

With the advent of cucumber, I haven’t been doing as much controller testing with RSpec. It’s kind of hard to test controllers in isolation, anyway.

In a lot of projects I’ve worked on, I’ve seen over-mocking and over-stubbing really trip up a dev team. I definitely recognize that as the biggest issue/anti-pattern.

Great post.

said on November 30th, 2009 at 09:02 · Permalink · Reply

About the spec_helper: why not simply use RSpec’s options file?

I have the following in my Rakefile:

1
2
3
Spec::Rake::SpecTask.new :spec do |t|
  t.spec_opts = ['--options', 'spec/spec.opts']
end

And my spec/spec.opts simply contains

1
2
3
--colour
--diff unified
--require spec/spec_helper
said on November 30th, 2009 at 11:38 · Permalink · Reply

This is a really nice idea. But it does not fit my needs: I love to use command line spec utility to run specs. Anyways, thank you for your suggestion.

said on November 30th, 2009 at 14:35 · Permalink · Reply

If you rather use the spec command, you’d be thrilled to know that it actually defaults to fetch the options from spec/spec.opts (and, of course, you could always explicitely alias spec='spec -O spec/spec.opts'). :)

said on November 30th, 2009 at 14:46 · Permalink · Reply

Okay, you’re right here. But what about Textmate and other editors (or even IDEs like RubyMine) which support “Run Examples” or “Run current Example”? :-)

There is too much work to exclude this single line from specs.

said on November 30th, 2009 at 21:59 · Permalink

Well, there must be a way to configure the text editors and IDEs (I just checked rspec-tmbundle’s Spec::Mate::Runner and it takes arbitrary options everywhere it makes sense to do so, and given that it’s RSpec’s David Chelimsky’s code, I’d be surprised if it didn’t handle the spec/spec.opts case sanely or even defaulted to it).

I don’t really get the ‘there is too much work to exclude this single line from specs’ comment; I simply never put any such line in my specs to begin with (I hate clutter and never understood the popular custom of starting each and every spec with the same noise line, when the same behaviour can be configured in one place).

But I definitely don’t want to evangelise; I just wanted to share my (IMHO, saner and more elegant) solution. Thanks for all the other great tips! :)

said on November 30th, 2009 at 22:23 · Permalink · Reply

BTW, spec command started support spec/spec.opts starting from 1.2.9, the latest version for today.

said on December 2nd, 2009 at 18:20 · Permalink · Reply

rake spec and spec spec return different results on my Rails app. Why are they different and how to determine when to use which command. From my observation, one use data from development db and the other use data from fixture.

said on December 2nd, 2009 at 18:55 · Permalink · Reply

The only difference is that rake spec executes db:test:prepare before running your test suite. It means, that spec spec is faster (on large databases — significantly faster), but you will have to run rake db:migrate RAILS_ENV=test manually to ensure your test database is up to day.

said on December 13th, 2009 at 16:52 · Permalink · Reply

I’ve tried using Rspec before and find that it takes longer to code the tests than to write the code correct in the first place. Maybe I’ve been doing this far too long (35 years now). Perhaps this system is useful for teams of people working on a project but for a single developer, I really don’t see the advantage of spending the time doing this when the time could be spent more productively.

Stephen Eilert
said on March 21st, 2011 at 19:44 · Permalink · Reply

Flyboy Art,

Indeed, it is faster to write the code in the first place. In some languages, such as Lisp (or Scheme), incredibly so, using the REPL. And, in the case of functional languages (NOT ruby), you can even prove that the code is correct.

The problem with that is when things change. And they *will*. You can’t do simple refactorings like changing a method name, while knowing for sure that your change didn’t break an obscure section in your app. Or, when you figure out a more efficient way to do something, it your business logic is still functioning.

said on December 14th, 2009 at 14:11 · Permalink · Reply

Hey FlyboyArt,

Okay. Please, share your experience with us. What makes you sure that your code working and working right? How easy will it be for you to change some logic in your project or just do a refactoring and be sure that you didn’t broke something your project?

said on December 16th, 2009 at 19:44 · Permalink · Reply

These are some good tips, but I don’t fully agree with all them. Particular points 4 and 5:

About mocks: In general, I agree with you that over mocking is a problem. But I disagree that it’s always better to use no mocking or stubbing in your controller specs. In my opinion, the danger of mocking is directly proportional to how likely it is the interface you are mocking will change. So, mocking a class method that frequently changes its interface is dangerous and should be avoided. Mocking an interface that is highly unlikely to change is generally safe and can lead to less brittle tests.

So, for example, when I’m spec’ing the #update and #create actions of a controller, I mock #valid? so that it returns true or false based on which code path I want to test. The alternative is to pass valid or invalid attributes in the request, but this leads to fragile specs: now my controller specs have to know what attribute values are valid and invalid, and when my model validation logic changes, my controller specs begin to fail. Really, I don’t care in my controller specs what is valid and invalid…I just care that when the attributes are valid, it does one thing, and when they are invalid, it does another thing. #valid? has been around in ActiveRecord for as long as I can remember (I imagine it was there from the first release) and the likely hood of that interface ever changing is approximately nil. I think it’s safe and useful to use mocking here.

About describe vs. context: In your example, you use context for both “.top” and “when just created”. I think context is appropriate for the latter (since “when just created” describes a context the examples run in) but describe is more appropriate for the former. “.top” isn’t a context; rather it’s simply grouping all the examples where you are describing the top method. I use use describe in these cases since you’re describing the behavior of a method.

said on December 17th, 2009 at 00:23 · Permalink · Reply

Hey Myron, thank you for the reply. I have updated an article with more consistent context usage, it was my failure.

About mocking in controllers, yes it makes sense. Sometimes. When you pretty sure you models work as expected, when you confident in their work. As I said, in some circumstances you can get false positives, for example, when add after_create hook which fails with an exception, or new super-useful NOT NULL restriction on one of columns in the database. If you have all your models covered with — it’s ok, use mocks to get controller specs clean in simple. By the way, did you hear about stub_model?

AnOldHacker
said on November 4th, 2010 at 20:59 · Permalink · Reply

First, lines like

1
require File.join(File.directory(__FILE__), '..', '..', 'spec_helper')

only load the file once per directory level, since the only difference is the number of ‘..’s. Second, if I want to be in the spec directory when I run, I should be allowed:

1
require File.expand_path(File.join(File.directory(__FILE__), '..', '..', 'spec_helper'))

Second, the problem of missing problems is only a problem if you don’t use integration tests and stories.

You have classes A and B. Each should be tested in near-isolation. You especially want to isolate from the database, for various reasons. BUT… you also need to have an integration test that runs A->B->database. If the later breaks, fixing it will break the obsolete unit tests, which is as it should be.

The other thing you did not seem to mention is the importance of testing interfaces, not implementations. Maybe that is a no-brainer to you, but code changes much faster than the interface.

said on March 10th, 2011 at 23:40 · Permalink · Reply

Excellent write-up, thanks for sharing! The last point with the snippet is useful, I placed it into my spec_helper.rb file.

said on August 6th, 2012 at 20:05 · Permalink · Reply

I methodized #7 into a file ‘prevent_multiple_require.rb’:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
# figure out where we are being loaded from
# see: http://kpumuk.info/ruby-on-rails/my-top-7-rspec-best-practices/
def prevent_multiple_require(regex, standardized_require_path)
  if $LOADED_FEATURES.grep(regex).any?
    begin
      raise "foo"
    rescue => e
      puts <<-MSG
    ===================================================
    It looks like #{File.basename(standardized_require_path)}.rb has been loaded
    multiple times. Normalize the require to:

    require "#{standardized_require_path}"

    Things like File.join and File.expand_path will
    cause it to be loaded multiple times.

    Loaded this time from:

      #{e.backtrace.join("\n    ")}
    ===================================================
      MSG
    end
  end
end

And then used in my spec helper like this:

1
2
require 'prevent_multiple_require'
prevent_multiple_require(/spec_helper\.rb/, "spec_helper")

This improves on #7 because you can customize the regex used for matching, and reuse it for other situations. I have made it as simple as possible because other paths that may require spec_helper may use ‘..’.

said on September 23rd, 2014 at 23:49 · Permalink · Reply

There is no need to raise an exception in order to get a backtrace; Kernel#caller provides the desired behaviour. Also, it is good practice to use Kernel#warn for warnings (which sends output to $stderr by default), instead of puts-ing out to $stdout where it is inseparable from the output of your specs:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
# figure out where we are being loaded from
if $LOADED_FEATURES.grep(/spec\/spec_helper\.rb$/).any?
  warn <<-MSG
  ===================================================
  It looks like spec_helper.rb has been loaded
  multiple times. Normalize the require to:

    require "spec/spec_helper"

  Things like File.join and File.expand_path will
  cause it to be loaded multiple times.

  Loaded this time from:

    #{caller.join("\n    ")}
  ===================================================
    MSG
  end
end

Post a comment

You can use simple HTML-formatting tags (like <a>, <strong>, <em>, <ul>, <blockquote>, and other). To format your code sample use [cc lang="php"]$a = "hello";[/cc] (allowed languages are ruby, php, yaml, html, csharp, javascript). Also you can use [cc][/cc] block and its syntax would not be highlighted.

Submit Comment