Create controller generator for routing specs #1806
Conversation
|
@00dav00 Thanks for this PR! This looks fine to me and I like most of the PR implementation, I do have a few notes. Before we go ahead with this I need to think about whether or not this is a feature we want to support going forward. I rarely if ever use a routing spec. Do you personally find a lot of value from them? |
|
@samphippen I use routing specs for API endpoints since they should follow the specification and not change. |
| class_option :view_specs, :type => :boolean, :default => true | ||
| class_option :controller_specs, :type => :boolean, :default => true, :desc => "Generate controller specs" | ||
| class_option :view_specs, :type => :boolean, :default => true, :desc => "Generate view specs" | ||
| class_option :routing_specs, :type => :boolean, :default => true, :desc => "Generate routing specs" |
There was a problem hiding this comment.
maybe we could have default : false ?
|
@samphippen I agree with @seuros, this specs can be useful when the routes does not follow rest conventions. |
|
@JonRowe what about having the option |
|
@JonRowe I think @seuros 's option may be a good fit, I mean not adding them as default. WDYT @samphippen ? |
|
I noticed routing specs are being created by the scaffold generator. So isn't this contribution helping us have a sense of completeness? I think this could work but in any case, I'm also open to not having it by default. WDYT? @JonRowe @samphippen |
|
Hi @00dav00 I've found some time to come back to this. I think I like this overall, but I have some questions:
|
|
Hi @samphippen, thanks for puting some time into this, highly appreciated.
If
The scaffold generator is creating the routing specs by default. In opposition with the current PR, the scaffold generator will only skip the routing specs if the flag |
|
@samphippen This has been here for a while now, do you think we will be able to move it forward or you consider it is not required? |
| class_option :view_specs, :type => :boolean, :default => true | ||
| class_option :controller_specs, :type => :boolean, :default => true, :desc => "Generate controller specs" | ||
| class_option :view_specs, :type => :boolean, :default => true, :desc => "Generate view specs" | ||
| class_option :routing_specs, :type => :boolean, :default => false, :desc => "Generate routing specs" |
There was a problem hiding this comment.
Double spacing here, unless you were intending to align the :desc, in which case the latst needs a space removed.
|
|
||
| def generate_routing_spec | ||
| return unless options[:routing_specs] | ||
|
|
There was a problem hiding this comment.
This should probably return is actions is empty like views...
| require 'rails_helper' | ||
|
|
||
| <% module_namespacing do -%> | ||
| RSpec.describe <%= class_name %>Controller, <%= type_metatag(:routing) %> do |
There was a problem hiding this comment.
This describe should be a doc string, we don't need the constant here.
|
Closing this one in favor of #2134 |
This PR covers #1160
This creates routing specs for controller generator
/cc @rspec/rspec