diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 81ce343a8f1c1..c9922f7f8f855 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Diff compared options with #assert_redirected_to [Rick] + * Add support in routes for semicolon delimited "subpaths", like /books/:id;:action [Jamis Buck] * Change link_to_function and button_to_function to (optionally) take an update_page block instead of a JavaScript string. Closes #4804. [zraii@comcast.net, Sam Stephenson] diff --git a/actionpack/lib/action_controller/assertions.rb b/actionpack/lib/action_controller/assertions.rb index 3fc65d5bd9f62..e94afae35adc4 100644 --- a/actionpack/lib/action_controller/assertions.rb +++ b/actionpack/lib/action_controller/assertions.rb @@ -73,8 +73,45 @@ def assert_response(type, message = nil) def assert_redirected_to(options = {}, message=nil) clean_backtrace do assert_response(:redirect, message) + ActionController::Routing::Routes.reload if ActionController::Routing::Routes.empty? + + begin + url = {} + f={ + :expected => options.is_a?(Symbol) ? @controller.send("hash_for_#{options}_url") : options.dup, + :actual => @response.redirected_to.is_a?(Symbol) ? @controller.send("hash_for_#{@response.redirected_to}_url") : @response.redirected_to.dup + }.each do |key, value| + unless value.is_a?(Hash) + request = case value + when NilClass then nil + when /^\w+:\/\// then recognized_request_for(%r{^(\w+://.*?(/|$|\?))(.*)$} =~ value ? $3 : nil) + else recognized_request_for(value) + end + value = request.path_parameters if request + end + + value.stringify_keys! if value.is_a?(Hash) + if value.respond_to?(:[]) && value['controller'] + if key == :actual && value['controller'][0..0] != '/' + value['controller'] = ActionController::Routing.controller_relative_to(value['controller'], @controller.class.controller_path) + end + value['controller'] = value['controller'][1..-1] if value['controller'][0..0] == '/' # strip leading hash + end + url[key] = value + end - if options.is_a?(String) + @response_diff = url[:expected].diff(url[:actual]) if url[:actual] + msg = build_message(message, "response is not a redirection to all of the options supplied (redirection is ), difference: ", + url[:actual], @response_diff) + + assert_block(msg) do + url[:expected].keys.all? do |k| + if k == :controller then url[:expected][k] == ActionController::Routing.controller_relative_to(url[:actual][k], @controller.class.controller_path) + else parameterize(url[:expected][k]) == parameterize(url[:actual][k]) + end + end + end + rescue ActionController::RoutingError # routing failed us, so match the strings only. msg = build_message(message, "expected a redirect to , found one to ", options, @response.redirect_url) url_regexp = %r{^(\w+://.*?(/|$|\?))(.*)$} eurl, epath, url, path = [options, @response.redirect_url].collect do |url| @@ -84,22 +121,6 @@ def assert_redirected_to(options = {}, message=nil) assert_equal(eurl, url, msg) if eurl && url assert_equal(epath, path, msg) if epath && path - else - @response_diff = options.diff(@response.redirected_to) if options.is_a?(Hash) && @response.redirected_to.is_a?(Hash) - msg = build_message(message, "response is not a redirection to all of the options supplied (redirection is )#{', difference: ' if @response_diff}", - @response.redirected_to || @response.redirect_url, @response_diff) - - assert_block(msg) do - if options.is_a?(Symbol) - @response.redirected_to == options - else - options.keys.all? do |k| - if k == :controller then options[k] == ActionController::Routing.controller_relative_to(@response.redirected_to[k], @controller.class.controller_path) - else options[k] == (@response.redirected_to[k].respond_to?(:to_param) ? @response.redirected_to[k].to_param : @response.redirected_to[k] unless @response.redirected_to[k].nil?) - end - end - end - end end end end @@ -122,14 +143,8 @@ def assert_template(expected = nil, message=nil) # Asserts that the routing of the given path was handled correctly and that the parsed options match. def assert_recognizes(expected_options, path, extras={}, message=nil) clean_backtrace do - path = "/#{path}" unless path[0..0] == '/' - # Load routes.rb if it hasn't been loaded. ActionController::Routing::Routes.reload if ActionController::Routing::Routes.empty? - - # Assume given controller - request = ActionController::TestRequest.new({}, {}, nil) - request.path = path - ActionController::Routing::Routes.recognize!(request) + request = recognized_request_for(path) expected_options = expected_options.clone extras.each_key { |key| expected_options.delete key } unless extras.nil? @@ -296,7 +311,7 @@ def assert_dom_equal(expected, actual, message="") def assert_dom_not_equal(expected, actual, message="") clean_backtrace do expected_dom = HTML::Document.new(expected).root - actual_dom = HTML::Document.new(actual).root + actual_dom = HTML::Document.new(actual).root full_message = build_message(message, " expected to be != to\n.", expected_dom.to_s, actual_dom.to_s) assert_block(full_message) { expected_dom != actual_dom } end @@ -315,6 +330,21 @@ def clean_backtrace(&block) path = File.expand_path(__FILE__) raise AssertionFailedError, e.message, e.backtrace.reject { |line| File.expand_path(line) =~ /#{path}/ } end + + private + def recognized_request_for(path) + path = "/#{path}" unless path[0..0] == '/' + + # Assume given controller + request = ActionController::TestRequest.new({}, {}, nil) + request.path = path + ActionController::Routing::Routes.recognize!(request) + request + end + + def parameterize(value) + value.respond_to?(:to_param) ? value.to_param : value + end end end end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index b359750d168b1..aae690d266635 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -226,12 +226,41 @@ def test_assert_redirect_url_match_pattern # test the redirection to a named route def test_assert_redirect_to_named_route - process :redirect_to_named_route - assert_raise(Test::Unit::AssertionFailedError) do - assert_redirected_to 'http://test.host/route_two' + with_routing do |set| + set.draw do + set.route_one 'route_one', :controller => 'action_pack_assertions', :action => 'nothing' + set.connect ':controller/:action/:id' + end + process :redirect_to_named_route + assert_redirected_to 'http://test.host/route_one' + assert_redirected_to route_one_url + assert_redirected_to :route_one end end - + + def test_assert_redirect_to_named_route_failure + with_routing do |set| + set.draw do + set.route_one 'route_one', :controller => 'action_pack_assertions', :action => 'nothing', :id => 'one' + set.route_two 'route_two', :controller => 'action_pack_assertions', :action => 'nothing', :id => 'two' + set.connect ':controller/:action/:id' + end + process :redirect_to_named_route + assert_raise(Test::Unit::AssertionFailedError) do + assert_redirected_to 'http://test.host/route_two' + end + assert_raise(Test::Unit::AssertionFailedError) do + assert_redirected_to :controller => 'action_pack_assertions', :action => 'nothing', :id => 'two' + end + assert_raise(Test::Unit::AssertionFailedError) do + assert_redirected_to route_two_url + end + assert_raise(Test::Unit::AssertionFailedError) do + assert_redirected_to :route_two + end + end + end + # test the flash-based assertions with something is in the flash def test_flash_assertions_full process :flash_me @@ -321,7 +350,6 @@ def test_flash_equals assert_flash_equal 'my name is inigo montoya...', 'hello' end - # check if we were rendered by a file-based template? def test_rendered_action process :nothing diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index d9b9042d5aed6..84f031923d595 100755 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -64,11 +64,11 @@ def test_redirect_error_with_pretty_diff assert_redirected_to :action => "other_host", :only_path => true rescue Test::Unit::AssertionFailedError => err redirection_msg, diff_msg = err.message.scan(/<\{[^\}]+\}>/).collect { |s| s[2..-3] } - assert_match %r(:only_path=>false), redirection_msg - assert_match %r(:host=>"other.test.host"), redirection_msg - assert_match %r(:action=>"other_host"), redirection_msg - assert_match %r(:only_path=>true), diff_msg - assert_match %r(:host=>"other.test.host"), diff_msg + assert_match %r("only_path"=>false), redirection_msg + assert_match %r("host"=>"other.test.host"), redirection_msg + assert_match %r("action"=>"other_host"), redirection_msg + assert_match %r("only_path"=>true), diff_msg + assert_match %r("host"=>"other.test.host"), diff_msg end end diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb index 88e0c159a71c0..2d33a3d9d422a 100644 --- a/actionpack/test/controller/test_test.rb +++ b/actionpack/test/controller/test_test.rb @@ -405,7 +405,14 @@ def test_test_uploaded_file_exception_when_file_doesnt_exist end def test_assert_redirected_to_symbol - get :redirect_to_symbol - assert_redirected_to :generate_url + with_routing do |set| + set.draw do + set.generate_url 'foo', :controller => 'test' + set.connect ':controller/:action/:id' + end + + get :redirect_to_symbol + assert_redirected_to :generate_url + end end end