From f8f7e66f5fe393136755ebd00c49cf4081a5dc12 Mon Sep 17 00:00:00 2001
From: yui-knk <spiketeika@gmail.com>
Date: Sun, 22 Nov 2015 19:22:36 +0900
Subject: [PATCH] Brush up errors of `ActionDispatch::Routing::Mapper#mount`

* Integrate to raise `ArgumentError`
* Detailed error message when `path` is not defined
* Add a test case, invalid rack app is passed
---
 actionpack/lib/action_dispatch/routing/mapper.rb | 15 +++++++++------
 actionpack/test/dispatch/mapper_test.rb          | 14 +++++++++++++-
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 7c0404ca62..e676c837b4 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -600,17 +600,20 @@ module ActionDispatch
         def mount(app, options = nil)
           if options
             path = options.delete(:at)
-          else
-            unless Hash === app
-              raise ArgumentError, "must be called with mount point"
-            end
-
+          elsif Hash === app
             options = app
             app, path = options.find { |k, _| k.respond_to?(:call) }
             options.delete(app) if app
           end
 
-          raise "A rack application must be specified" unless path
+          raise ArgumentError, "A rack application must be specified" unless app.respond_to?(:call)
+          raise ArgumentError, <<-MSG.strip_heredoc unless path
+            Must be called with mount point
+
+              mount SomeRackApp, at: "some_route"
+              or
+              mount(SomeRackApp => "some_route")
+          MSG
 
           rails_app = rails_app? app
           options[:as] ||= app_name(app, rails_app)
diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb
index e783df855e..df27e41997 100644
--- a/actionpack/test/dispatch/mapper_test.rb
+++ b/actionpack/test/dispatch/mapper_test.rb
@@ -158,7 +158,7 @@ module ActionDispatch
         assert_equal '/*path.:format', fakeset.asts.first.to_s
       end
 
-      def test_raising_helpful_error_on_invalid_arguments
+      def test_raising_error_when_path_is_not_passed
         fakeset = FakeSet.new
         mapper = Mapper.new fakeset
         app = lambda { |env| [200, {}, [""]] }
@@ -166,6 +166,18 @@ module ActionDispatch
           mapper.mount app
         end
       end
+
+      def test_raising_error_when_rack_app_is_not_passed
+        fakeset = FakeSet.new
+        mapper = Mapper.new fakeset
+        assert_raises ArgumentError do
+          mapper.mount 10, as: "exciting"
+        end
+
+        assert_raises ArgumentError do
+          mapper.mount as: "exciting"
+        end
+      end
     end
   end
 end