Diff compared options with #assert_redirected_to [Rick]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4248 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

5 files changed, 104 insertions, 37 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 81ce343a8f..c9922f7f8f 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 3fc65d5bd9..e94afae35a 100644
--- a/actionpack/lib/action_controller/assertions.rb
+++ b/actionpack/lib/action_controller/assertions.rb
@@ -73,8 +73,45 @@ module Test #:nodoc:
       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 @@ module Test #:nodoc:
 
             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 @@ module Test #:nodoc:
       # 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 @@ module Test #:nodoc:
       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 @@ module Test #:nodoc:
         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 b359750d16..aae690d266 100644
--- a/actionpack/test/controller/action_pack_assertions_test.rb
+++ b/actionpack/test/controller/action_pack_assertions_test.rb
@@ -226,12 +226,41 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
 
   # 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 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase
     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 d9b9042d5a..84f031923d 100755
--- a/actionpack/test/controller/redirect_test.rb
+++ b/actionpack/test/controller/redirect_test.rb
@@ -64,11 +64,11 @@ class RedirectTest < Test::Unit::TestCase
       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 88e0c159a7..2d33a3d9d4 100644
--- a/actionpack/test/controller/test_test.rb
+++ b/actionpack/test/controller/test_test.rb
@@ -405,7 +405,14 @@ HTML
   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