On Jul 5, 2010, at 4:14 AM, Matt Wynne wrote:
> On 5 Jul 2010, at 08:00, Wincent Colaiuta wrote:
>> Hi folks,
>> 
>> I've been unhappy with routing specs for a long time now and last night when 
>> updating some old 1.3 specs for 2.0 I decided to see if I could come up with 
>> something that didn't make me feel unhappy.
>> 
>> Principal causes of unhappiness:
>> 
>> 1. Historically we had "route_for" and "params_from", which felt awfully 
>> repetitive because we ended up doing:
>> 
>> route_for(lengthy_hash_of_params).should == 
>> string_or_hash_describing_destination
>> params_from(list_describing_destination).should == lengthy_hash_of_params
>> 
>> Of course, it was worse than that in practice because those two lines 
>> usually appeared in separate example blocks with the associated boilerplate. 
>> It felt like a lot of work for testing such a simple thing. It also felt 
>> irritating to have to repeat basically the same thing twice but in a 
>> different order.
>> 
>> 2. So then RSpec gave us "route_to", which is a wrapper for Rails' 
>> "assert_routing"; being a bi-directional test that encompasses the function 
>> of both "assert_recognizes" and "assert_generates", this allows us to avoid 
>> some, or even all, of the repetition:
>> 
>> { :get => 'foo' }.should route_to(:controller => 'foo', :action => 'index')
>> 
>> The unhappiness here comes from three causes:
>> 
>> One is that { :get => 'foo' } feels inconsistent with other places in RSpec 
>> like controller specs where "get" is a method, so we can do things like "get 
>> 'thing'".
>> 
>> The second issue is that the "to" in "route_to" feels misleadingly 
>> uni-directional when in reality it is a bi-directional test.
>> 
>> The third issue is that for routes which don't actually have that 
>> bi-directional mapping, "route_to" can't be used and we must instead drop 
>> down to Rails' assert_recognizes() and/or assert_generates() methods, or 
>> wrap them using our own matchers.

This is the source of confusion/trouble that others have brought up here on 
this list as well. Thanks for identifying the underlying issue.

