opensoul.org:

Ruby's require doesn't expand paths

I ran across this issue several weeks ago, but it came up at the latest Grand Rapids Ruby Group meeting, so I thought I would share it.

Ruby’s require, for better or worse, doesn’t expand paths. As the docs point out, require 'a'; require './a' will load a.rb twice. This doesn’t matter most of the time, but there’s one place it’s used often that will bite you: tests and specs.

Every Rails’ test has a line similar to this:

require File.dirname(__FILE__) + '/../test_helper'

This doesn’t normally cause any problems, as long as every test has an identical require statement. Where you start to get into problems is when you have tests in a nested subdirectory (like test/controllers/admin/users_controller_test.rb), in which case the require statement would look like this:

require File.dirname(__FILE__) + '/../../test_helper'

require sees this as a different file and will re-load it. This still shouldn’t hurt you unless you’re doing something in test_helper.rb that would be changed by loading it twice (like aliasing a method). This also effects RSpec with requiring spec_helper.rb

The solution? Expand the path yourself.

require File.expand_path(File.dirname(__FILE__) + '/../../test_helper')

It’s not really clear to me why Ruby’s require works this way. You would think a method that was intended to only load a file once would make sure that it never re-loaded the same file, no matter how it was referenced. It definitely doesn’t adhere to the “principle of least surprise”. Any idea why?

RSpec is getting too intimate with my code

The theory is that tests are supposed to be agnostic of the implementation. This leads to less brittle tests and actually tests the outcome (or behavior).

With RSpec, I feel like the common approach of completely mocking your models to test your controllers ends up forcing you to look too much into the implementation of your controller.

Here is an example of a spec that is generated by the rspec_scaffold generator:

describe ThingsController, "handling POST /things" do

  before do
    @params = {}
    @thing = mock_model(Thing, :to_param => "1", :save => true)
    Thing.stub!(:new).and_return(@thing)
  end

  def do_post
    post :create, :thing => @params
  end

  it "should create a new thing" do
    Thing.should_receive(:new).with(@params).and_return(@thing)
    do_post
  end

  it "should redirect to the new thing" do
    do_post
    response.should redirect_to(@thing)
  end
end

This by itself is not too bad, but the problem is that it peers too much into the controller to dictate how the model is used. Why does it matter if my controller calls Thing.new? What if my controller decides to take the Thing.create! and rescue route? What if my model has a special initializer method, like Thing.build_with_foo? My spec for behavior should not fail if I change the implementation.

This problem gets even worse when you have nested resources and are creating multiple models per controller. Some of my setup methods end up being 15 or more lines long and VERY fragile.

RSpec’s intention is to completely isolate your controller logic from your models, which sounds good in theory, but almost runs against the grain for an integrated stack like Rails. Especially if you practice the skinny controller/fat model discipline, the amount of logic in the controller becomes very small, and the setup becomes huge.

So what’s a BDD-wannabe to do? Taking a step back, the behavior that I really want to test is not that my controller calls Thing.new, but that given parameters X, it creates a new thing and redirects to it.

Here’s the approach I’ve been taking:

# spec_helper.rb
def valid_thing_attrs(attrs = {})
  {:name => 'Foo'}.merge(attrs)
end

# things_controller_spec.rb
describe ThingsController, "handling POST /things" do

  def do_post
    post :create, :thing => valid_thing_attrs
  end

  it "should create a new thing" do
    lambda { do_post }.should change { Thing.count }.by(1)
  end

  it "should redirect to the new thing" do
    do_post
    response.should redirect_to(@thing)
  end
end

My spec is now testing that posting certain attributes creates a new thing (in the database) and redirects to it. You’ll notice that I have a method called valid_thing_attrs. I don’t remember where I picked up this pattern, but it is something that has allowed me to minimize all of my test’s dependencies on the model.

My spec is now somewhat dependent on my model–and thus will break if my model gets hosed–but that is a small price to pay, in my opinion, for an implementation-agnostic spec with significantly less overhead.

What do you think?

RSpec 1.0

I am hanging out in the lobby of our hotel at RailsConf as David Chelimsky and Aslak Hellesøy are releasing RSpec 1.0. Congrats to them and everyone else involved with the project!

rspec: model.should be_valid

One of the things I always check when spec’ing is that the model that I just created is valid.

@mymodel.should be_valid

This uses RSpec’s predicate support (any call to be_something? calls something? on the target object), and returns a correct but very unhelpful error:

expected valid? to return true, got false

Here is a little rspec matcher that I threw together this morning to check if a model is valid and spit out the validation errors:

module Spec
  module Rails
    module Matchers
      class BeValid  #:nodoc:

        def matches?(model)
          @model = model
          @model.valid?
        end

        def failure_message
          "#{@model.class} expected to be valid but had errors:\n  #{@model.errors.full_messages.join("\n  ")}"
        end

        def negative_failure_message
          "#{@model.class} expected to have errors, but it did not"
        end

        def description
          "be valid"
        end

      end

      def be_valid
        BeValid.new
      end
    end
  end
end

Now, I get:

MyModel expected to be valid but had errors:
  Password confirmation can't be blank
  Email has already been taken

This is nothing fancy, but it’s the small stuff that make life meaningful, so I thought I would share it. Throw the matcher code above into your spec_helper and check if your models are valid.

update: added negative_failure_message so “model.should_not be_valid” will work