[rspec-users] Possible improvements to routing spec API

Lalish-Menagh, Trevor trev at trevreport.org
Wed Jul 7 22:45:32 EDT 2010


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 at wincent.com> wrote:
> 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
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>



-- 
Trevor Lalish-Menagh
484.868.6150 mobile
trev at trevreport.org
http://www.trevmex.com/


More information about the rspec-users mailing list