>> So I thought about what I would rather be writing and in my first cut came 
>> up with this:
>> 
>> describe ArticlesController do
>>   describe 'routing' do
>>     example 'GET /wiki' do
>>       get('/wiki').should   map_to(:controller => 'articles', :action => 
>> 'index')
>>       get('/wiki').should map_from(:controller => 'articles', :action => 
>> 'index')
>>       articles_path.should == '/wiki'
>>     end
>>   end
>> end
>> 
>> Things to note:
>> 
>> - make the bi-directionality of the mapping explicit by having separate 
>> "map_to" and "map_from" lines.
>> 
>> - for ease of readability and writability, keep the order as "method -> path 
>> -> destination" for both assertions by using "to" and "from", rather than 
>> swapping the order around
>> 
>> - "map" here is the right verb because we've always used that language to 
>> talk about how a given URL "maps to" a given controller#action. And remember 
>> how in the router DSL prior to Rails 3 everything in config/routes.rb 
>> started with "map"?
>> 
>> - I've tacked a test for the "articles_path" URL helper in there, because as 
>> a user of the Rails router I generally want to know two things: firstly, 
>> that requests get mapped to the appropriate controller#action; and secondly, 
>> that when I generate URLs (almost exclusively with named helpers; I use 
>> "url_for" in only 4 places in my entire app) that they take me where I think 
>> they take me. In the end, however, I moved this into a separate "describe 
>> 'URL helpers'" block.
>> 
>> - conscious use of "example" rather than "it" because I want my specs to be 
>> identified as "ArticlesController routing GET /wiki" and not 
>> "ArticlesController routing recognizes and generates #index".
>> 
>> - the repetition is a conscious choice because I value 
>> readability/scannability over DRYness-at-all-costs, especially in specs; the 
>> following is more DRY, for example, but less readable/scannable:
>> 
>> path = '/wiki'
>> destination = { :controller => 'articles, :action => 'index' }
>> get(path).should map_to(destination)
>> get(path).should map_from(destination)
>> 
>> So I went ahead and converted a bunch of specs to this syntax and found 
>> that, surprise, surprise, in an application like this one where almost 
>> everything consists of a "standard" RESTful resource, over 90% of routes 
>> were testable in the bi-directional sense and in a typical routing spec file 
>> I needed to use "map_to" with no corresponding "map_from" for only one or 
>> two cases. So I needed a new method that meant "map_to_and_from".
>> 
>> Funnily, I just can't decide on a name for this method. As a placeholder I 
>> am just using "map" for now:
>> 
>> get('/wiki').should map(:controller => 'articles', :action => 'index')
>> 
>> But others I have tried are:
>> 
>> get('/wiki').should map_as(:controller => 'articles', :action => 'index')
>> get('/wiki').should map_via(:controller => 'articles', :action => 'index')
>> get('/wiki').should map_with(:controller => 'articles', :action => 'index')
>> get('/wiki').should map_to_and_from(:controller => 'articles', :action => 
>> 'index')
>> get('/wiki').should map_both(:controller => 'articles', :action => 'index')
>> get('/wiki').should map_both_ways(:controller => 'articles', :action => 
>> 'index')
>> get('/wiki').should have_routing(:controller => 'articles', :action => 
>> 'index')
>> get('/wiki').should have_route(:controller => 'articles', :action => 'index')
>> get('/wiki').should be_route(:controller => 'articles', :action => 'index')
>> get('/wiki').should be_routing(:controller => 'articles', :action => 'index')
>> get('/wiki').should route_as(:controller => 'articles', :action => 'index')
>> get('/wiki').should route_via(:controller => 'articles', :action => 'index')
>> get('/wiki').should route(:controller => 'articles', :action => 'index')
>> get('/wiki').should <=> (:controller => 'articles', :action => 'index')
>> get('/wiki').should > (:controller => 'articles', :action => 'index') # 
>> map_to
>> get('/wiki').should < (:controller => 'articles', :action => 'index') # 
>> map_from
>> 
>> If anybody has a suitable suggestion please let me know.

Naming is hard.

>> In the meantime, here is an example of a spec file that has been converted 
>> to use this new "API":
>> 
>> http://gist.github.com/464081

Nice overall. Much of the code belongs in Rails, though, so I'd like to try to 
get a patch in to Rails once we get this worked out. I'd like the rspec-rails 
matchers to be simple wrappers for rails' assertions wherever possible.

>> It also includes the supporting code that adds these new "map", "map_to", 
>> "map_from" matchers, and the "get", "post", "put" and "delete" methods. All 
>> of this for Rails 3/RSpec 2 only.
>> 
>> I'm going to convert more routing specs and see if any more changes are 
>> needed to handle edge cases, but for a first cut I am pretty happy with the 
>> results, apart from my inability to decide on the right name for the 
>> bi-directional "map" matcher.

I think having three separate matchers would land us in a good spot. The 
wrapped rails assertions also accept a hash representing query params, so let's 
see if we can accommodate that as well:

  get('/something', :ref_id => '1234').should route_to(....)

>> 
>> Cheers,
>> Wincent
> 
> Seems like progress. One thought: Why not use a macro-style syntax to 
> eliminate the boring boilerplate call to #it / #example and generate examples 
> instead?

Something like:

  describe "The router" do
    it { should recognize(get('/wiki/foo').as("wiki#show", :id => 'foo') }
    it { should generate(get('/wiki/foo').from("wiki#show", :id => 'foo') }
    it { should route(get('/wiki/foo').to("wiki#show", :id => 'foo') }
  end

That's definitely nice and clean, and it makes it easier to align the names 
with the rails assertions, which I think adds some value. The output could be 
formatted something like this:

  The router
    should recognize get('/wiki/foo') as wiki#show with :id => 'foo'
    should generate get('/wiki/foo') from wiki#show with :id => 'foo'
    should route get('/wiki/foo') to wiki#show with :id => 'foo'

Thoughts?
_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users

Reply via email to