Saturday, September 08, 2012

rspec, let(), let!(), activerecord and global state

Yesterday at work I was beating my head against a brick wall. The setup we have is machinist, rspec, and devise in an otherwise standard rails application.

We have blueprints for Users and Profiles, which are all pretty straight forward. When doing coverage of security related scenarios, we've got a helper

def login_as
   user = User.make!

   # .. and bits to make a faux current_user
end

All fairly straight forward so far. A typical security test might look like this:

describe "cat" do
  login_user

  let(:profile) {
    Profile.make!(:user_id => @user.id)
  }


  let(:cat) {
    Cat.make!(:profile_id => profile.id)
  }
  
  let(:brush) {
    Brush.make!(:way => "wrong", :owner => @user.id)
  }

  it "should not allow users to rub them the wrong way" do
    should_not be_able_to :brush, cat
  end


  it "should allow users to feed them" do
    should be_able_to :feed, cat
  end
end

Seen the problem? Of course, let() is being used as a kind of setup(), and if the guts of our code actually relies on the relationships created by executing the above code, you'd expect a User, Profile, Cat and Brush to be made for each it() block.

You'd be wrong, like I was, so helpfully a colleague tells you to always use let!() to ensure it's executed and instantiated.


describe "cat" do
  login_user


  let!(:profile) {
    Profile.make!(:user_id => @user.id)
  }


  let!(:cat) {
    Cat.make!(:profile_id => profile.id)
  }
  
  let!(:brush) {
    Brush.make!(:way => "wrong", :owner => @user.id)
  }

  it "should not allow users to rub them the wrong way" do
    should_not be_able_to :brush, cat
  end


  it "should allow users to feed them" do
    should be_able_to :feed, cat
  end
end

... and the test still doesn't work as you'd expect, with using let!() as a setup().


describe "cat" do
  login_user


  let!(:profile) {
    Profile.make!(:user_id => @user.id)
  }


  let!(:cat) {
    Cat.make!(:profile_id => profile.id)
  }
  
  let!(:brush) {
    Brush.make!(:way => "wrong", :owner => @user.id)
  }

  it "should allow users to feed them" do
    should be_able_to :feed, cat
  end

  it "should not allow users to rub them the wrong way" do
    should_not be_able_to :brush, cat
  end

end


But this version does.
What?

So after much further staring, I remembered that let() and let!() are all about caching - you call it once and the result is persisted across tests.

That's not the case with login_user, which executes every time - so all of a sudden, @user.id is different in multiple places, and it's completely non obvious - let() is making global state.

Why would anyone want to add global state to testing frameworks? It just causes Spooky Action at a Distance, like above.
What do we want to do instead? There's a before() which should almost always be used, as it's a direct relation to setup() from the xUnit family of tools.
But let() is there, so what's it actually for?

Dan's rules for let()

  • Only use this for otherwise dynamic values or complex calculations that you do want to fix at a point in time.
    let(:time) { Time.now }
    let!(:number) { Math.random }
  • If you have to use let() as a setup type thing, try to make One Big Setup
  • Don't listen to co-workers unless they are offering to buy you a beer


No comments: