rm add_route2

refactor the tests with a backwards compatible method call so we can rm
add_route2 from the journey router

actionpack/lib/action_dispatch/journey/routes.rb

@@ -62,13 +62,7 @@ def simulator
         end
       end
 
-      MyMapping = Struct.new(:application, :path, :conditions, :required_defaults, :defaults)
-
-      def add_route(app, path, conditions, required_defaults, defaults, name = nil)
-        add_route2(name, MyMapping.new(app, path, conditions, required_defaults, defaults))
-      end
-
-      def add_route2(name, mapping)
+      def add_route(name, mapping)
         route = Route.new(name,
                           mapping.application,
                           mapping.path,

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -525,7 +525,7 @@ def add_route(mapping, path_ast, name, anchor)
             "http://guides.rubyonrails.org/routing.html#restricting-the-routes-created"
         end
 
-        route = @set.add_route2(name, mapping)
+        route = @set.add_route(name, mapping)
         named_routes[name] = route if name
         route
       end

actionpack/test/journey/router_test.rb

@@ -33,7 +33,7 @@ def test_dashes
 
         path = Path::Pattern.build '/foo-bar-baz', {}, ['/.?'], true
 
-        routes.add_route nil, path, {}, [], {:id => nil}, {}
+        add_route nil, path, {}, [], {:id => nil}, {}
 
         env = rails_env 'PATH_INFO' => '/foo-bar-baz'
         called = false
@@ -49,7 +49,7 @@ def test_unicode
         #match the escaped version of /ほげ
         path = Path::Pattern.build '/%E3%81%BB%E3%81%92', {}, ['/.?'], true
 
-        routes.add_route nil, path, {}, [], {:id => nil}, {}
+        add_route nil, path, {}, [], {:id => nil}, {}
 
         env = rails_env 'PATH_INFO' => '/%E3%81%BB%E3%81%92'
         called = false
@@ -67,7 +67,7 @@ def test_request_class_and_requirements_success
 
         path = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true
 
-        routes.add_route nil, path, requirements, [], {:id => nil}, {}
+        add_route nil, path, requirements, [], {:id => nil}, {}
 
         env = rails_env({'PATH_INFO' => '/foo/10'}, klass)
         router.recognize(env) do |r, params|
@@ -86,7 +86,7 @@ def test_request_class_and_requirements_fail
 
         path = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true
 
-        router.routes.add_route nil, path, requirements, [], {:id => nil}, {}
+        add_route nil, path, requirements, [], {:id => nil}, {}
 
         env = rails_env({'PATH_INFO' => '/foo/10'}, klass)
         router.recognize(env) do |r, params|
@@ -112,7 +112,7 @@ def test_request_class_overrides_path_info
 
         path = Path::Pattern.build '/bar', {}, ['/.?'], true
 
-        routes.add_route nil, path, {}, [], {}, {}
+        add_route nil, path, {}, [], {}, {}
 
         env = rails_env({'PATH_INFO' => '/foo',
                          'custom.path_info' => '/bar'}, CustomPathRequest)
@@ -185,7 +185,7 @@ def test_only_required_parts_are_verified
       def test_knows_what_parts_are_missing_from_named_route
         route_name = "gorby_thunderhorse"
         path = Path::Pattern.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false)
-        @router.routes.add_route nil, path, {}, [], {}, route_name
+        add_route nil, path, {}, [], {}, route_name
 
         error = assert_raises(ActionController::UrlGenerationError) do
           @formatter.generate(route_name, { }, { })
@@ -227,7 +227,7 @@ def test_clear_trailing_slash_from_script_name_on_root_unanchored_routes
 
       def test_defaults_merge_correctly
         path  = Path::Pattern.from_string '/foo(/:id)'
-        @router.routes.add_route nil, path, {}, [], {:id => nil}, {}
+        add_route nil, path, {}, [], {:id => nil}, {}
 
         env = rails_env 'PATH_INFO' => '/foo/10'
         @router.recognize(env) do |r, params|
@@ -310,7 +310,7 @@ def test_recall_should_be_used_when_scoring
 
       def test_nil_path_parts_are_ignored
         path  = Path::Pattern.from_string "/:controller(/:action(.:format))"
-        @router.routes.add_route @app, path, {}, [], {}, {}
+        add_route @app, path, {}, [], {}, {}
 
         params = { :controller => "tasks", :format => nil }
         extras = { :action => 'lol' }
@@ -324,7 +324,7 @@ def test_generate_slash
                    [:action, "show"] ]
         path = Path::Pattern.build("/", Hash[params], ['/', '.', '?'], true)
 
-        @router.routes.add_route @app, path, {}, [], {}, {}
+        add_route @app, path, {}, [], {}, {}
 
         path, _ = @formatter.generate(nil, Hash[params], {})
         assert_equal '/', path
@@ -332,7 +332,7 @@ def test_generate_slash
 
       def test_generate_calls_param_proc
         path  = Path::Pattern.from_string '/:controller(/:action)'
