OK, I will chime in here, since I think I might have opened up this
can of worms. :)
I agree with David that we should stick with wrapping the Rails public
APIs. That is: assert_generates, assert_recognizes, and assert_routing
(http://edgeapi.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html).
OK, here is my thoughts on a format:
http://gist.github.com/464636
I want it to be readable in English, so let's try:
describe "routing" do
specify "days#index" do
route get('/days').should generate 'days#index' # wraps
assert_generates
end
end
specify [that the] route [path] get('/days') should generate [the
options] 'days#index'
OK! Next:
describe "routing" do
specify "days#index" do
routing 'days#index'.should recognize get('/days') # wraps
assert_recognizes
end
end
specify [that the] route [options] 'days#index' should recognize [the
path] get('/days')
(Might need some work... but it is uniform). Next:
describe "routing" do
specify "days#index" do
route get('/days').should be 'days#index' # wraps assert_routing
end
end
specify [that the] route [path] get('/days') should be [the options]
'days#index'
Finally, this makes nested routes (my original issue) look nice:
describe "routing" do
specify "days#index" do
route 'days#index'.should recognize get('/students/1/days') with
:student_id => '1'
end
end
specify [that the] route [options] 'days#index' should recognize [the
path] get('/students/1/days') with [extras] :student_id => '1'
You could also write out the options longhand to include the extras,
but I think this is too ugly:
describe "routing" do
specify "days#index" do
route {:controller => 'days', :action => 'index', :student_id =>
'1'}.should recognize ('/students/1/days')
end
end
specify [that the] route [options] {:controller => 'days', :action =>
'index', :student_id => '1'} should recognize [the path]
('/students/1/days')
What do you think?
Trevor
on 2010-07-05 22:00
on 2010-07-06 15:16
On Jul 5, 2010, at 2:59 PM, Lalish-Menagh, Trevor wrote: > I want it to be readable in English, so let's try: > describe "routing" do > specify "days#index" do > route 'days#index'.should recognize get('/students/1/days') with > specify "days#index" do > > Trevor Hey Trevor, These are all great DSL ideas, but they strike me as being sufficiently different from anything else in either RSpec or Rails that it would be for confusing for users. Do you have any thoughts on the other proposals? Cheers, David
on 2010-07-07 01:17
Hi David,
You make a good point. I was talking with a coworker about this
problem, and he suggested a simpler format, that I think will coincide
some with Wincent's thoughts. Here is my next stab
(http://gist.github.com/466064):
describe "routing" do
it "recognizes and generates #index" do
get("/days").should have_routing('days#index')
end
it "generates #index" do
get("/days").should generate('days#index')
end
it "recognizes #index" do
('days#index').should recognize get("/days")
end
describe "nested in students" do
it "recognizes #index" do
('days#index').with(:student_id => "1").should recognize
get("/students/1/days")
end
end
end
Notes:
- I am using have_routing, recognize, and generate because those are
the verbs used in Rails for the functions we are wrapping.
- I like using the word "with" to represent "extras," in the Rails API
(see
http://edgeapi.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html),
since I think it fits here, and we already use with in RSpec in other
places (like stubs, etc.).
- I like using get('path') since it is similar to the routing calls in
the Rails 3 route file, and I think it is easy to intuit.
- We can use the hash notation to conform to Rails 3, with an option
to provide a full hash as well (Rails 2 style).
- The format still reads like English, and using "have_routing"
instead of "routes_to" avoids the "_to" and "_from" problem that we
have been talking about, PLUS it makes it easier to draw a
relationship between the RSpec command and the Test::Unit command
(assert_routing).
- Using these verbs still allow "should_not" to make sense.
Thoughts,
Trevor
On Tue, Jul 6, 2010 at 9:14 AM, David Chelimsky <dchelimsky@gmail.com>
wrote:
> Hey Trevor,
>
> These are all great DSL ideas, but they strike me as being sufficiently different from anything else in either RSpec or Rails that it would be for confusing for users.
>
> Do you have any thoughts on the other proposals?
>
> Cheers,
> David
--
Trevor Lalish-Menagh
484.868.6150 mobile
trev@trevreport.org
http://www.trevmex.com/
on 2010-07-07 08:43
El 07/07/2010, a las 01:16, Lalish-Menagh, Trevor escribió: > Hi David, > > You make a good point. I was talking with a coworker about this > problem, and he suggested a simpler format, that I think will coincide > some with Wincent's thoughts. Here is my next stab > (http://gist.github.com/466064): > describe "routing" do > it "recognizes and generates #index" do > get("/days").should have_routing('days#index') > end Very nice. I definitely like this better than the "map()" terminology that I used. > it "generates #index" do > get("/days").should generate('days#index') > end Reads well, _but_ I think you've got it back-to-front. assert_generates, and therefore the corresponding matcher in RSpec, asserts that a _path_ can be generated from a set of _params_ (controller, action, additional params), but here you're asserting the opposite. (One thing to note is that assert_generates really only does assert about the _path_, not the method + path, but it's still nice to write it as "get(path)" just for symmetry with the reverse version of the spec.) > it "recognizes #index" do > ('days#index').should recognize get("/days") > end This one is back-to-front too; assert_recognizes is an assertion that a path (this time method + path) is recognized as a particular route (action, controller, additional params) but here you're asserting the opposite. If we're really serious about using the same verbs as the Rails assertions, and we want to forward and reverse versions of the spec to read in the same way, we could use something like: specify { get('/days').should recognize_as('days#index') } specify { get('/days').should generate_from('days#index') } If we don't mind swapping the order around specify { get('/days').should recognize_as('days#index') } specify { 'days#index'.should generate('/days') } Note that in the "generate" spec here I drop the "get()" seeing as Rails isn't actually looking at the method, and it is just verbiage IMO to start nesting other method calls inside the generate() matcher. Weighing up the two orders, I prefer the first pair of specs, I think, because: - the repetition of the pattern makes it easier to remember and apply. - if you have to supply additional parameters (as you do in your later example), using the first format you can write ('days#index', :student_id => '1') whereas with the second format you are obliged to involve another method like with() in order to express it as "('days#index').with(:student_id => '1')". > describe "nested in students" do > it "recognizes #index" do > ('days#index').with(:student_id => "1").should recognize > get("/students/1/days") > end > end > end This is another back-to-front one, I think. You're not recognizing here, but generating. > Notes: > - I am using have_routing, recognize, and generate because those are > the verbs used in Rails for the functions we are wrapping. Seems like a good idea that we should definitely try to do, although with one reservation; if we feel that the language is confusing or suboptimal then we should feel free to change it. I personally have lingering doubts about the Rails language because I think it _is_ confusing in a way. Look at the way you used recognize and generate back-to-front in this message. And I know that every time I want to make a comment about "assert_recognizes" and "assert_generates" I end up double-checking the API docs just to make sure that I'm about to use them in the right way. > - I like using the word "with" to represent "extras," in the Rails API > (see http://edgeapi.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html), > since I think it fits here, and we already use with in RSpec in other > places (like stubs, etc.). I don't really like it; I think it adds unnecessary verbiage to the specs. > - I like using get('path') since it is similar to the routing calls in > the Rails 3 route file, and I think it is easy to intuit. > - We can use the hash notation to conform to Rails 3, with an option > to provide a full hash as well (Rails 2 style). Yes, the example I posted at http://gist.github.com/464081 does that. > - The format still reads like English, and using "have_routing" > instead of "routes_to" avoids the "_to" and "_from" problem that we > have been talking about, PLUS it makes it easier to draw a > relationship between the RSpec command and the Test::Unit command > (assert_routing). Yeah, I think "have_routing" is a definite improvement over "map". I am less convinced that "recognize_as" is an improvement over the highly readable "map_to". I am less attached to "map_from", though, and think "generate_from" might be an improvement. Like I said above, I think the Rails terminology does have the potential to be confusing, so I don't necessarily feel "wedded" to it. > - Using these verbs still allow "should_not" to make sense. Yes, I think that's important. One thing I wanted to add is that I discovered in my testing that the existing "be_routable" matcher isn't very useful. This is because it relies on "recognize_path", which I am not sure is public API or not, and "recognize_path" sometimes recognizes the unrecognizable... For example: in config/routes.rb: resource :issues, :except => :index visit issues#index (/issues) in your browser: get a RoutingError (expected) in your spec: recognize_path('/issues', :method => :get) # recognizes! So this means you can't do stuff like get('issues#index').should_not be_routable, because the Rails recognize_path() method will tell you that it _is_ routable. Seems that it is only useful for a subset of unroutable routes (like completely non-existent resources, for example). If you look in the Gist you'll see that I work around this limitation by adding a "be_recognized" matcher which doesn't use "recognize_path()" under the hood. Instead it uses "assert_recognizes" and decides what to do based on what kind of exception, if any, gets thrown by it. So you can write: specify { get('/issues').should_not be_recognized } (Funnily enough, assert_recognizes() _does_ use recognize_path() under the hood, but it's evidently doing some additional set-up and tear-down to make it work. It seems like wrapping assert_recognizes rather than replicating its logic, though, is the right thing to do.) Cheers, Wincent
on 2010-07-07 13:32
On Jul 7, 2010, at 1:24 AM, Wincent Colaiuta wrote: >> end > > > > This is another back-to-front one, I think. You're not recognizing here, but generating. >> (see http://edgeapi.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html), > Yes, the example I posted at http://gist.github.com/464081 does that. > > in config/routes.rb: > If you look in the Gist you'll see that I work around this limitation by adding a "be_recognized" matcher which doesn't use "recognize_path()" under the hood. Instead it uses "assert_recognizes" and decides what to do based on what kind of exception, if any, gets thrown by it. So you can write: > > specify { get('/issues').should_not be_recognized } > > (Funnily enough, assert_recognizes() _does_ use recognize_path() under the hood, but it's evidently doing some additional set-up and tear-down to make it work. It seems like wrapping assert_recognizes rather than replicating its logic, though, is the right thing to do.) Seems as though this format has been abandoned in this conversation: it { should route(get "/issues/new").to("issues#new") } it { should generate("/issues/new").from("issues#new") } I think that verbiage is concise and intention revealing, and keeps the path on the left and the details on the right. Supported variants would be: it { should route(:get => "/issues/new") ... # explicit hash for method/path it { should route("/issues/new")... # defaults to get it { should ...to(:controller => "issues", :action => "new") } # explicit hash for params The negative version would be: it { should_not route(get "/issues/new") } We could alias route() with recognize() to get: it { should_not recognize(get "/issues/new") } We still need to consider output. For example: it { should route("/issues/37").to("issues#show", :id => 37) } I think in this case we _might_ be able to figure out that 37 is the value of :id and output should route "/issues/:id" to "issues#show" Not sure if that'd be asking for trouble, but it seems feasible/reasonable. In terms of the more verbose format for custom messages, we could add a router() method for sugar: it "routes /issues/new to issues#new" do router.should route("/issues/new").to("issues#new") } end And we still need the bi-directional version. I'm really at a loss on this one. Thoughts? David
on 2010-07-07 15:56
El 07/07/2010, a las 13:29, David Chelimsky escribió: > Seems as though this format has been abandoned in this conversation: > > it { should route(get "/issues/new").to("issues#new") } > it { should generate("/issues/new").from("issues#new") } > > I think that verbiage is concise and intention revealing, and keeps the path on the left and the details on the right. True, but it also looks like it might be a little painful to implement. Seeing as you propose a little further down that we should also be able to do stuff like: it { should_not route(get "/issues/new") } That means that "route" needs to be (or return) a matcher that also, optionally, responds to the "to" message and returns another matcher. And "generate" needs to return a proxy that responds to "from" and returns the real matcher, but itself raises an exception if the user mistakenly uses it without chaining the "from" onto it. Seems fairly magical compared to what we have right now, or what I proposed earlier on. > We could alias route() with recognize() to get: > > it { should_not recognize(get "/issues/new") } Sounds sensible. > We still need to consider output. For example: > > it { should route("/issues/37").to("issues#show", :id => 37) } > > I think in this case we _might_ be able to figure out that 37 is the value of :id and output > > should route "/issues/:id" to "issues#show" > > Not sure if that'd be asking for trouble, but it seems feasible/reasonable. I don't think it's worth it, to be honest. It would add a lot of complexity just to save some characters in the spec output. (Saving characters when _writing_ the specs seems more important.) And I think it's throwing away information, because the user reading the spec now has to mentally unpack the ":id" parameter, but doesn't actually see it. Multiple examples with different :ids in them are now all going to look the same. And what if the user has complex routes with requirements and regexp formats and stuff on them? Won't that be a little hard to handle? > In terms of the more verbose format for custom messages, we could add a router() method for sugar: > > it "routes /issues/new to issues#new" do > router.should route("/issues/new").to("issues#new") } > end Yeah, adding router() is a good idea. > And we still need the bi-directional version. I'm really at a loss on this one. Perhaps something with/like the "have_routing" that Trevor suggested earlier? Possibly: it { should have_routing('/issues/new').for('issues#new') } I've pasted a comparison of the two proposals in their current forms here so we can get a side-by-side (well, above/below) look at them: http://gist.github.com/466624 Still not convinced about the bi-directional language, though (used "have_routing" in both flavors). If anyone can come up with better language I'll update the comparison. For my taste, the first one reads nicer for most of the specs, but I like the "router.should_not recognize('foo')" syntax, at least for the GET case. I like it less for the non-GET case, though, where we have to nest another method call (ie. "router.should_not recognize(put '/issues/123')". Cheers, Wincent
on 2010-07-07 16:44
On Jul 7, 2010, at 7:39 AM, Wincent Colaiuta wrote: > it { should_not route(get "/issues/new") } > > That means that "route" needs to be (or return) a matcher that also, optionally, responds to the "to" message and returns another matcher. > > And "generate" needs to return a proxy that responds to "from" and returns the real matcher, but itself raises an exception if the user mistakenly uses it without chaining the "from" onto it. > > Seems fairly magical compared to what we have right now, or what I proposed earlier on. The chain method in matchers is intended just for this purpose: matcher :foo do |expected_foo| match do |actual| if @expected_bar # do one thing else # do another end end chain :bar do |expected_bar| @expected_bar = expected_bar end end thing.should foo('blah').bar('blah again') >> We could alias route() with recognize() to get: >> >> should route "/issues/:id" to "issues#show" >> >> Not sure if that'd be asking for trouble, but it seems feasible/reasonable. > > I don't think it's worth it, to be honest. It would add a lot of complexity just to save some characters in the spec output. (Saving characters when _writing_ the specs seems more important.) And I think it's throwing away information, because the user reading the spec now has to mentally unpack the ":id" parameter, but doesn't actually see it. Multiple examples with different :ids in them are now all going to look the same. And what if the user has complex routes with requirements and regexp formats and stuff on them? Won't that be a little hard to handle? Agreed. I was thinking of an earlier suggestion that the output should read in a more generic way, but that can be managed explicitly by the spec-author. > Perhaps something with/like the "have_routing" that Trevor suggested earlier? Possibly: > > it { should have_routing('/issues/new').for('issues#new') } > > I've pasted a comparison of the two proposals in their current forms here so we can get a side-by-side (well, above/below) look at them: > > http://gist.github.com/466624 > > Still not convinced about the bi-directional language, though (used "have_routing" in both flavors). If anyone can come up with better language I'll update the comparison. How about going back to map, with to_and_from: it { should map(get "/issues/new").to_and_from("issues#new") } ????? > > For my taste, the first one reads nicer for most of the specs, but I like the "router.should_not recognize('foo')" syntax, at least for the GET case. I like it less for the non-GET case, though, where we have to nest another method call (ie. "router.should_not recognize(put '/issues/123')". What don't you like about the nesting?
on 2010-07-07 17:25
El 07/07/2010, a las 16:42, David Chelimsky escribió: > On Jul 7, 2010, at 7:39 AM, Wincent Colaiuta wrote: > >> El 07/07/2010, a las 13:29, David Chelimsky escribió: > > How about going back to map, with to_and_from: > > it { should map(get "/issues/new").to_and_from("issues#new") } > > ????? "?????" is precisely what I think whenever I try to figure out the question of naming this particular thing... >> For my taste, the first one reads nicer for most of the specs, but I like the "router.should_not recognize('foo')" syntax, at least for the GET case. I like it less for the non-GET case, though, where we have to nest another method call (ie. "router.should_not recognize(put '/issues/123')". > > What don't you like about the nesting? Well, it's subtle, but it smells bad to me because it feels like unnecessary indirection. Instead of saying: this_thing.should satisfy(that_thing) I'm forced to say: this_thing.should satisfy(transform(that_thing)) ie. the nested method in there is transforming "that_thing" to make it suitable for consumption by the "satisfy" matcher. Seeing as we provide the "satisfy" matcher and we also define the requirements for its input, it feels somehow wrong to make the user feed the input through an additional, intermediate method. Why not just make the "satisfy" matcher accept the "that_thing" input as-is? Or any needed transformation itself? So this is why: it { should map(:get => '/issues/new').to_and_from('issues#new') } Feels "right", at least to me, compared to: it { should map(get '/issues/new').to_and_from('issues#new') } Which smells. Seeing as we provide the "map" matcher, at least in the common case of the GET request, we can apply the shortcut that you mentioned earlier and assume GET if not explicitly stated; eg. it { should map('/issues/new').to_and_from('issues#new') } Like I said, it's subtle. To me it feels ok to have an unnested transformer on the left side of the "should"; eg: specify { get('/issues/new').should route('issues#new') } But nesting it inside a matcher just sets off alarm bells for me. (Even worse than the "too much magic" alarm bells that ring for me when I see matcher chaining.) Cheers, Wincent
on 2010-07-08 04:57
OK, here is an idea. I was thinking about how to make routing tests
that make sense. I agree with Wincent that the Rails verbiage for the
routing tests is confusing, but what is NOT confusing is the new
routing format, so why not try out this format
(http://gist.github.com/467563):
describe 'routing another way' do
it { should have_resources(:days) }
it { should get('/days' => 'days#index') }
it { should match('/days' => 'days#index', :via => :get) }
it { should recognize('/days', :via => :get).as('days#index') }
it { should generate('days#index').from('/days', :via => :get) }
it { should recognize('/students/1/days', :via =>
:get).as('days#index', :student_id => '1') }
end
Notes:
it { should have_resources(:days) }
- tests all resource routes (index, show, etc.)
it { should get('/days' => 'days#index') }
- wrapper for assert_routing({:method => :get, :path => '/days'},
{:controller => 'days', :action => 'index'})
it { should match('/days' => 'days#index', :via => :get) }
- same as the previous example
it { should recognize('/days', :via => :get).as('days#index') }
- wrapper for assert_recognizes({:controller => 'days', :action =>
'index'}, {:method => :get, :path => '/days'})
it { should generate('days#index').from('/days', :via => :get) }
- wrapper for assert_generates({:method => :get, :path => '/days'},
{:controller => 'days', :action => 'index'})
it { should recognize('/students/1/days', :via =>
:get).as('days#index', :student_id => '1') }
- wrapper for assert_recognizes({:controller => 'days', :action =>
'index', :student_id => '1'}, {:method => :get, :path =>
'/students/1/days'})
The Rails test read backwards, look at this example from
http://edgeapi.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html:
# assert that POSTing to /items will call the create action on
ItemsController
assert_recognizes({:controller => 'items', :action => 'create'},
{:path => 'items', :method => :post})
The comment reads like the inverse of the actual method call. I think
that recognize(path).as(options) reads better (more like the comment
above).
The same goes for generate(options).from(path).
This approach gives us a very Rails-readable test and keeps it readable.
Thoughts?
Trevor
On Wed, Jul 7, 2010 at 11:00 AM, Wincent Colaiuta <win@wincent.com>
wrote:
>> ?????
>
> Â it { should map(:get => '/issues/new').to_and_from('issues#new') }
>
> rspec-users mailing list
> rspec-users@rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>
--
Trevor Lalish-Menagh
484.868.6150 mobile
trev@trevreport.org
http://www.trevmex.com/
on 2010-07-08 08:45
El 08/07/2010, a las 04:45, Lalish-Menagh, Trevor escribió: > it { should recognize('/days', :via => :get).as('days#index') } > it { should generate('days#index').from('/days', :via => :get) } > it { should recognize('/students/1/days', :via => > :get).as('days#index', :student_id => '1') } > end [snipped for brevity] > it { should generate('days#index').from('/days', :via => :get) } Hehe. You got it back-to-front again. "assert_generates" asserts that a _path_ is generated from a set of params (action, controller, additional params). Here you are doing the opposite, saying that certain params (action, controller) get generated from a path. > The Rails test read backwards, look at this example from > http://edgeapi.rubyonrails.org/classes/ActionDispatch/Assertions/RoutingAssertions.html: > # assert that POSTing to /items will call the create action on ItemsController > assert_recognizes({:controller => 'items', :action => 'create'}, > {:path => 'items', :method => :post}) > > The comment reads like the inverse of the actual method call. I think > that recognize(path).as(options) reads better (more like the comment > above). I think that's the general pattern of pretty much all Test::Unit assertions which compare two parameters. Where RSpec tends to say: actual.should == expected Test::Unit tends to say: assert expected, actual > Thoughts? Well, this comes back to the eternal question of specing things like validations. Where the spec ends up being an almost identical re-statement of the implementation: validates_presence_of :foo it { should validate_presence_of(:foo) } Are you testing the Rails validation code itself (which is already tested)? Are you testing that you remembered to include the validation itself? Do you trust your "validate_presence_of" matcher? Are you testing your ability to cut and paste? etc. Well, same questions can arise with routing when the implementation and spec look like this: resource :days it { should have_resource(:days) } So, when I see this kind of thing, I think _what_ are we trying to achieve in our routing specs and _why_? At least for me, the answers are: 1. Confirm: To confirm that the routes I've defined actually do what I think they do 2. Describe: To serve as a complete, explicit, readable specification of the behavior I expect from my routes And finally: 3. That the Rails routing code actually works as advertised This last one is a bit dubious, but the truth is the Rails 3 router is a complete re-write and bugs in it and incompatibilities with the old router are being found and squished constantly. An exhaustive set of routing specs can definitely help to uncover undiscovered edge cases. So, bearing in mind those goals, what I actually need from RSpec in order to achieve them is: - most importantly, a way of asserting that a user action (eg. HTTP verb + path + params) gets routed to a given controller + action with certain additional parameters (ie. a wrapper for assert_recognizes) - less importantly, a way of asserting that a set of params (action, controller, additional params) generates a particular path (ie. a wrapper for assert_generates); this is less important for me because in practice close to all (98%) of my URL generation is done with named URL helpers, and I can test those explicitly if I want - as syntactic sugar, a way of combining the above two assertions into one for those cases where the mapping is perfectly bidirectional (ie. a wrapper for assert_routing). With these tools I can achieve pretty much everything I need: not only test that user actions end up hitting the right spots in my application, but also specify clearly and explicitly what I expect those mappings to be. So for me, anything else doesn't really help me achieve my goals. The "have_resource(s)" matcher, for instance, doesn't help me and in fact actually undermines my goal of providing a complete and explicit specification of how my routes work. The "recognize" and "generate" matchers you suggest obviously are "on target" to help me fulfill my goals. Of these, the "recognize" one is the most important one though. Of the "match" and "get" matchers you suggest, seeing as they both wrap the same thing (assert_routing), one of them would have to go. I'd probably ditch "match" because it is just a repetition of the router DSL method of the same name, and my goal here isn't just to repeat that DSL in my specs. In fact, if you look at my most important goal -- asserting that a user action (HTTP verb, path, params) hits a specific end point (controller + action + additional params) -- you'll understand why, in my proposal, all of my specifications start with "get"/"post" etc followed by path, params and then controller/action. It's not just for resonance with other parts of RSpec like where we use "get" etc in controller specs. But if the goal is just to wrap the 3 Rails assertions as faithfully as possible, then you wind up with a different proposal. What David has posted is probably the closest to this goal. And if the goal is make the specs map as closely as possible onto the language of config/routes.rb, then you wind up with a different proposal still... (ie. the one you just made). So, I guess what I'm saying here is I'd like to hear _what_ people are wanting to achieve in their routing specs and _why_; and then to ask what kind of means RSpec should provide to best achieve those goals. Cheers, Wincent
on 2010-08-17 19:47
Lots of good proposals here. I would at least like to chime in that I agree with the goals as set out by Wincent below, and from a readability standpoint have preferred the API as suggested by Trevor. -Michael
on 2010-09-29 13:57
On Aug 17, 2010, at 12:46 PM, Michael Kintzer wrote: >> 3. That the Railsroutingcode actually works as advertised >> >> But if the goal is just to wrap the 3 Rails assertions as faithfully as possible, then you wind up with a different proposal. What David has posted is probably the closest to this goal. >> >> And if the goal is make the specs map as closely as possible onto the language of config/routes.rb, then you wind up with a different proposal still... (ie. the one you just made). >> >> So, I guess what I'm saying here is I'd like to hear _what_ people are wanting to achieve in theirroutingspecs and _why_; and then to ask what kind of means RSpec should provide to best achieve those goals. >> >> Cheers, >> Wincent > Lots of good proposals here. I would at least like to chime in that I > agree with the goals as set out by Wincent below, and from a > readability standpoint have preferred the API as suggested by > Trevor. > > -Michael Time to resurrect this discussion. Thinking about Wincent's very thoughtful synopsis has lead me to a slightly different line of thinking here. We've always seen route recognition and route generation as opposite sides of the same coin, but I think that's due to how they are presented to us by the Rails testing facilities. The more I think of it, route generation is not really the concern of the router at all. It's a responsibility of controllers (and views, by proxy) to generate a recognizable route from a set of params. What this leads me to is that I'd like to simplify all of this and make route_to only care about the routing side of this equation (i.e. have it delegate to assert_recognizes, rather than assert_routing). The implication of this would be that we'd theoretically lose coverage that routes are generated correctly, but I don't think that actually matters. We still need to assemble the right parameters in our controllers/views to get the routes we want, so those should be specified in relation to the controller or view. i.e. in a view spec: rendered.should have_css("a", :href => projects_path) The fact that the controller can generate that link from {:controller => "projects", :action => "index"} is a matter of Rails working as advertised. Plus, if we wrote that link in a view and it generated an unrecognizable route, the error message we'd get is that no route matches, not that the route could not be generated. To sum up: I'd like to have route_to delegate to assert_recognizes, and leave it at that. That said, I'd be perfectly willing to add some of the sugar that has been proposed in this thread, so we can say: get("/").should route_to("projects#index") I agree that this is more terse, yet very expressive in the context of other facilities we're dealing with. Thoughts?
Please log in before posting. Registration is free and takes only a minute.
Existing account
(Switch to SSL-encrypted connection)
NEW: Do you have a Google/GoogleMail or Yahoo account? No registration required!
Log in with Google account | Log in with Yahoo account
Log in with Google account | Log in with Yahoo account
No account? Register here.