Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

It would be nice to have something like controller.should use_before_filter(:filter_name).for(:show)... #106

Closed
fidothe opened this Issue Jun 16, 2010 · 8 comments

Comments

Projects
None yet
7 participants

fidothe commented Jun 16, 2010

... to enable declarative assertions about whether filters should fire for a given action. I use this a lot in conjunction with shared guard filters where I don't want to have to do the setup for every controller test so I assert that the filter will be used and then stub it out... My implementation is at http://github.com/fidothe/shoulda/tree/use_filter, in lib/shoulda/action_controller/matchers/use_filter_matcher, tests in test/matchers/controller/use_filter_matcher_test.rb.

fidothe commented Sep 14, 2010

I rebranched from master this morning and cherry-picked in the commits from my old branch for ease of understanding. The new branch is at http://github.com/fidothe/shoulda/tree/should_use_filter. (The tests and features all pass)

Contributor

croaky commented Sep 14, 2010

Hi Matt,

Thanks for following up on this and sorry we never got back to you originally. I think our feeling on this is that testing that the before_filter is invoked is too implementation-specific. If the before_filter was doing some kind of authentication, we'd likely write a custom matcher like should authenticate(:admin) or something, not should use_filter(:authenticate).

We're always willing to hear the reasons otherwise, but I think it's unlikely that this matcher will make its way into Shoulda.

Thanks again,
Dan

fidothe commented Sep 14, 2010

Hey, no problem.

I've been using this approach because it helps me keep testing of the implementation of the controller super structure (filters, other non-action controller methods) separate from testing the action methods themselves. I tend to use an approach to writing controllers which puts almost all authorisation-type logic, and setup like finding/initialising models, into before filters used as guard methods, leaving the action method's body as vanilla as possible (cf voxdolo's decent_exposure)

It's pretty rare nowadays that I use a controller-focussed unit test rather than a cucumber feature, but when I do its probably for something like:

  • non-trivial logic in the action method (getting very rare these days)
  • non-trivial guard filters
  • guard-filters only run on certain actions
  • several guard filters

With specs (I use RSpec) looking a bit like this:

describe InterestingController do
  describe "guard_filter" do
    it "only does X when the moon is full" do
      ...
    end
  end

  describe "GET :show" do
    it { should use_before_filter(:guard_filter).for(:show) }

    describe "complex workings of show method" do
      before(:each) do
        controller.stub!(:guard_filter)
      end

      it "correctly does X" do
        ...
      end

      ...
    end
  end

  describe "GET :index" do
    it { should_not use_before_filter(:guard_filter).for(:index) }
  end
end

The should use_X_filter becomes, in my mind anyway, something closer to model validation matchers like should validate_presence_of. It's tied to implementation, but I see it as being of a piece with the model validation matchers. Certainly, I only use it in a declarative fashion. I've found that a lot of my projects wind up sharing several guard filters through ApplicationController, and then should use_X_filter().for() becomes a design-time validation-like assertion, something I use when sketching out the behaviour of a new controller.

Hope that makes things a bit clearer -- I've no problem with you not wanting to put it in Shoulda, but it felt worthwhile explaining myself a little.

Matt

Owner

jferris commented Dec 21, 2010

Hey Matt,

Thanks for sharing your thoughts on this. My issue with testing before_filters in a controller vs testing validations for models is that a filter isn't part of the public interface for a controller. You can use a model as it's intended by providing attributes and checking the validation errors. However, testing controller filters directly doesn't test the actual effect they have on the states that are publicly available, like redirect locations. Thanks again for the patch, but this isn't something we want to pull into shoulda.

-Joe

pzac commented Dec 21, 2010

I think this matcher could be useful to simplify access control tests, which are usually very boring to code.

Paolo

I'm curious about this, since I just recently got a similar problem. I can find this particularly useful for controller wide before filters.

If I have a before filter defined in my application controller I would test it by using an anonymous controller in RSpec. In other controllers I don't want branch out and test all the different possibilities because I already tested that in my application controller spec. Instead I would like to ensure that a before filter is present for a specific action.

I don't see this as "testing" the before filters per se, but instead asserting that a given action is "covered".

Any thoughts?

👍 I see huge value in what @KevinSjoberg suggests.

molfar commented Apr 28, 2014

This is a greate idea to use construction like this

use_before_filter(:guard_filter).for(:index)

I don't understand why this issue is closed.

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment