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
(new_rspec_routing.rb · GitHub):
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 forums_routing_spec.rb · GitHub 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
