Re: Possible improvements to routing spec API

OK, I will chime in here, since I think I might have opened up this
can of worms. :slight_smile:

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:

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

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 C. [email protected]
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
[email protected]
http://www.trevmex.com/

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 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 Jul 7, 2010, at 1:24 AM, Wincent C. 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 Jul 7, 2010, at 7:39 AM, Wincent C. 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?

El 07/07/2010, a las 13:29, David C.
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

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 C. [email protected]
wrote:

???

 it { should map(:get => ‘/issues/new’).to_and_from(‘issues#new’) }

rspec-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users


Trevor Lalish-Menagh
484.868.6150 mobile
[email protected]
http://www.trevmex.com/

El 07/07/2010, a las 16:42, David C.
escribió:

On Jul 7, 2010, at 7:39 AM, Wincent C. wrote:

El 07/07/2010, a las 13:29, David C. 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

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

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:

  1. 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 Aug 17, 2010, at 12:46 PM, Michael K. wrote:

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

This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.

| Privacy Policy | Terms of Service | Remote Ruby Jobs