Make with_routing test helper work for integration tests

Adds support for with_routing test helpers in ActionDispatch::IntegrationTest.
Previously, this helper didn't work in an integration context because
the rack app and integration session under test were not mutated.
Because controller tests are integration tests by default, we should
support test routes for these kinds of test cases as well.

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