[rspec-users] What do you think of this controller spec?

[David Chelimsky]

On 12/24/06, Pat Maddox <pergesu at gmail.com> wrote:
> Here's a controller spec I wrote, it's for a very simple RESTful
> controller (well, aren't all RESTful controllers simple? :)
>
> I've created a couple spec helper methods to refactor some of the
> common code...for example, require_login_and_correct_user creates two
> specifications: one for when the user isn't logged in, and one when
> the user is logged in but requests someone else's book.  do_request
> automatically calls do_get/post/put/delete depending on what's
> implemented.
>
> Here's all the code:
>
> books_controller.rb: http://pastie.caboo.se/29469
> books_controller_spec.rb: http://pastie.caboo.se/29470
> spec_helper.rb: http://pastie.caboo.se/29471
>
> The specs just feel somewhat unwieldy to me...maybe it's because the
> controller code is very simple and elegant, and the spec looks big and
> bloated in comparison.  Also, it feels like it's taking me a lot
> longer to write the controller specs with all these individual
> verifications.
>
> Am I just tripping for no reason, and the specs look fine?  What would
> you change?  Also I know that the controller code could be improved by
> using the finder on the association, this is just how I have it for
> now.  I'm interested in how the specs could be better.
>
> I'll be understanding if I don't get a response for a few days :)
> Merry Christmas everybody!

Quick observations:

1. At first glance I like the structure of wrapping specify blocks in
methods like this.

2. Instead of having free floating methods I'd wrap them in a module
and include the module in the context:

module MyHelperMethods
  def login(user, pw)
    ...
  end
end

context "Given blah...." do
  include MyHelperMethods
  setup {..}
  specify "a logged in user..." do
    login(@user, @pw)
    ..
  end
end

3. The extractions are inconsistent. For example:

setup do
  create_user
  login_as @mock_user
end

The reference to @mock_user is confusing here. Where does it come from?

To make it easier to understand, I might phrase this like this:

setup do
  @mock_user = create_user
  login_as @mock_user
end

or, more terse:

module MyHelpers
  def mock_user
    @mock_user ||= mock("user")
  end
  def login_as(user)
    ..
  end
end

context ...
  setup do
    login_as mock_user
  end
end

Using a mock_user method like that could make the specs look more
consistent (i.e. not sometimes referencing @mock_user and sometimes
referencing mock_user).

4. The magic involved in having should_find_user call do_get is a bit
troubling for me. I appreciate the need to indicate what to call, but
I'd do it more explicitly:

should_find_user_on :get

... or something like that. Then you can eliminate do_request and the
spec becomes a bit more explicit.

====================
Pat, this is great stuff in general. The challenge is to eliminate
duplication in such a way that each spec is still completely
understandable without having to go read the other helpers, etc. I
hope my suggestions here help a bit.

I'll give this a more thorough look over later in the week.

Cheers,
David

>
> Pat
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>