Problem verifying routing error

Testing RSpec
1

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:inassert_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:inassert_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

2

Hi again. I haven’t heard any responses on this thread (is this thing
on? :wink: .

Is there any known pattern for verifying a route that doesn’t exist,
as of Rspec 1.2.6?

Thanks,

Randy

3

Hi again. I haven’t heard any responses on this thread (is this thing
on? ;).

Is there any known pattern for verifying a route that doesn’t exist,
as of Rspec 1.2.6?

Thanks,

Randy

4

Randy H. wrote:

/Library/Ruby/Gems/1.8/gems/actionpack-2.2.2/lib/action_controller/assertions.rb:54:in

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
Lighthouse - Beautifully Simple Issue Tracking).
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

5

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

----- Original Message ----

6

Thanks, Ben.

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 M.:

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
Lighthouse - Beautifully Simple Issue Tracking).

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

7

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 ----

8

On Tue, Jun 16, 2009 at 3:07 PM, [email protected] 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 :slight_smile:

HTH,
David

9

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 ----

10

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 … }

  1. 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:

  1. 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 ----

11

On Wed, Jun 17, 2009 at 6:54 PM, [email protected] wrote:

Therefore, route generation and recognition are a) redundant and b) incomplete.
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.
Hey, did you catch that? be_post(), I said. Well, because it catches exceptions, it’s not so useful for positive cases. So:
RouteFor class · GitHub

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