On Wed, Jun 17, 2009 at 6:54 PM, <r_j_h_box...@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...@yahoo.com" <r_j_h_box...@yahoo.com>
>> To: rspec-users <rspec-users@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...@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...@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@rubyforge.org
>> > > > http://rubyforge.org/mailman/listinfo/rspec-users
>> > > >
>> > > _______________________________________________
>> > > rspec-users mailing list
>> > > rspec-users@rubyforge.org
>> > > http://rubyforge.org/mailman/listinfo/rspec-users
>> >
>> > _______________________________________________
>> > rspec-users mailing list
>> > rspec-users@rubyforge.org
>> > http://rubyforge.org/mailman/listinfo/rspec-users
>>
>> _______________________________________________
>> rspec-users mailing list
>> rspec-users@rubyforge.org
>> http://rubyforge.org/mailman/listinfo/rspec-users
>
> _______________________________________________
> rspec-users mailing list
> rspec-users@rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>
_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users
