let request specs use integration test services

[bronson]

Rails integration tests have some services like assert_select.
This pull request makes them available in request specs.

This is already the case for controller specs: https://github.com/rspec/rspec-rails/blob/4a2a078632990/lib/rspec/rails/example/controller_example_group.rb#L13

This came up while converting controller specs to request specs as suggested by the Rails 5 release notes. (I have an API-only app so capybara is out)

This will be a no-op until Rails merges rails/rails#23880

[bronson]

Lint fail: (and technically I agree)

lib/rspec/rails/example/request_example_group.rb:8:7: C: Avoid using rescue in its modifier form.
      include ActionDispatch::IntegrationTest::Behavior rescue NameError
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Fixed, and includes a helpful comment.

[JonRowe]

I'm not sure we need this, we already get the NoMethodError for free after all ;)

[bronson]

Totally agree. This is my way around Rubocop's "do not suppress exceptions error". Turns out, after writing it, I actually like it. Yay for helpful error messages.

I originally wrote this PR as a one-liner:

include ActionDispatch::IntegrationTest::Behavior rescue NameError

but Rubocop didn't like that.

So I did a block:

     begin
        include ActionDispatch::IntegrationTest::Behavior
      rescue NameError
        # if AD::IT::Behavior doesn't exist, that just means that Rails is old
        # and integration test helpers (like assert_select) aren't available.
      end

And it didn't like that either. So I went all the way to this.

Happy to throw a # rubocop:disable Lint/HandleExceptions and got back to the one-liner if you'd prefer. But I kinda vote for the error messages.

[bronson]

The Rails side is merged.

@JonRowe shall I delete IntegrationHelperWarnings and put in # rubocop:disable Lint/HandleExceptions instead? (otherwise Rubocop complains about the empty rescue)

Like we discussed, I'm partial to the helpful error message, but I'd like to do whatever you think is best.

[filialpails]

Regarding the modifier form of rescue,

include ActionDispatch::IntegrationTest::Behavior rescue NameError

isn't equivalent to

begin
  include ActionDispatch::IntegrationTest::Behavior
rescue NameError
end

but to

begin
  include ActionDispatch::IntegrationTest::Behavior
rescue
  NameError
end

You can't rescue a particular class of exceptions with the modifier form. It always rescues StandardError and its descendants.

[bronson]

That's true. Block form would be best here.

The question is, what should go in that block? The fake module, or a comment to placate the linter?

[JonRowe]

# rubocop:disable Lint/HandleExceptions is fine if a comment in the rescue block doesn't do that for you.

[bronson]

Rebased and ready. The Travis breakage is identical to master so this branch should be fine.

[bronson]

Arg, updated the comment:

- # rails is too old to provide request helpers
+ # rails is too old to provide integration test helpers

Everything else is unchanged, still ready to merge.

[JonRowe]

Thanks

[bronson]

just for posterity, this is the patch that displayed a helpful error:

diff --git a/lib/rspec/rails/example/request_example_group.rb b/lib/rspec/rails/example/request_example_group.rb
index d308bce..f53bcef 100644
--- a/lib/rspec/rails/example/request_example_group.rb
+++ b/lib/rspec/rails/example/request_example_group.rb
@@ -1,5 +1,14 @@
 module RSpec
   module Rails
+    # Raises an error if the user tries to call integration test helpers.
+    # If the helpers are available then this module will be ignored.
+    module IntegrationHelperWarnings
+      extend ActiveSupport::Concern
+      [:assert_select, :url_for, :url_options].each do |name|
+        define_method(name) { |*| raise "you must upgrade Rails to call #{name}" }
+      end
+    end
+
     # @api public
     # Container class for request spec functionality.
     module RequestExampleGroup
@@ -11,6 +20,12 @@ module RSpec
       include RSpec::Rails::Matchers::RenderTemplate
       include ActionController::TemplateAssertions

+      begin
+        include ActionDispatch::IntegrationTest::Behavior
+      rescue NameError
+        include IntegrationHelperWarnings
+      end
+
       # Delegates to `Rails.application`.
       def app
         ::Rails.application