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
_______________________________________________
rspec-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users