-        @router.routes.add_route @app, path, {}, [], {}, {}
+        add_route @app, path, {}, [], {}, {}
 
         parameterized = []
         params = [ [:controller, "tasks"],
@@ -349,7 +349,7 @@ def test_generate_calls_param_proc
 
       def test_generate_id
         path  = Path::Pattern.from_string '/:controller(/:action)'
-        @router.routes.add_route @app, path, {}, [], {}, {}
+        add_route @app, path, {}, [], {}, {}
 
         path, params = @formatter.generate(
           nil, {:id=>1, :controller=>"tasks", :action=>"show"}, {})
@@ -359,7 +359,7 @@ def test_generate_id
 
       def test_generate_escapes
         path  = Path::Pattern.from_string '/:controller(/:action)'
-        @router.routes.add_route @app, path, {}, [], {}, {}
+        add_route @app, path, {}, [], {}, {}
 
         path, _ = @formatter.generate(nil,
           { :controller        => "tasks",
@@ -370,7 +370,7 @@ def test_generate_escapes
 
       def test_generate_escapes_with_namespaced_controller
         path  = Path::Pattern.from_string '/:controller(/:action)'
-        @router.routes.add_route @app, path, {}, [], {}, {}
+        add_route @app, path, {}, [], {}, {}
 
         path, _ = @formatter.generate(
           nil, { :controller        => "admin/tasks",
@@ -381,7 +381,7 @@ def test_generate_escapes_with_namespaced_controller
 
       def test_generate_extra_params
         path  = Path::Pattern.from_string '/:controller(/:action)'
-        @router.routes.add_route @app, path, {}, [], {}, {}
+        add_route @app, path, {}, [], {}, {}
 
         path, params = @formatter.generate(
           nil, { :id                => 1,
@@ -395,7 +395,7 @@ def test_generate_extra_params
 
       def test_generate_missing_keys_no_matches_different_format_keys
         path  = Path::Pattern.from_string '/:controller/:action/:name'
-        @router.routes.add_route @app, path, {}, [], {}, {}
+        add_route @app, path, {}, [], {}, {}
         primarty_parameters = {
           :id                => 1,
           :controller        => "tasks",
@@ -422,7 +422,7 @@ def test_generate_missing_keys_no_matches_different_format_keys
 
       def test_generate_uses_recall_if_needed
         path  = Path::Pattern.from_string '/:controller(/:action(/:id))'
-        @router.routes.add_route @app, path, {}, [], {}, {}
+        add_route @app, path, {}, [], {}, {}
 
         path, params = @formatter.generate(
           nil,
@@ -434,7 +434,7 @@ def test_generate_uses_recall_if_needed
 
       def test_generate_with_name
         path  = Path::Pattern.from_string '/:controller(/:action)'
-        @router.routes.add_route @app, path, {}, [], {}, "tasks"
+        add_route @app, path, {}, [], {}, "tasks"
 
         path, params = @formatter.generate(
           "tasks",
@@ -452,7 +452,7 @@ def test_generate_with_name
         define_method("test_recognize_#{expected.keys.map(&:to_s).join('_')}") do
           path  = Path::Pattern.from_string "/:controller(/:action(/:id))"
           app   = Object.new
-          route = @router.routes.add_route(app, path, {}, [], {}, {})
+          route = add_route(app, path, {}, [], {}, {})
 
           env = rails_env 'PATH_INFO' => request_path
           called   = false
@@ -474,7 +474,7 @@ def test_generate_with_name
         define_method("test_recognize_#{name}") do
           path  = Path::Pattern.from_string '/:segment/*splat'
           app   = Object.new
-          route = @router.routes.add_route(app, path, {}, [], {}, {})
+          route = add_route(app, path, {}, [], {}, {})
 
           env = rails_env 'PATH_INFO' => request_path
           called   = false
@@ -497,7 +497,7 @@ def test_namespaced_controller
           true
         )
         app   = Object.new
-        route = @router.routes.add_route(app, path, {}, [], {}, {})
+        route = add_route(app, path, {}, [], {}, {})
 
         env = rails_env 'PATH_INFO' => '/admin/users/show/10'
         called   = false
@@ -518,7 +518,7 @@ def test_namespaced_controller
       def test_recognize_literal
         path   = Path::Pattern.from_string "/books(/:action(.:format))"
         app    = Object.new
-        route  = @router.routes.add_route(app, path, {}, [], {:controller => 'books'})
+        route  = add_route(app, path, {}, [], {:controller => 'books'})
 
         env    = rails_env 'PATH_INFO' => '/books/list.rss'
         expected = { :controller => 'books', :action => 'list', :format => 'rss' }
@@ -536,7 +536,7 @@ def test_recognize_head_route
         path   = Path::Pattern.from_string "/books(/:action(.:format))"
         app    = Object.new
         conditions = { request_method: 'HEAD' }
-        @router.routes.add_route(app, path, conditions, [], {})
+        add_route(app, path, conditions, [], {})
 
         env = rails_env(
           'PATH_INFO' => '/books/list.rss',
@@ -557,7 +557,7 @@ def test_recognize_head_request_as_get_route
         conditions = {
           :request_method => 'GET'
         }
-        @router.routes.add_route(app, path, conditions, [], {})
+        add_route(app, path, conditions, [], {})
 
         env = rails_env 'PATH_INFO' => '/books/list.rss',
                         "REQUEST_METHOD"    => "HEAD"
@@ -574,7 +574,7 @@ def test_recognize_cares_about_verbs
         path   = Path::Pattern.from_string "/books(/:action(.:format))"
         app    = Object.new
         conditions = { request_method: 'GET' }
-        @router.routes.add_route(app, path, conditions, [], {})
+        add_route(app, path, conditions, [], {})
 
         env = rails_env 'PATH_INFO' => '/books/list.rss',
                         "REQUEST_METHOD" => "POST"
@@ -589,7 +589,7 @@ def test_recognize_cares_about_verbs
         conditions = conditions.dup
         conditions[:request_method] = 'POST'
 
-        post = @router.routes.add_route(app, path, conditions, [], {})
+        post = add_route(app, path, conditions, [], {})
 
         called = false
         @router.recognize(env) do |r, params|
@@ -609,7 +609,7 @@ def add_routes router, paths, anchor = true
           else
             path
           end
-          router.routes.add_route @app, path, {}, [], {}, {}
+          add_route @app, path, {}, [], {}, {}
         end
       end
 
@@ -636,6 +636,12 @@ def rack_env env
           "CONTENT_LENGTH"    => "0"
         }.merge env
       end
+
+      MyMapping = Struct.new(:application, :path, :conditions, :required_defaults, :defaults)
+
+      def add_route(app, path, conditions, required_defaults, defaults, name = nil)
+        @routes.add_route(name, MyMapping.new(app, path, conditions, required_defaults, defaults))
+      end
     end
   end
 end

actionpack/test/journey/routes_test.rb

@@ -3,16 +3,24 @@
 module ActionDispatch
   module Journey
     class TestRoutes < ActiveSupport::TestCase
-      setup do
+      attr_reader :routes
+
+      def setup
         @routes = Routes.new
+        super
+      end
+
+      MyMapping = Struct.new(:application, :path, :conditions, :required_defaults, :defaults)
+
+      def add_route(app, path, conditions, required_defaults, defaults, name = nil)
+        @routes.add_route(name, MyMapping.new(app, path, conditions, required_defaults, defaults))
       end
 
       def test_clear
-        routes = Routes.new
         path    = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true
         requirements = { :hello => /world/ }
 
-        routes.add_route nil, path, requirements, [], {:id => nil}, {}
+        add_route nil, path, requirements, [], {:id => nil}, {}
         assert_not routes.empty?
         assert_equal 1, routes.length
 
@@ -22,29 +30,27 @@ def test_clear
       end
 
       def test_ast
-        routes = Routes.new
         path   = Path::Pattern.from_string '/hello'
 
-        routes.add_route nil, path, {}, [], {}, {}
+        add_route nil, path, {}, [], {}, {}
         ast = routes.ast
-        routes.add_route nil, path, {}, [], {}, {}
+        add_route nil, path, {}, [], {}, {}
         assert_not_equal ast, routes.ast
       end
 
       def test_simulator_changes
-        routes = Routes.new
         path   = Path::Pattern.from_string '/hello'
 
-        routes.add_route nil, path, {}, [], {}, {}
+        add_route nil, path, {}, [], {}, {}
         sim = routes.simulator
-        routes.add_route nil, path, {}, [], {}, {}
+        add_route nil, path, {}, [], {}, {}
         assert_not_equal sim, routes.simulator
       end
 
       def test_partition_route
         path   = Path::Pattern.from_string '/hello'
 
-        anchored_route = @routes.add_route nil, path, {}, [], {}, {}
+        anchored_route = add_route nil, path, {}, [], {}, {}
         assert_equal [anchored_route], @routes.anchored_routes
         assert_equal [], @routes.custom_routes
 
@@ -52,19 +58,17 @@ def test_partition_route
           "/hello/:who", { who: /\d/ }, ['/', '.', '?'], false
         )
 
-        custom_route = @routes.add_route nil, path, {}, [], {}, {}
+        custom_route = add_route nil, path, {}, [], {}, {}
         assert_equal [custom_route], @routes.custom_routes
         assert_equal [anchored_route], @routes.anchored_routes
       end
 
       def test_first_name_wins
-        routes = Routes.new
-
         one   = Path::Pattern.from_string '/hello'
         two   = Path::Pattern.from_string '/aaron'
 
-        routes.add_route nil, one, {}, [], {}, 'aaron'
-        routes.add_route nil, two, {}, [], {}, 'aaron'
+        add_route nil, one, {}, [], {}, 'aaron'
+        add_route nil, two, {}, [], {}, 'aaron'
 
         assert_equal '/hello', routes.named_routes['aaron'].path.spec.to_s
       end