Raise Assertion instead of RoutingError for routing assertion failures.

Before this change, assert_recognizes, assert_generates, and
assert_routing raised ActionController::RoutingError when they failed to
recognize the route.

This commit changes them to raise Assertion instead. This aligns with
convention for logical failures, and supports reporting tools that care
about the difference between logical failures and errors e.g. the
summary at the end of a test run.

- Fixes #5899

actionpack/CHANGELOG.md

@@ -186,6 +186,9 @@
 *   `ActionView::Helpers::TextHelper#highlight` now defaults to the
     HTML5 `mark` element. *Brian Cardarella*
 
+*   `assert_generates`, `assert_recognizes`, and `assert_routing` all raise
+    `Assertion` instead of `RoutingError` *David Chelimsky*
+
 
 ## Rails 3.2.3 (March 30, 2012) ##
 

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

@@ -69,11 +69,9 @@ module ActionDispatch
       #   assert_generates "changesets/12", { :controller => 'scm', :action => 'show_diff', :revision => "12" }
       def assert_generates(expected_path, options, defaults={}, extras = {}, message=nil)
         if expected_path =~ %r{://}
-          begin
+          fail_on(URI::InvalidURIError) do
             uri = URI.parse(expected_path)
             expected_path = uri.path.to_s.empty? ? "/" : uri.path
-          rescue URI::InvalidURIError => e
-            raise ActionController::RoutingError, e.message
           end
         else
           expected_path = "/#{expected_path}" unless expected_path.first == '/'
@@ -189,14 +187,12 @@ module ActionDispatch
           request = ActionController::TestRequest.new
 
           if path =~ %r{://}
-            begin
+            fail_on(URI::InvalidURIError) do
               uri = URI.parse(path)
               request.env["rack.url_scheme"] = uri.scheme || "http"
               request.host = uri.host if uri.host
               request.port = uri.port if uri.port
               request.path = uri.path.to_s.empty? ? "/" : uri.path
-            rescue URI::InvalidURIError => e
-              raise ActionController::RoutingError, e.message
             end
           else
             path = "/#{path}" unless path.first == "/"
@@ -205,11 +201,21 @@ module ActionDispatch
 
           request.request_method = method if method
 
-          params = @routes.recognize_path(path, { :method => method, :extras => extras })
+          params = fail_on(ActionController::RoutingError) do
+            @routes.recognize_path(path, { :method => method, :extras => extras })
+          end
           request.path_parameters = params.with_indifferent_access
 
           request
         end
+
+        def fail_on(exception_class)
+          begin
+            yield
+          rescue exception_class => e
+            raise MiniTest::Assertion, e.message
+          end
+        end
     end
   end
 end

actionpack/test/controller/resources_test.rb

@@ -109,7 +109,7 @@ class ResourcesTest < ActionController::TestCase
     expected_options = {:controller => 'messages', :action => 'show', :id => '1.1.1'}
 
     with_restful_routing :messages do
-      assert_raise(ActionController::RoutingError) do
+      assert_raise(Assertion) do
         assert_recognizes(expected_options, :path => 'messages/1.1.1', :method => :get)
       end
     end
@@ -660,15 +660,15 @@ class ResourcesTest < ActionController::TestCase
       options = { :controller => controller_name.to_s }
       collection_path = "/#{controller_name}"
 
-      assert_raise(ActionController::RoutingError) do
+      assert_raise(Assertion) do
         assert_recognizes(options.merge(:action => 'update'), :path => collection_path, :method => :patch)
       end
 
-      assert_raise(ActionController::RoutingError) do
+      assert_raise(Assertion) do
         assert_recognizes(options.merge(:action => 'update'), :path => collection_path, :method => :put)
       end
 
-      assert_raise(ActionController::RoutingError) do
+      assert_raise(Assertion) do
         assert_recognizes(options.merge(:action => 'destroy'), :path => collection_path, :method => :delete)
       end
     end
@@ -1353,7 +1353,7 @@ class ResourcesTest < ActionController::TestCase
     end
 
     def assert_not_recognizes(expected_options, path)
-      assert_raise ActionController::RoutingError, Assertion do
+      assert_raise Assertion do
         assert_recognizes(expected_options, path)
       end
     end

actionpack/test/dispatch/routing_assertions_test.rb

@@ -54,21 +54,21 @@ class RoutingAssertionsTest < ActionController::TestCase
   end
 
   def test_assert_recognizes_with_hash_constraint
-    assert_raise(ActionController::RoutingError) do
+    assert_raise(Assertion) do
       assert_recognizes({ :controller => 'secure_articles', :action => 'index' }, 'http://test.host/secure/articles')
     end
     assert_recognizes({ :controller => 'secure_articles', :action => 'index', :protocol => 'https://' }, 'https://test.host/secure/articles')
   end
 
   def test_assert_recognizes_with_block_constraint
-    assert_raise(ActionController::RoutingError) do
+    assert_raise(Assertion) do
       assert_recognizes({ :controller => 'block_articles', :action => 'index' }, 'http://test.host/block/articles')
     end
     assert_recognizes({ :controller => 'block_articles', :action => 'index' }, 'https://test.host/block/articles')
   end
 
   def test_assert_recognizes_with_query_constraint
-    assert_raise(ActionController::RoutingError) do
+    assert_raise(Assertion) do
       assert_recognizes({ :controller => 'query_articles', :action => 'index', :use_query => 'false' }, '/query/articles', { :use_query => 'false' })
     end
     assert_recognizes({ :controller => 'query_articles', :action => 'index', :use_query => 'true' }, '/query/articles', { :use_query => 'true' })
@@ -87,14 +87,14 @@ class RoutingAssertionsTest < ActionController::TestCase
   end
 
   def test_assert_routing_with_hash_constraint
-    assert_raise(ActionController::RoutingError) do
+    assert_raise(Assertion) do
       assert_routing('http://test.host/secure/articles', { :controller => 'secure_articles', :action => 'index' })
     end
     assert_routing('https://test.host/secure/articles', { :controller => 'secure_articles', :action => 'index', :protocol => 'https://' })
   end
 
   def test_assert_routing_with_block_constraint
-    assert_raise(ActionController::RoutingError) do
+    assert_raise(Assertion) do
       assert_routing('http://test.host/block/articles', { :controller => 'block_articles', :action => 'index' })
     end
     assert_routing('https://test.host/block/articles', { :controller => 'block_articles', :action => 'index' })
@@ -107,7 +107,7 @@ class RoutingAssertionsTest < ActionController::TestCase
       end
 
       assert_routing('/artikel', :controller => 'articles', :action => 'index')
-      assert_raise(ActionController::RoutingError) do
+      assert_raise(Assertion) do
         assert_routing('/articles', { :controller => 'articles', :action => 'index' })
       end
     end