We were having an internal discussion about this.
Looking at https://github.com/rspec/rspec-rails/pull/1839 and considering the documentation has not been changed since it leaves me with questions.
Use request specs to describe the client-facing behavior of the application
Starting background jobs or sending notification mails is not client-facing behaviour. So that should not be covered in request specs. So far so good.
Controller specs can be used to describe the behavior of Rails controllers. As of version 3.5, however, controller specs are discouraged in favor of request specs (which also focus largely on controllers, but capture other critical aspects of application behavior as well).
And here it's stating we should be using request specs for everything previously tested in conrollers. So I am wondering what the recommendation is for non client-facing behaviour. If controller tests are not to be used, and request specs should not test client-facing behavior, then where would you recommend testing them?
This is the kind of tests I currently use for controllers:
expect { post(:create, params: {}) }.to have_enqueued_job(DoSomethingJob).with(model, foo: :bar)
That's a question that was bothering me as well for a long time.
After all, RSpec Rails is a shim on top of Rails' testing tools. I'm not aware and couldn't find anything reasonable on this topic, including in Rails' own testing guide.
Do you think it makes sense to start a discussion on this topic in the Rails discussion group.
We'll be happy to adjust our recommendations/documentation on this topic to reflect Rails' vision of how this should be done.
Related: https://github.com/rubocop-hq/rspec-style-guide/issues/113 - I've been postponing working on this since there's lack of information/best practices around that.
It's confusing why we mention ActionController::TestCase::Behavior instead of ActionDispatch::IntegrationTest that Rails's testing guide mentions as the base for controller specs/functional controller tests.
ControllerExampleGroup includes ActionController::TestCase::Behavior, while ActionDispatch::Integration::Runner is included in request and system spec groups only. @JonRowe do you have some recollection on this topic?
I can dig the changes and find the reasoning behind the current state of affairs.
Do you think it makes sense to start a discussion on this topic in the Rails discussion group.
I think this is a good idea :)
Starting background jobs or sending notification mails is not client-facing behaviour. So that should not be covered in request specs. So far so good.
I disagree here, sending notification emails is certainly client facing behaviour, and a background job triggered by a client might not have a visible side effect but is still a side effect of client facing behaviour.
It's confusing why we mention ActionController::TestCase::Behavior instead of ActionDispatch::IntegrationTest that Rails's testing guide mentions as the base for controller specs/functional controller tests.
That link was last updated in 2011, might be time to update it.
ControllerExampleGroup includes ActionController::TestCase::Behavior, while ActionDispatch::Integration::Runner is included in request and system spec groups only. @JonRowe do you have some recollection on this topic?
I can dig the changes and find the reasoning behind the current state of affairs.
I suspect thats related to controller tests being provided by the older Rails / rails-controller-test gem where as our other end-to-end tests use the newer runner.
I disagree here, sending notification emails is certainly client facing behaviour, and a background job triggered by a client might not have a visible side effect but is still a side effect of client facing behaviour.
There are other background jobs than sending email to the user. It could be an email to the company staff of the application which the client will never notice. It could be a call to an external API of which the result is invisible to the client.
If your stance is to test this in integration tests, that's one thing, but you can't possibly call them client-facing.
Client doesn't mean human.
Client doesn't mean human.
I am aware.
I'm open to improving the wording, but client facing here means "behaviour triggered by the outside world".
My understanding of client-facing is the provided API, whether that's a JSON endpoint or a form in a HTML page. My interpretation of "client-facing behavior" is any behavior which is communicated to that client. Not just triggered by the client.
I could be wrong. I have been searching for a clear definition of the term, but it's not easy to find.
Given that, I still would not check for background jobs being executed in system tests, which I definitly consider to be user-facing.
But I don't really see an advantage in moving tests of side-effects from the controller to a request spec. Or would you advocate checking side-effects in system tests?
But I don't really see an advantage in moving tests of side-effects from the controller to a request spec. Or would you advocate checking side-effects in system tests?
Ah you are missing a few pieces of information here, which comes back to my point about improving the documentation.
rspec-rails is a thin wrapper around Rails own test helpers to enable them to be used with RSpec. Controller specs were a wrapper around controller tests which have been deprecated by the Rails team, and thus the wrapper is deprecated by us.
Request specs do largely the same job as controller specs, the main difference being they run through the entirety of the rack stack, where as controller specs did not.
Thus the advantage in moving them is future proofing, having a more realistic behaviour, and being better integration tests (controller specs gave the impression of being a unit test when they were nothing of the sort, they were a partial integration of the rack stack).
At some point controller specs will go away entirely because the mechanisms supporting them will be unsupported.
rspec-railsis a thin wrapper around Rails own test helpers to enable them to be used with RSpec. Controller specs were a wrapper around controller tests which have been deprecated by the Rails team, and thus the wrapper is deprecated by us.
I am currently bringing up to date a large test suite, and I've seen this soft deprecation of the controller specs mentioned elsewhere, but mostly in rails 5.x discussions. In the rails 6 guide, I can't find a reference for this! I'm looking at the guide for v6.10 https://guides.rubyonrails.org/testing.html#functional-tests-for-your-controllers
What I can see is that the Rails team calls them functional tests instead of controller tests. Could this naming convention part of the deprecation? (Old controller tests are no more, functional tests are the new norm?)
If this is the case, then there is no further deprecation coming for RSpec's controller specs, as they are just wrapping the functional tests from Rails.
If this is the case, then there is no further deprecation coming for RSpec's controller specs, as they are just wrapping the functional tests from Rails.
This is not the case, controller specs have a different underlying class requiring the rails-controller-testing gem.
Use request specs.
So, the specific deprecated methods from rails 4 to rails 5 where assigns and assert_template?
If I'm not using these methods in my controller specs could I consider them ok?
(asked in the spirit of deepening my understanding, not trying to be repetitive)
If I'm not using these methods in my controller specs could I consider them ok?
If you have the rails-controller-testing gem so that it provides the relevant infill.
Ok, I see your point about the need of improving the documentation.
Thank you for your time and expertise! I'll follow your advice and move as many controller specs as I can into request specs
Most helpful comment
Ah you are missing a few pieces of information here, which comes back to my point about improving the documentation.
rspec-railsis a thin wrapper around Rails own test helpers to enable them to be used with RSpec. Controller specs were a wrapper around controller tests which have been deprecated by the Rails team, and thus the wrapper is deprecated by us.Request specs do largely the same job as controller specs, the main difference being they run through the entirety of the rack stack, where as controller specs did not.
Thus the advantage in moving them is future proofing, having a more realistic behaviour, and being better integration tests (controller specs gave the impression of being a unit test when they were nothing of the sort, they were a partial integration of the rack stack).
At some point controller specs will go away entirely because the mechanisms supporting them will be unsupported.