RSpec: Causing ourselves much pain through 'attr' misuse

While testing some code that we were mixing into one of our controllers we made what I thought was an interesting mistake.

The module we wanted to test had some code a bit like this…

module OurModule
def some_method
@User = User.find(params[:id])
# in the test code this is always true
if @user == user
...
end
end
end

..and we had the spec setup like so:

describe 'OurController' do
class TestController
include OurModule
attr_accessor :user
end
before(:each) do
@controller = TestController.new
@controller.user = Factory(:user)
end
it "should do something" do
# whole load of setup not related to our mistake
@controller.some_method
end
end

We created the ‘attr_accessor’ so that we would be able to choose a value for ‘user’ to simulate the fact that Warden mixes in a ‘user’ method into our ApplicationController at runtime.

The problem we ran into was that both ‘user’ and ‘@user’ inside ‘some_method’ were always returning the same value even though we set ‘user’ to be something else in our spec before each spec is run.

We eventually rotated pairs and immediately realised that the reason that was happening was because ‘user’ was in fact referring to the value we’d just set for ‘@user’ since in the test ‘user’ is created by an ‘attr_accessor’ call.

As it turns out Warden also mixes in a ‘current_user’ method so we changed the code to make use of that instead of ‘user’ since that was the team’s agreed standard and the call to ‘user’ hadn’t yet been replaced.

It does show an interesting side effect of setting instance variables in modules and also suggested that we had setup the test wrong to being with.

A better approach would have been to simulate what the actual code would do more closely and define a ‘user’ method which returned a canned user in a way that only relied on code in the test and wouldn’t change based on what the system under test did.