El 05/07/2010, a las 13:56, David Chelimsky escribió:
> 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.
Well, I'm not sure if there is a problem with the Rails assertions. In my
(albeit limited) experience I haven't yet seen a situation where they couldn't
express something during testing. So as you point out, the job of RSpec should
just be to expose those APIs within specs in a usable and effective manner. If
we discover some limitation that we can't get around, then sure, the upstream
assertions will need some work.
> I think having three separate matchers would land us in a good spot.
Agreed. That's why I added a matcher for each of assert_recognizes,
assert_generates, and assert_routing (although so far in practice I've only had
to use two of them).
> 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(....)
Yes, in my initial implementation I left that out seeing as I haven't yet run
into a spec that needed it, but I did have it in mind when I wrote the "get"
etc methods (in fact, when I first typed them in I started with 'def get path,
options = {}', but then thought, YAGNI, better not add that in until I am sure
I need it...)
> 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
Yes, the "action#controller" notation is a good idea. I actually added that to
the gist that I posted already.
Not sure about nesting the get() method inside the matcher though. It seems
that in most other places, we use hashes or literal values; eg:
something.should do_something(:when => 'always', :how => 'quickly')
So, it would be more consistent with existing style to write:
it { should recognize(:get => '/wiki/foo').as('wiki#show', :id => 'foo') }
But then we lose some of the conciseness I was looking for in my original
proposal. In any case we also lose the similarity with controller specs (where
"get" is the first thing on the line) which I was also looking for.
Nevertheless, your choice of verbs and prepositions work well together;
"recognize as", "generate from" and "route to" all read fairly well.
Just to clarify, are you suggesting that:
- "recognize as" wrap assert_recognizes?
- "generate from" wrap assert_generates?
- "route to" wrap assert_routing?
If so, one misgiving I have is that we still have a unidirectional preposition
("to") being used in conjunction with the bidirectional assertion.
An interesting consequence of the language you propose is that the focus of
testing changes ie. in the current generator templates, we have:
describe FooController do
describe 'routing' do
it 'recognizes and generates #index' do
{ :get => 'bar' }.should route_to(:controller => 'bar', :action =>
'index')
So the focus is on controller actions, and the subject of each 'it' block is a
literal hash representing a request to that controller and action.
The new language puts the router at the center and the implicit subject of each
'it' block is really the router itself.
> 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?
I think it would be nicer to format that as "GET /wiki/foo" (ie. HTTP verb
followed by path) rather than embedding the literal text of the
"get('/wiki/foo')" method call.
In any case, I converted another RESTful resource over to the syntax I
mentioned earlier, now using the "controller#action" syntax:
describe IssuesController do
describe 'routing' do
example 'GET /issues' do
get('/issues').should map('issues#index')
end
example 'GET /issues/new' do
get('/issues/new').should map('issues#new')
end
example 'GET /issues/:id' do
get('/issues/123').should map('issues#show', :id => '123')
end
example 'GET /issues/:id/edit' do
get('/issues/123/edit').should map('issues#edit', :id => '123')
end
example 'PUT /issues/:id' do
put('/issues/123').should map('issues#update', :id => '123')
end
example 'DELETE /issues/:id' do
delete('/issues/123').should map('issues#destroy', :id => '123')
end
end
end
Which, if you really care about conciseness, can be written as:
describe IssuesController do
describe 'routing' do
it { post('/issues').should map('issues#create') }
it { get('/issues').should map('issues#index') }
it { get('/issues/new').should map('issues#new') }
it { get('/issues/123').should map('issues#show', :id => '123') }
it { get('/issues/123/edit').should map('issues#edit', :id => '123') }
it { put('/issues/123').should map('issues#update', :id => '123') }
it { delete('/issues/123').should map('issues#destroy', :id => '123') }
end
end
These get formatted in the spec output as:
IssuesController
routing
GET /issues
GET /issues/new
GET /issues/:id
GET /issues/:id/edit
PUT /issues/:id
DELETE /issues/:id
And:
IssuesController
routing
should map "issues#create"
should map "issues#index"
should map "issues#new"
should map "issues#show" and {:id=>"123"}
should map "issues#edit" and {:id=>"123"}
should map "issues#update" and {:id=>"123"}
should map "issues#destroy" and {:id=>"123"}
Respectively. By adding a description method to the matcher (see updated Gist:
http://gist.github.com/464081), we can produce:
IssuesController
routing
should map POST /issues as issues#create
should map GET /issues as issues#index
should map GET /issues/new as issues#new
should map GET /issues/123 as issues#show with {:id=>"123"}
should map GET /issues/123/edit as issues#edit with {:id=>"123"}
should map PUT /issues/123 as issues#update with {:id=>"123"}
should map DELETE /issues/123 as issues#destroy with {:id=>"123"}
Which IMO looks pretty nice.
Cheers,
Wincent
_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users
