[rspec-users] New rescue_from handling in rspec-rails 1.1.12

David Chelimsky dchelimsky at gmail.com
Wed Jan 14 22:30:38 EST 2009


On Wed, Jan 14, 2009 at 8:23 PM, Chris Kampmeier <chris at kampers.net> wrote:
> Howdy,
>
> This change to rspec-rails in 1.1.12 tripped me up:
> http://github.com/dchelimsky/rspec-rails/commit/ef6d2fc15a
> (or, see the relevant Lighthouse ticket, here:
> http://rspec.lighthouseapp.com/projects/5645/tickets/85)
>
> Basically, we were depending on that "bug" in 1.1.11, since it seemed to
> make sense to us. Here's some sample code... I tried to simplify it as much
> as possible:
>
> (Or, here's a syntax-highlighted pastie: http://pastie.org/361114)
>
>
> # spec/controllers/application.rb
> class AccessDeniedError < StandardError; end
> class ApplicationController < ActionController::Base
>   rescue_from AccessDeniedError, :with => :handle_access_denied
>
>   protected
>
>   def handle_access_denied(exception)
>     redirect_to "/"
>   end
> end
>
>
> # app/controllers/posts_controller.rb
> class PostsController < ApplicationController
>   def destroy
>     @post = Post.find(params[:id])
>     raise AccessDeniedError unless @post.destroyable_by?(current_user)
>
>     @post.destroy
>     redirect_to posts_url
>   end
> end
>
>
> # spec/controllers/posts_controller_spec.rb
> describe PostsController do
>   describe "handling DELETE /posts/2" do
>     it "raises AccessDeniedError if the post isn't destroyable by the
> current user" do
>       user = mock_model(User)
>       post = mock_model(Post, :destroyable_by? => false)
>
>       controller.stub!(:current_user).and_return(user)
>       Post.stub!(:find).and_return(post)
>
>       post.should_not_receive(:destroy)
>       lambda { delete :destroy, :id => '2' }.should
> raise_error(AccessDeniedError)
>     end
>   end
> end
>
>
> # spec/controllers/appcliation_controller_spec.rb
> class ApplicationTestController < ApplicationController
>   def access_denied_action
>     raise AccessDeniedError
>   end
> end
>
> describe ApplicationTestController, "#handle_not_found" do
>   before do
>     controller.use_rails_error_handling!
>   end
>
>   it "redirects to the front page" do
>     get :access_denied_action
>     response.should redirect_to("/")
>   end
> end
>
>
> So we'd check that the controller action raises a given error, even thought
> it might be rescued somewhere else (not using
> controller.use_rails_error_handling!). And then in a different file, we'd
> specifically check the ApplicationController to make sure that it rescued
> properly (*with* controller.use_rails_error_handling!). We figured that it's
> not PostsController's responsibility to do the rescue, so it shouldn't be
> tested there.
>
> This also kept our tests DRY; if we changed the rescue behavior, we wouldn't
> need to go through all our controller examples and e.g. change the redirect.
> It also felt nice and "unit test"-like to me, in that the implementation of
> PostsController clearly raises AccessDeniedError, and we'd check for that in
> the spec. This impedence mismatch bothers me:
>
> implementation: raise AccessDeniedError
> specification: response.should redirect_to("/")

Don't forget that the spec should come first :) Also, you're spec'ing
behaviour, not implementation. So for me, what you've got is this:

specification: response.should redirect_to("/")
behaviour: response.should redirect_to("/")

> I'm guessing that the argument _for_ the new behavior (in which rescue_from
> is always used) is that my PostsController inherits from
> ApplicationController, and therefore, inherits its behavior. So when I'm
> testing the behavior, it would violate the POLS if the inherited behavior
> was somehow missing. For example, I'd sure be confused if a Rails model was
> missing a feature of ActiveRecord::Base when used under test.

Exactly!

> Anyway, what should I really be doing here? I could use shared examples to
> maintain DRY (it_should_behave_like :access_denied);

Personally, I've grown weary of it_should_behave_like. I'd write a
macro so you could say something like:

describe PostsController do
  context "handling DELETE /posts/2" do
    denies_access_when "the post isn't destroyable by the current user" do
      post = mock_model(Post, :destroyable_by? => false)
      Post.stub!(:find).and_return(post)
      post.should_not_receive(:destroy)
    end
  end
end

>or I could just repeat
> myself, because that's the behavior I'm expecting: response.should
> redirect_to("/"). Or, I could alias_method_chain
> ActionController::Rescue#rescue_action and hack the old behavior back in. I
> don't really want to do that, though--somebody who was familiar with RSpec
> but hadn't seen our code before would be mighty confused.

A great reason not to do it :)

> Thanks for reading! That was a lot.

Sure - hope these responses are helpful.

Cheers,
David

> Chris Kampmeier
> http://shiftcommathree.com


More information about the rspec-users mailing list