Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Consistent request spec file naming #2376

Closed
wants to merge 3 commits into from
Closed

Consistent request spec file naming #2376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d3ab909
Use consistent naming for request specs
eloyesp Jun 17, 2020
9f0bc1f
Changelog entry for #2355
klyonrad Aug 21, 2020
b1e05e0
Reduce dependencies between smoke tests for request spec generators
klyonrad Aug 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ Enhancements:

* Issue a warning when using job matchers with `#at` mis-match on `usec` precision.
(Jon Rowe, #2350)
* Generate request specs with consistent file naming, without `request_spec` suffix.
(Eloy Espinaco, #2355)

Bug Fixes:

Expand Down
8 changes: 6 additions & 2 deletions example_app_generator/generate_stuff.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,12 @@ def using_source_path(path)
generate('rspec:install')
generate('controller wombats index') # plural
generate('controller welcome index') # singular
generate('rspec:request wombats')
generate('integration_test widgets')
# Generate controllers so that Rails generates routes
generate('controller logins index --no-request_specs --no-view_specs --no-helper_specs')
generate('controller signups index --no-request_specs --no-view_specs --no-helper_specs')
# The generated specs from IntegrationGenerator rely on index routes being present
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be addressed again in #2375

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You wouldn't have to do the above if you stick to wombats and widgets? Are these changes even necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you seen the test output for the commit before that 9f0bc1f?

The IntegrationSpecGenerator / RequestSpecGenerator generate a file with an index_path helper that does not exist when we have not generated the corresponding route yet, causing a failure of the smoke tests. So there was a hidden dependency between generating the controllers and testing the generated request specs. That I think you understood, right?
This was a very surprising dependency by the way. Dependencies in this generates_stuff file are quite the pain generally. And I assume that these test failures were what motived @eloyesp to change the behaviour in the previous PR.

When we stick to wombats and widgets, then we would try to overwrite the already generated request spec files from the earlier lines. generate('controller wombats index') generates spec/requests/wombats_spec.rb. Then generate('rspec:request wombats') was also trying to generate spec/requests/wombats_spec.rb, so a conflict. We could try using the --force option to just ignore that file conflict but I thought that then it would be pointless to even use the request spec generator here.

The usage of --no-request_specs is necessary so that generating the controller does not change the conflicting file name. The usage --no-view_specs --no-helper_specs options I just added to avoid unnecessary increase in testing time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are controllers generated above, if they are missing a route then ok add some more, but don't remove the old ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what you mean.
Could you please checkout the commit locally, run rake smoke:app, see the failures and tell more detailed how you would solve them?

I did not remove anything. I changed the names for using the request spec generation and the integration_test generation.

  • generate('rspec:request wombats') fails because generate('controller wombats index') already has generated the request spec file. This was not an issue before because generate('rspec:request wombats') generated spec/requests/wombats_request_spec file. And this file worked only because generate('controller wombats index') created the correct route.
  • generate('integration_test widgets') would work, but the file spec/requests/widgets_spec will later on be overwritten by generate('scaffold widget name:string category:string instock:boolean foo_id:integer bar_id:integer --force')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I pulled down your branch and ran it and the main issue is that the generator now overwrites itself due to the change in defaults, fixed in #2378

generate('rspec:request logins')
generate('integration_test signups')
generate('mailer Notifications signup')

generate('model thing name:string')
Expand Down
2 changes: 1 addition & 1 deletion lib/generators/rspec/controller/controller_generator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def generate_request_spec
return unless options[:request_specs]

template 'request_spec.rb',
File.join('spec/requests', class_path, "#{file_name}_request_spec.rb")
File.join('spec/requests', class_path, "#{file_name}_spec.rb")
end

def generate_controller_spec
Expand Down
4 changes: 2 additions & 2 deletions spec/generators/rspec/controller/controller_generator_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
setup_default_destination

describe 'request specs' do
subject { file('spec/requests/posts_request_spec.rb') }
subject { file('spec/requests/posts_spec.rb') }

describe 'generated by default' do
before do
Expand Down Expand Up @@ -38,7 +38,7 @@
end

describe 'with namespace and actions' do
subject { file('spec/requests/admin/external/users_request_spec.rb') }
subject { file('spec/requests/admin/external/users_spec.rb') }

before do
run_generator %w[admin::external::users index custom_action]
Expand Down