[rspec-users] Problem verifying routing error
Randy Harmon
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?
Thanks,
Randy
_______________________________________________
rspec-users mailing list
rspec...@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users
Randy Harmon
on? ;) .
Is there any known pattern for verifying a route that *doesn't* exist,
as of Rspec 1.2.6?
Thanks,
Randy
Randy Harmon
on? ;).
Is there any known pattern for verifying a route that *doesn't* exist,
as of Rspec 1.2.6?
Thanks,
Randy
Ben Mabey
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
Randy Harmon
>> 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'
[clip]
[reordered] Ben Mabey:
> 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).
Right, the bypass_rescue() method. Just to clarify:
Me:
>> 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.
"worked fine", by which I mean "didn't have a problem with me calling
it" - but didn't provide any exception for me to catch.
Ben:
> 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...
Maybe I'm out of my tree a bit to want it. I guess I don't really care
to validate has_many relationships, why should it matter to validate
:except rules? Should I adjust my thinking on this one?
Thanks,
Randy
r_j_h_...@yahoo.com
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?
Thanks,
Randy
David Chelimsky
>
> 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
r_j_h_...@yahoo.com
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
#<Spec::Rails::Example::RoutingHelpers::RouteFor:0x208ca44>
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 <dchel...@gmail.com>
> To: rspec-users <rspec...@rubyforge.org>
> Sent: Tuesday, June 16, 2009 1:28:18 PM
> Subject: Re: [rspec-users] Problem verifying routing error
>
r_j_h_...@yahoo.com
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_...@yahoo.com" <r_j_h_...@yahoo.com>
> To: rspec-users <rspec...@rubyforge.org>
> 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
> #
>
r_j_h_...@yahoo.com
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.
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_...@yahoo.com" <r_j_h_...@yahoo.com>
> To: rspec-users <rspec...@rubyforge.org>
> Sent: Tuesday, June 16, 2009 5:23:34 PM
> Subject: Re: [rspec-users] Problem verifying routing error
>
>
David Chelimsky
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