Merge pull request rails#49819 from gmcgibbon/integration_with_routes

Make with_routing test helper work for integration tests

Author: gmcgibbon

actionpack/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Add support for `with_routing` test helper in `ActionDispatch::IntegrationTest`
+
+    *Gannon McGibbon*
+
 *   Remove deprecated support to set `Rails.application.config.action_dispatch.show_exceptions` to `true` and `false`.
 
     *Rafael Mendonça França*

actionpack/lib/action_dispatch/testing/assertions/routing.rb

@@ -11,6 +11,65 @@ module Assertions
     module RoutingAssertions
       extend ActiveSupport::Concern
 
+      module WithIntegrationRouting # :nodoc:
+        extend ActiveSupport::Concern
+
+        module ClassMethods
+          def with_routing(&block)
+            old_routes = nil
+            old_integration_session = nil
+
+            setup do
+              old_routes = app.routes
+              old_integration_session = integration_session
+              create_routes(&block)
+            end
+
+            teardown do
+              reset_routes(old_routes, old_integration_session)
+            end
+          end
+        end
+
+        def with_routing(&block)
+          old_routes = app.routes
+          old_integration_session = integration_session
+          create_routes(&block)
+        ensure
+          reset_routes(old_routes, old_integration_session)
+        end
+
+        private
+          def create_routes
+            app = self.app
+            routes = ActionDispatch::Routing::RouteSet.new
+            rack_app = app.config.middleware.build(routes)
+            https = integration_session.https?
+            host = integration_session.host
+
+            app.instance_variable_set(:@routes, routes)
+            app.instance_variable_set(:@app, rack_app)
+            @integration_session = Class.new(ActionDispatch::Integration::Session) do
+              include app.routes.url_helpers
+              include app.routes.mounted_helpers
+            end.new(app)
+            @integration_session.https! https
+            @integration_session.host! host
+            @routes = routes
+
+            yield routes
+          end
+
+          def reset_routes(old_routes, old_integration_session)
+            old_rack_app = app.config.middleware.build(old_routes)
+
+            app.instance_variable_set(:@routes, old_routes)
+            app.instance_variable_set(:@app, old_rack_app)
+            @integration_session = old_integration_session
+            @routes = old_routes
+          end
+      end
+
       module ClassMethods
         # A helper to make it easier to test different route configurations.
         # This method temporarily replaces @routes with a new RouteSet instance
@@ -44,6 +103,26 @@ def setup # :nodoc:
         super
       end
 
+      # A helper to make it easier to test different route configurations.
+      # This method temporarily replaces @routes with a new RouteSet instance.
+      #
+      # The new instance is yielded to the passed block. Typically the block
+      # will create some routes using <tt>set.draw { match ... }</tt>:
+      #
+      #   with_routing do |set|
+      #     set.draw do
+      #       resources :users
+      #     end
+      #     assert_equal "/users", users_path
+      #   end
+      #
+      def with_routing(&block)
+        old_routes, old_controller = @routes, @controller
+        create_routes(&block)
+      ensure
+        reset_routes(old_routes, old_controller)
+      end
+
       # Asserts that the routing of the given +path+ was handled correctly and that the parsed options (given in the +expected_options+ hash)
       # match +path+. Basically, it asserts that \Rails recognizes the route given by +expected_options+.
       #
@@ -167,26 +246,6 @@ def assert_routing(path, options, defaults = {}, extras = {}, message = nil)
         assert_generates(path.is_a?(Hash) ? path[:path] : path, generate_options, defaults, extras, message)
       end
 
-      # A helper to make it easier to test different route configurations.
-      # This method temporarily replaces @routes with a new RouteSet instance.
-      #
-      # The new instance is yielded to the passed block. Typically the block
-      # will create some routes using <tt>set.draw { match ... }</tt>:
-      #
-      #   with_routing do |set|
-      #     set.draw do
-      #       resources :users
-      #     end
-      #     assert_equal "/users", users_path
-      #   end
-      #
-      def with_routing(&block)
-        old_routes, old_controller = @routes, @controller
-        create_routes(&block)
-      ensure
-        reset_routes(old_routes, old_controller)
-      end
-
       # ROUTES TODO: These assertions should really work in an integration context
       def method_missing(selector, *args, &block)
         if defined?(@controller) && @controller && defined?(@routes) && @routes && @routes.named_routes.route_defined?(selector)

actionpack/lib/action_dispatch/testing/integration.rb

@@ -657,6 +657,7 @@ module Behavior
       included do
         include ActionDispatch::Routing::UrlFor
         include UrlOptions # don't let UrlFor override the url_options method
+        include ActionDispatch::Assertions::RoutingAssertions::WithIntegrationRouting
         ActiveSupport.run_load_hooks(:action_dispatch_integration_test, self)
         @@app = nil
       end

actionpack/test/abstract_unit.rb

