Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

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
  • Loading branch information...
commit dcce01132de9734c9f7a6973bfff1b16b07ef84a 1 parent 3655d66
@dchelimsky dchelimsky authored
View
3  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) ##
View
20 actionpack/lib/action_dispatch/testing/assertions/routing.rb
@@ -69,11 +69,9 @@ def assert_recognizes(expected_options, path, extras={}, message=nil)
# 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 @@ def recognized_request_for(path, extras = {})
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 @@ def recognized_request_for(path, extras = {})
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
View
10 actionpack/test/controller/resources_test.rb
@@ -109,7 +109,7 @@ def test_irregular_id_with_no_constraints_should_raise_error
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 @@ def test_should_not_allow_delete_or_patch_or_put_on_collection_path
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 @@ def assert_whether_allowed(allowed, not_allowed, options, action, path, method)
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
View
12 actionpack/test/dispatch/routing_assertions_test.rb
@@ -54,21 +54,21 @@ def test_assert_recognizes_with_method
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 @@ def test_assert_routing_with_extras
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 @@ def test_with_routing
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
Please sign in to comment.
Something went wrong with that request. Please try again.