[rspec-users] Problem verifying routing error

David Chelimsky dchelimsky at gmail.com
Wed Jun 17 23:30:47 EDT 2009


On Wed, Jun 17, 2009 at 6:54 PM, <r_j_h_box-sf at yahoo.com> wrote:
>
>
> Okay, after such a harsh analysis of the problem, I figured it was worth digging in just a little bit more.  Side note, some of what I was seeing yesterday was a by-product of having default routes still existing.  But there are still some essential facts that remain, which I present here.
>
> 1. The current implementation of route_for().should == "/something" is only consulting route recognition. A more descriptive phrasing of this behavior would be something like
>
>>   route_recognize("/something").should == { :action ... :controller ... }
>
> 2. of course, params_for() was *also* consulting route recognition through a slightly different code path, but eventually using exactly the same test.
>
> Therefore, route generation and recognition are a) redundant and b) incomplete.
>
>
> I did dig in more to the problem and spiked out a fairly solid
> alternative, which is 100% backward-compatible, plus a bunch.  Here's
> my thinking process:
>
> 1. If we were to use assert_generates(), it could test that the params result in the specified path, the way implied by the current route_for().should == syntax.  Note, however, that it doesn't verify that the :method arg is correct (if the hash with :path, :method is provided, it actually causes failure).
>
> 2. assert_recognizes() does test the method of { :path ... :method  }
>
> Therefore, if we can do both assert_generate() and assert_recognizes(), then we have covered testing a) actual route generation, b) method matching, and c) (bonus result) params_for() matching.  This last is nice, though kind of a hidden benefit that doesn't make itself obvious to the reader of the briefer routing spec.  Fortunately, it's optional - as I said, what I've done is 100% back-compatible.
>
>
> Also, to support my original goals, some methods (which require catching exceptions that would otherwise be informative) to provide:
>
> 1.  Non-routeability tests (:action ... :controller).should_not be_routable
>
> 2.  Path-cleanliness and negative method tests
>
>> route_for(:action ... :controller ... :args ).should_not be_post("/path")
>
> Hey, did you catch that?  be_post(), I said.  Well, because it catches exceptions, it's not so useful for positive cases.  So:
>
> 42.  A few extra methods that directly test expected positive cases, which can (but don't have to) replace .should == { :path ... :method }
>
>> route_for(:action ... :controller ... :args ).should post("/path")
>> route_for(:action ... :controller ... :args ).should put("/path")
>> route_for(:action ... :controller ... :args ).should get("/path")
>> route_for(:action ... :controller ... :args ).should delete("/path")
>
> This gist demonstrates a currently-working example of this method, annotated to introduce things step by step, including the results of my spike that makes it all work.
>
> http://gist.github.com/131569
>
> I hope that this is both more helpful than just my griping of yesterday, and that it adds some value to the project.

This is all good stuff Randy. Thanks.

Wanna make it a lighthouse ticket with patch? We can talk about the
get/put/post/delete-ish methods there.

Cheers,
David

>
> Randy
>
>
>
> ----- Original Message ----
>> From: "r_j_h_box-sf at yahoo.com" <r_j_h_box-sf at yahoo.com>
>> To: rspec-users <rspec-users at rubyforge.org>
>> Sent: Tuesday, June 16, 2009 5:23:34 PM
>> Subject: Re: [rspec-users] Problem verifying routing error
>>
>>
>> Here's another interesting symptom.  After tracing through the code, I've come
>> to the understanding that the current implementation (delegated to outside
>> rspec, I understand) of route "generation" is not
>> testing generation at all, but rather is using backward-recognition as a proxy.
>> Further, that recognition doesn't correspond to what the real router does
>> recognize.
>>
>> For clarity, here's the background: A resource that requires nesting for new,
>> create; requires
>> no nesting for edit, update, destroy, and has no index or show.
>>
>> >  map.resources :designs, :only => [:edit, :update, :destroy]
>> > map.resources :product, :member => { :redraw => :get } do |product|
>> >   product.resources :designs, :member => { :set_default =>
>> :put }, :except => [ :edit, :update, :destroy, :index, :show ]
>> > end
>>
>> Okay: when I go to /designs/new in the browser, it borks with a RoutingError.
>> That's the way I want it to behave, real-world.  Yet, this fails:
>>
>> > expect { route_for(:controller => "designs", :action => "new") ==
>> "/designs/new" }.to raise_error( ActionController::RoutingError )
>>
>> There's no error raised at all here.
>>
>> The following does gripe, but... what it's *really* griping about (in a hidden
>> way) is "bogus path", not about the route_for() params at all.
>>
>>
>> > expect {  route_for(:controller => "designs", :action
>> => "new") == "bogus path" }.to raise_error(
>> ActionController::RoutingError )
>>
>> (so if we replace route_for([bad]) with route_for([good]) == "bogus path", then
>> we still get the routing error.
>>
>>
>> Furthermore, the first one really recognizes the route string (/designs/new),
>> without actually verifying that there is a route in the routing table for it.
>> So I fear that it's not actually testing what I'm asking it to test.  Taking it
>> out of the expect {} *does* make it barf, but with evidence that something is
>> just plain confused, not that it's actually testing what we're asking it to
>> test:
>>
>> [wrapping is mine]
>> > The recognized options <{"action"=>"1", "controller"=>"designs"}>
>> > did not match <{"action"=>"show", "id"=>"1", "controller"=>"designs"}>,
>> > difference: <{"action"=>"show", "id"=>"1"}>
>>
>> At the end of the day, what I find is:
>>
>> * Route generation tests are not testing generation at all, but recognition only
>> * They're only testing recognition of ideal cases
>> * Non-existence of routes is currently not testable with rspec
>>
>> I hoped to just assert something on url_for() - that's the practical
>> application, here.  Does, or does not, url_for() produce a useful result with
>> specific args?  But I see from ActionController::Base how that's not super
>> practical.
>>
>> I sincerely hope that my understanding is wildly mistaken.
>>
>> Sorry if this is a sore spot; I know that this part has been a lot of painful
>> effort so far, far more for others than for myself.  I'll end with an expression
>> of deep and sincere appreciation for this great software.
>>
>> Randy
>>
>>
>>
>>
>> ----- Original Message ----
>> > From: "r_j_h_box-sf at yahoo.com"
>> > To: rspec-users
>> > Sent: Tuesday, June 16, 2009 2:14:52 PM
>> > Subject: Re: [rspec-users] Problem verifying routing error
>> >
>> >
>> > David, thank you for your reply on this.  I really dig the expect { }.to
>> > raise_error() syntax!!
>> >
>> > To clarify: All the things you're claiming match my expectation.
>> Unfortunately,
>> > my expectation does not match reality according to my tests.
>> >
>> > The thing is, route_for([bad stuff]) does not in and of itself raise a routing
>>
>> > error.  It constructs an object that hasn't yet been compared with == to
>> > anything.
>> >
>> > 23   t = route_for(:controller => "designs", :action => "create")
>> >
>> > (rdb:1) puts t
>> > #
>> >
>> > According to my tests, the routing error only occurs after route_for()'s
>> result
>> > gets compared to something.  So lambda { route_for(...) } does not raise
>> error.
>> >
>> > The following code passes with flying colors, either in lambda or expect {}.to
>>
>> > form:
>> >
>> >     t = route_for(:controller => "designs", :action => "create")
>> >     expect { t == "anything" }.to raise_error( ActionController::RoutingError
>> )
>> >     expect { t.should == "anything" }.to raise_error(
>> > ActionController::RoutingError )
>> >
>> > Any further ideas?
>> >
>> > Randy
>> >
>> >
>> >
>> >
>> >
>> > ----- Original Message ----
>> > > From: David Chelimsky
>> > > To: rspec-users
>> > > Sent: Tuesday, June 16, 2009 1:28:18 PM
>> > > Subject: Re: [rspec-users] Problem verifying routing error
>> > >
>> > > On Tue, Jun 16, 2009 at 3:07 PM, wrote:
>> > > >
>> > > > I finally figured this out.
>> > > >
>> > > > lambda { route_for(:controller => "designs", :action => "create").should
>> ==
>> > > "anything" }.should raise_error( ActionController::RoutingError )
>> > > >
>> > > > The clue was that I wasn't getting a routing error until I tried to
>> compare
>> > > route_for() with something.  route_for() seems to generate an object that
>> > > overrides ==(), and at that time it does raise the exception.  Now we wrap
>> > that
>> > > comparison in a lambda and assert that the *comparison* should raise the
>> > > expected routing error.
>> > > >
>> > > > So - great, we can actually test it.  But the syntax does leave something
>> to
>> >
>> > > be desired.  dchelimsky, can you recommend any alternatives that would be a
>> > bit
>> > > cleaner for testing that a route doesn't exist?
>> > > >
>> > >
>> > > You don't need the .should == "anything" in there. So this is a bit cleaner:
>> > >
>> > >   lambda { route_for(:controller => "designs", :action => "create")
>> > > }.should raise_error( ActionController::RoutingError )
>> > >
>> > > Also, since rspec-1.2.5 you can use expect/to:
>> > >
>> > >   expect { route_for(:controller => "designs", :action => "create")
>> > > }.to raise_error( ActionController::RoutingError )
>> > >
>> > > You could always kick it old-school:
>> > >
>> > >   e = nil
>> > >   begin
>> > >     route_for(:controller => "designs", :action => "create")
>> > >   rescue ActionController::RoutingError => e
>> > >   ensure
>> > >     e.should_not be_nil
>> > >   end
>> > >
>> > > And you could always wrap this in an new matcher:
>> > >
>> > > def be_routable
>> > >   Spec::Matchers.new :be_routable, self do |example|
>> > >     match do |params|
>> > >       e = nil
>> > >       begin
>> > >         example.route_for(params)
>> > >       rescue ActionController::RoutingError => e
>> > >       end
>> > >       !!e
>> > >     end
>> > >   end
>> > > end
>> > >
>> > > {:controller => "designs", :action => "create"}.should_not be_routable
>> > >
>> > > In this case you need to wrap the matcher's construction in a method
>> > > in order to provide access to the scope of the example (which is where
>> > > route_for lives). Also, I just whipped that up off the top of my head
>> > > - no idea if it actually works :)
>> > >
>> > > HTH,
>> > > David
>> > >
>> > >
>> > >
>> > >
>> > > > Thanks,
>> > > >
>> > > > Randy
>> > > >
>> > > >
>> > > >
>> > > >
>> > > > ----- Original Message ----
>> > > >> From: Ben Mabey
>> > > >> To: r_j_h_box-sf at yahoo.com; rspec-users
>> > > >> Sent: Friday, May 8, 2009 10:25:03 AM
>> > > >> Subject: Re: [rspec-users] Problem verifying routing error
>> > > >>
>> > > >> Randy Harmon wrote:
>> > > >> > Hi,
>> > > >> >
>> > > >> > When upgrading to rspec/rspec-rails 1.2.6 gem (from 1.1.12), I'm having
>> > > >> > a new problem verifying routes that should not exist.
>> > > >> >
>> > > >> > This is to support something like this in routes.rb:
>> > > >> >
>> > > >> > map.resources :orders do |orders|
>> > > >> >     orders.resources :items, :except => [:index,:show]
>> > > >> > end
>> > > >> >
>> > > >> > I used to use lambda {}.should_raise( routing error ), but it stopped
>> > > >> > detecting any raised error.  Requesting it through the browser produces
>> > > >> > ActionController::MethodNotAllowed (Only post requests are allowed).
>> But
>> > > >> > that error wasn't detected.
>> > > >> >
>> > > >> > When I skip the lambda, and just ask it to verify that the route does
>> > > >> > exist (which *should* fail), I get the same result for those :except
>> > > >> > actions as for a made-up action name.  Seems this must have something
>> to
>> > > >> > do with the change in how route_for delegates back to
>> ActionController's
>> > > >> > routing assertion (sez the backtrace :).
>> > > >> >
>> > > >> >
>> > > >> > NoMethodError in 'ItemsController route generation should NOT map
>> > > >> > #indewfefwex'
>> > > >> > You have a nil object when you didn't expect it!
>> > > >> > You might have expected an instance of Array.
>> > > >> > The error occurred while evaluating nil.first
>> > > >> >
>> > > >>
>> > >
>> >
>> /Library/Ruby/Gems/1.8/gems/actionpack-2.2.2/lib/action_controller/assertions/routing_assertions.rb:134:in
>> > > >> > `recognized_request_for'
>> > > >> >
>> > > >>
>> > >
>> >
>> /Library/Ruby/Gems/1.8/gems/actionpack-2.2.2/lib/action_controller/assertions/routing_assertions.rb:49:in
>> > > >> > `assert_recognizes'
>> > > >> >
>> > > >>
>> > >
>> >
>> /Library/Ruby/Gems/1.8/gems/actionpack-2.2.2/lib/action_controller/assertions.rb:54:in
>> > > >> > `clean_backtrace'
>> > > >> >
>> > > >>
>> > >
>> >
>> /Library/Ruby/Gems/1.8/gems/actionpack-2.2.2/lib/action_controller/assertions/routing_assertions.rb:47:in
>> > > >> > `assert_recognizes'
>> > > >> > ./spec/controllers/thoughts_routing_spec.rb:9:
>> > > >> >
>> > > >> >
>> > > >> > I tried using bypass_rescue in my routing/items_routing_spec.rb file as
>> > > >> > mentioned by the upgrade doc, but it wasn't valid in the "routing" spec
>> > > >> > - worked fine when I moved the file back to spec/controllers/, though.
>> > > >> > Seems like that's not the issue, but I'm mentioning for more
>> > completeness.
>> > > >> >
>> > > >> > Any ideas what I should be doing instead, or how I can troubleshoot
>> > > further?
>> > > >> >
>> > > >>
>> > > >>
>> > > >> Hmm.. yeah, it seems like it might have to do with how the exceptions
>> > > >> are being handled in the newer version of rspec-rials (see
>> > > >>
>> > >
>> >
>> https://rspec.lighthouseapp.com/projects/5645/tickets/85-11818-have-mode-for-rails-error-handling).
>> > > >>
>> > > >> I don't use RSpec to verify my routes very often and have never used it
>> > > >> to verify the non-existence of a route so I'm afraid I don't really have
>> > > >> any ideas...
>> > > >>
>> > > >> Does anyone else have an idea to do this?
>> > > >>
>> > > >> -Ben
>> > > >
>> > > > _______________________________________________
>> > > > rspec-users mailing list
>> > > > rspec-users at rubyforge.org
>> > > > http://rubyforge.org/mailman/listinfo/rspec-users
>> > > >
>> > > _______________________________________________
>> > > rspec-users mailing list
>> > > rspec-users at rubyforge.org
>> > > http://rubyforge.org/mailman/listinfo/rspec-users
>> >
>> > _______________________________________________
>> > rspec-users mailing list
>> > rspec-users at rubyforge.org
>> > http://rubyforge.org/mailman/listinfo/rspec-users
>>
>> _______________________________________________
>> rspec-users mailing list
>> rspec-users at rubyforge.org
>> http://rubyforge.org/mailman/listinfo/rspec-users
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>


More information about the rspec-users mailing list