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.

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

Already on GitHub? Sign in to your account

Update and discourage use of controller specs #113

Open
corroded opened this issue Aug 14, 2020 · 8 comments
Open

Update and discourage use of controller specs #113

corroded opened this issue Aug 14, 2020 · 8 comments
Labels
help wanted Beginner-friendly contribution

Comments

@corroded
Copy link

The guide still has a section for controller specs. I think it should be mentioned that they have been deprecated and discouraged since Rails 3 (or was it 4?). I reckon the guidelines can probably stay for legacy stuff, but a suggestion mentioning request specs might be a good idea.

@pirj
Copy link
Member

pirj commented Aug 14, 2020

Absolutely. The guide is behind the reality in this regard. There's a mention of request specs here https://github.com/rubocop-hq/rspec-style-guide/#rails-integration, but the problem with slow request specs is long gone.

I'm against of keeping controller spec guidelines and recommending request specs over controller specs at the same time. It's time for the change.

Would you like to take a stab at it?

Useful resources:

@pirj pirj added the help wanted Beginner-friendly contribution label Aug 14, 2020
@andyw8
Copy link
Contributor

andyw8 commented Aug 14, 2020

For reference, it was Rails 5 when controller tests were deprecated:

https://blog.bigbinary.com/2016/04/19/changes-to-test-controllers-in-rails-5.html

@pirj
Copy link
Member

pirj commented Aug 14, 2020

Also this rails/rails#22496

@rpbaptist
Copy link

For reference, it was Rails 5 when controller tests were deprecated:

I think this is an overstatement leading to current misunderstandings. ActionController::TestCase is deprecated, which includes assigns and assert_template. There's more to controller testing than those methods.

From the blog post you linked:

According to Rails team, controller tests should be more concerned about what is the result of that controller action like what cookies are set, or what HTTP code is set rather than testing of the internals of the controller. So, these methods are removed from the core.

From the original issue: rails/rails#22076

ActionController::TestCase is deprecated. Let's get the controller test generation using our ActionDispatch::IntegrationTest future instead.

So controller tests are still around. It's just not testing which templates are rendered and which instance variables are set.

@pirj
Copy link
Member

pirj commented Aug 20, 2020

Agree, functional controller tests (that map to RSpec Rails' Controller specs) are said to use ActionDispatch::IntegrationTest in Rails's testing guide, not ActionController::TestCase::Behavior as RSpec Rails states.

This needs to be fixed in RSpec Rails' docs and reflected in the guideline as well.
It's just those two, asssigns and assert_template that were removed/extracted:

"Removed assigns and assert_template. Both methods have been extracted into the rails-controller-testing gem (Pull Request)" (https://github.com/rails/rails/blob/eca6c273fe2729b9634907562c2717cf86443b6b/guides/source/5_0_release_notes.md#L319).

@pirj
Copy link
Member

pirj commented Aug 24, 2020

@klyonrad
Copy link

So wait a second, either there is some massive hair splitting going on or all this controller vs request specs thing is based on misunderstandings.

What is supposed to be the difference between controller tests and integration tests in a rails non-rspec test suite?

They both are using the same class
https://github.com/rails/rails/blob/master/railties/lib/rails/generators/test_unit/controller/templates/functional_test.rb.tt
https://github.com/rails/rails/blob/master/railties/lib/rails/generators/test_unit/integration/templates/integration_test.rb.tt

@pirj
Copy link
Member

pirj commented Oct 6, 2020

Nice find, @klyonrad!

So, from the Rails Guidelines on Functional Tests (RSpec's Controller Specs):

testing how your actions handle the requests and the expected result or response, in some cases an HTML view.
You should test for things such as:
was the web request successful?
was the user redirected to the right page?
was the user successfully authenticated?
was the appropriate message displayed to the user in the view?
was the correct information displayed in the response?

and for Integration Tests (RSpec's request specs):

test how various parts of your application interact. They are generally used to test important workflows within our application.
testing the response of our request by asserting the presence of key HTML elements and their content.

So, if I understand this correctly, those tests have the same capabilities, but they are intended to test different things in different ways.

  1. It's possible to stub things for controller specs, but not recommended for request specs.
  2. Subsequent HTTP operations don't reinitialize the controller in controller specs (at least last time I checked with Rails 4.2).

Those two types of specs have access to the same matchers (apart from deprecated assigns/assert_template available in the controller specs).

From my point of view, the difference is subtle, and there's no reason not to use request specs to check cookies, headers, response status code, redirects etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Beginner-friendly contribution
Projects
None yet
Development

No branches or pull requests

5 participants