@@ -91,15 +91,23 @@ class TestCase
 end
 
 class RoutedRackApp
+  class Config < Struct.new(:middleware)
+  end
+
   attr_reader :routes
 
   def initialize(routes, &blk)
     @routes = routes
-    @stack = ActionDispatch::MiddlewareStack.new(&blk).build(@routes)
+    @stack = ActionDispatch::MiddlewareStack.new(&blk)
+    @app = @stack.build(@routes)
   end
 
   def call(env)
-    @stack.call(env)
+    @app.call(env)
+  end
+
+  def config
+    Config.new(@stack)
   end
 end
 
@@ -150,19 +158,6 @@ def self.stub_controllers(config = ActionDispatch::Routing::RouteSet::DEFAULT_CO
     yield DeadEndRoutes.new(config)
   end
 
-  def with_routing(&block)
-    temporary_routes = ActionDispatch::Routing::RouteSet.new
-    old_app, self.class.app = self.class.app, self.class.build_app(temporary_routes)
-    old_routes = SharedTestRoutes
-    silence_warnings { Object.const_set(:SharedTestRoutes, temporary_routes) }
-
-    yield temporary_routes
-  ensure
-    self.class.app = old_app
-    remove!
-    silence_warnings { Object.const_set(:SharedTestRoutes, old_routes) }
-  end
-
   def with_autoload_path(path)
     path = File.join(__dir__, "fixtures", path)
     Zeitwerk.with_loader do |loader|

actionpack/test/controller/flash_test.rb

@@ -377,6 +377,14 @@ def get(path, **options)
       super(path, **options)
     end
 
+    def app
+      @app ||= self.class.build_app do |middleware|
+        middleware.use ActionDispatch::Session::CookieStore, key: SessionKey
+        middleware.use ActionDispatch::Flash
+        middleware.delete ActionDispatch::ShowExceptions
+      end
+    end
+
     def with_test_route_set
       with_routing do |set|
         set.draw do
@@ -385,12 +393,6 @@ def with_test_route_set
           end
         end
 
-        @app = self.class.build_app(set) do |middleware|
-          middleware.use ActionDispatch::Session::CookieStore, key: SessionKey
-          middleware.use ActionDispatch::Flash
-          middleware.delete ActionDispatch::ShowExceptions
-        end
-
         yield
       end
     end

actionpack/test/controller/integration_test.rb

@@ -172,15 +172,17 @@ class RackLintIntegrationTest < ActionDispatch::IntegrationTest
         get "/", to: ->(_) { [200, {}, [""]] }
       end
 
-      @app = self.class.build_app(set) do |middleware|
-        middleware.unshift Rack::Lint
-      end
-
       get "/"
 
       assert_equal 200, status
     end
   end
+
+  def app
+    @app ||= self.class.build_app do |middleware|
+      middleware.unshift Rack::Lint
+    end
+  end
 end
 
 # Tests that integration tests don't call Controller test methods for processing.

actionpack/test/dispatch/request/query_string_parsing_test.rb

@@ -157,16 +157,19 @@ def test_array_parses_without_nil
   end
 
   private
+    def app
+      @app ||= self.class.build_app do |middleware|
+        middleware.use(EarlyParse)
+      end
+    end
+
     def assert_parses(expected, actual)
       with_routing do |set|
         set.draw do
           ActionDispatch.deprecator.silence do
             get ":action", to: ::QueryStringParsingTest::TestController
           end
         end
-        @app = self.class.build_app(set) do |middleware|
-          middleware.use(EarlyParse)
-        end
 
         get "/parse", params: actual
         assert_response :ok

actionpack/test/dispatch/request_id_test.rb

@@ -55,6 +55,10 @@ def index
     end
   end
 
+  setup do
+    @header = "X-Request-Id"
+  end
+
   test "request id is passed all the way to the response" do
     with_test_route_set do
       get "/"
@@ -70,25 +74,28 @@ def index
   end
 
   test "using a custom request_id header key" do
-    with_test_route_set(header: "X-Tracer-Id") do
+    @header = "X-Tracer-Id"
+    with_test_route_set do
       get "/"
       assert_match(/\w+/, @response.headers["X-Tracer-Id"])
     end
   end
 
   private
+    def app
+      @app ||= self.class.build_app do |middleware|
+        middleware.use Rack::Lint
+        middleware.use ActionDispatch::RequestId, header: @header
+        middleware.use Rack::Lint
+      end
+    end
+
     def with_test_route_set(header: "X-Request-Id")
       with_routing do |set|
         set.draw do
           get "/", to: ::RequestIdResponseTest::TestController.action(:index)
         end
 
-        @app = self.class.build_app(set) do |middleware|
-          middleware.use Rack::Lint
-          middleware.use ActionDispatch::RequestId, header: header
-          middleware.use Rack::Lint
-        end
-
         yield
       end
     end