On Jul 7, 2010, at 1:24 AM, Wincent Colaiuta wrote:
> 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.)
Seems as though this format has been abandoned in this conversation:
it { should route(get "/issues/new").to("issues#new") }
it { should generate("/issues/new").from("issues#new") }
I think that verbiage is concise and intention revealing, and keeps the path on
the left and the details on the right. Supported variants would be:
it { should route(:get => "/issues/new") ... # explicit hash for method/path
it { should route("/issues/new")... # defaults to get
it { should ...to(:controller => "issues", :action => "new") } # explicit
hash for params
The negative version would be:
it { should_not route(get "/issues/new") }
We could alias route() with recognize() to get:
it { should_not recognize(get "/issues/new") }
We still need to consider output. For example:
it { should route("/issues/37").to("issues#show", :id => 37) }
I think in this case we _might_ be able to figure out that 37 is the value of
:id and output
should route "/issues/:id" to "issues#show"
Not sure if that'd be asking for trouble, but it seems feasible/reasonable.
In terms of the more verbose format for custom messages, we could add a
router() method for sugar:
it "routes /issues/new to issues#new" do
router.should route("/issues/new").to("issues#new") }
end
And we still need the bi-directional version. I'm really at a loss on this one.
Thoughts?
David
_______________________________________________
rspec-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users
