[rspec-users] Problem verifying routing error

r_j_h_box-sf at yahoo.com r_j_h_box-sf at yahoo.com
Wed Jun 17 19:54:15 EDT 2009



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.

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



More information about the rspec-users mailing list