infer base_class for anonymous controller #10

Closed
bjeanes opened this Issue Jul 30, 2011 · 3 comments

Comments

Projects
None yet
2 participants

bjeanes commented Jul 30, 2011

Given the following contrived spec, designed to test the Admin::BaseController behavior (through a subclass)

# Admin::BaseController is not-routed. Superclass of all Admin controllers
describe Admin::BaseController do
  controller(Admin::BaseController) do
    def index
      render :text => "ok"
    end
  end

  it "denies acces" do
    get :index
    request.should_not be_ok
  end
end

The controller method call expects a base class argument to make the spec work as expected. However, I've already declaratively stated the controller whose behavior I wish to describe in the describe block.

I put forth that this can be inferred by changing the default argument to controller_class instead of ApplicationController, providing this doesn't present a large backwards-compatibility issue.


IRC conversation to jog your memory about our discussion, if needed:

[14:23] <dchelimsky> so there are two controller methods
[14:23] <dchelimsky> one at the class level (which is the one you're talking about)
[14:23] <bjeanes__> right, one in a spec, and one in a spec group
[14:23] <dchelimsky> exactly
[14:24] <bjeanes__> unless they are tied together in a way I don't realize, I'm just speaking about the specgroup one
[14:24] <dchelimsky> they are tied together
[14:24] <dchelimsky> the example method provides an instance of the controller class being described
[14:24] <bjeanes__> yup
[14:25] <dchelimsky> the group method lets  you specify a base class to work from
[14:25] <dchelimsky> but you don't need that under normal circumstances
[14:25] <dchelimsky> if you say "describe FooController", then the example method gives you an instance of that
[14:25] <bjeanes__> yup I realize that
[14:26] <dchelimsky> guess I don't understand what you're trying to do
[14:26] <bjeanes__> my interpretation of the specgroup controller method was to give you an interface for testing behaviour of non-routable controllers like ApplicationController (or others, in my case)
[14:26] <bjeanes__> e.g. Admin::BaseController
[14:26] <dchelimsky> in which case you can pass it Admin::BaseController
[14:26] <bjeanes__> that very much seems to be the example in the code comment anyway
[14:27] <bjeanes__> which is what I am doing, but I'm already wrapping that controller call in `describe Admin::BaseController`
[14:27] <dchelimsky> oh I see
[14:27] <dchelimsky> you want it to infer that from the described class
[14:27] <dchelimsky> got it
[14:27] <dchelimsky> let me think about that
[14:27] <bjeanes__> so I'm wondering why there is a use-case for it to default to an ApplicationController subclass when I've already declaratively stating which controller's behaviour I wish to describe
[14:27] <dchelimsky> not sure I want to add anything implicit there
[14:27] <bjeanes__> yeah I don't mind either way, just curious
[14:28] <bjeanes__> anywho thanks for listening
[14:29] <dchelimsky> sure - thanks for explaining
[14:29] <bjeanes__> love the direction rspec has taken recently in helping me make my specs nice and clear
[14:29] <dchelimsky> I think it's probably a good idea, had it been that way from the beginning
[14:29] <bjeanes__> if this is a place to improve that, awesome... if not, no biggy
[14:29] <bjeanes__> sure, I can see that
[14:29] <dchelimsky> I need to think about what the risk would be to make the change
[14:29] <bjeanes__> alrighty
[14:29] <dchelimsky> would  you do me a favor and file a feature request for it?
[14:30] <dchelimsky> or call it a bug report, if you prefer ;)
Owner

dchelimsky commented Aug 2, 2011

I think this is the right way to go, but it has the potential to break existing specs, so let's do this in the 3.0 release (whenever we get to it).

Owner

dchelimsky commented Aug 2, 2011

Oh - just realized this is the rspec-rails-1 tracker - would you do me one more favor and repost this at http://github.com/rspec/rspec-rails/issues? That way it's still coming from you and you'll get notified w/ updates.

dchelimsky closed this Aug 2, 2011

bjeanes commented Aug 10, 2011

Sure, sorry for the delay. I'm re-opened it in the other tracker.

rspec/rspec-rails#421

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