Forum: RSpec Re: Possible improvements to routing spec API

Posted by Lalish-Menagh, Trevor (Guest)
on 2010-07-05 22:00
(Received via mailing list)
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
Posted by David Chelimsky (Guest)
on 2010-07-06 15:16
(Received via mailing list)
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
Posted by Lalish-Menagh, Trevor (Guest)
on 2010-07-07 01:17
(Received via mailing list)
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/
Posted by Wincent Colaiuta (Guest)
on 2010-07-07 08:43
(Received via mailing list)
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
Posted by David Chelimsky (Guest)
on 2010-07-07 13:32
(Received via mailing list)
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
Posted by Wincent Colaiuta (Guest)
on 2010-07-07 15:56
(Received via mailing list)
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
Posted by David Chelimsky (Guest)
on 2010-07-07 16:44
(Received via mailing list)
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?
Posted by Wincent Colaiuta (Guest)
on 2010-07-07 17:25
(Received via mailing list)
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
Posted by Lalish-Menagh, Trevor (Guest)
on 2010-07-08 04:57
(Received via mailing list)
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/
Posted by Wincent Colaiuta (Guest)
on 2010-07-08 08:45
(Received via mailing list)
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
Posted by Michael Kintzer (Guest)
on 2010-08-17 19:47
(Received via mailing list)
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
Posted by David Chelimsky (Guest)
on 2010-09-29 13:57
(Received via mailing list)
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
No account? Register here.