r1475@iwill: jeremy | 2005-06-28 23:19:51 -0700

Ticket 1543 - Fix test_process
 r1476@iwill:  jeremy | 2005-06-29 00:20:53 -0700
 Correct expected, actual order for assert_equal.  Use new render method in TestController.
 r1477@iwill:  jeremy | 2005-06-29 00:23:45 -0700
 Generate route and assign parameters without modifying the user's params.
 r1480@iwill:  jeremy | 2005-06-29 00:28:52 -0700
 Update changelog.
 r1481@iwill:  jeremy | 2005-06-29 00:34:02 -0700
 Directly generate paths with a leading slash instead of tacking it on later.


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

actionpack/CHANGELOG

@@ -1,5 +1,9 @@
 *SVN*
 
+* Directly generate paths with a leading slash instead of tacking it on later.  #1543 [Nicholas Seckar]
+
+* Fixed errant parameter modification in functional tests.  #1542 [Nicholas Seckar]
+
 * Routes fail with leading slash #1540 [Nicholas Seckar]
 
 * Added support for upload progress indicators in Apache and lighttpd 1.4.x (won't work in WEBrick or lighttpd 1.3.x) #1475 [Sean Treadway]

actionpack/lib/action_controller/assertions.rb

@@ -123,6 +123,7 @@ module Test #:nodoc:
 
       # Asserts that the provided options can be used to generate the provided path.
       def assert_generates(expected_path, options, defaults={}, extras = {}, message=nil)
+        expected_path = "/#{expected_path}" unless expected_path[0] == ?/
         # Load routes.rb if it hasn't been loaded.
         ActionController::Routing::Routes.reload if ActionController::Routing::Routes.empty? 
       

actionpack/lib/action_controller/code_generation.rb

@@ -217,7 +217,7 @@ module ActionController
       end
     
       def finish
-        line %("#{segments.join('/')}")
+        line %("/#{segments.join('/')}")
       end
 
       def check_conditions(conditions)

actionpack/lib/action_controller/test_process.rb

@@ -66,10 +66,11 @@ module ActionController #:nodoc:
     def path
       @path || super()
     end
-    
-    def assign_parameters(parameters)
-      path, extras = ActionController::Routing::Routes.generate(parameters.symbolize_keys)
-      non_path_parameters = (get? ? query_parameters : request_parameters)
+
+    def generate_route_and_assign_parameters(controller_path, action, parameters)
+      parameters = parameters.symbolize_keys.merge(:controller => controller_path, :action => action)
+      path, extras = ActionController::Routing::Routes.generate(parameters)
+      non_path_parameters = get? ? query_parameters : request_parameters
       parameters.each do |key, value|
         (extras.key?(key.to_sym) ? non_path_parameters : path_parameters)[key] = value
       end
@@ -248,9 +249,7 @@ module Test
           @request.action = action.to_s
 
           parameters ||= {}
-          parameters[:controller] = @controller.class.controller_path
-          parameters[:action] = action.to_s
-          @request.assign_parameters(parameters)
+          @request.generate_route_and_assign_parameters(@controller.class.controller_path, action.to_s, parameters)
 
           @request.session = ActionController::TestSession.new(session) unless session.nil?
           @request.session["flash"] = ActionController::Flash::FlashHash.new.update(flash) if flash
@@ -307,11 +306,13 @@ module Test
         end
 
         def build_request_uri(action, parameters)
-          return if @request.env['REQUEST_URI']
-          url = ActionController::UrlRewriter.new(@request, parameters)
-          @request.set_REQUEST_URI(
-            url.rewrite(@controller.send(:rewrite_options,
-              (parameters||{}).update(:only_path => true, :action=>action))))
+          unless @request.env['REQUEST_URI']
+            options = @controller.send(:rewrite_options, parameters)
+            options.update(:only_path => true, :action => action)
+
+            url = ActionController::UrlRewriter.new(@request, parameters)
+            @request.set_REQUEST_URI(url.rewrite(options))
+          end
         end
 
         def html_document

actionpack/lib/action_controller/url_rewriter.rb

@@ -44,7 +44,6 @@ module ActionController
           extras.update(params_copy)
         end
 
-        path  = "/#{path}"
         path <<  build_query_string(extras) unless extras.empty?
         
         path

actionpack/test/controller/routing_test.rb

@@ -354,16 +354,16 @@ not_expired = true
     c = [Static.new("hello"), Static.new("world")]
     go c
     
-    assert_equal "hello/world", execute({}, {})
+    assert_equal "/hello/world", execute({}, {})
   end
   
   def test_basic_dynamic
     c = [Static.new("hi"), Dynamic.new(:action)]
     go c
     
-    assert_equal 'hi/index', execute({:action => 'index'}, {:action => 'index'})
-    assert_equal 'hi/show', execute({:action => 'show'}, {:action => 'index'})
-    assert_equal 'hi/list+people', execute({}, {:action => 'list people'})
+    assert_equal '/hi/index', execute({:action => 'index'}, {:action => 'index'})
+    assert_equal '/hi/show', execute({:action => 'show'}, {:action => 'index'})
+    assert_equal '/hi/list+people', execute({}, {:action => 'list people'})
     assert_nil execute({},{})
   end
   
@@ -371,21 +371,21 @@ not_expired = true
     c = [Static.new("hi"), Dynamic.new(:action, :default => 'index')]
     go c
     
-    assert_equal 'hi', execute({:action => 'index'}, {:action => 'index'})
-    assert_equal 'hi/show', execute({:action => 'show'}, {:action => 'index'})
-    assert_equal 'hi/list+people', execute({}, {:action => 'list people'})
-    assert_equal 'hi', execute({}, {})
+    assert_equal '/hi', execute({:action => 'index'}, {:action => 'index'})
+    assert_equal '/hi/show', execute({:action => 'show'}, {:action => 'index'})
+    assert_equal '/hi/list+people', execute({}, {:action => 'list people'})
+    assert_equal '/hi', execute({}, {})
   end
   
   def test_dynamic_with_regexp_condition
     c = [Static.new("hi"), Dynamic.new(:action, :condition => /^[a-z]+$/)]
     go c
     
-    assert_equal 'hi/index', execute({:action => 'index'}, {:action => 'index'})
+    assert_equal '/hi/index', execute({:action => 'index'}, {:action => 'index'})
     assert_nil execute({:action => 'fox5'}, {:action => 'index'})
     assert_nil execute({:action => 'something_is_up'}, {:action => 'index'})
     assert_nil execute({}, {:action => 'list people'})
-    assert_equal 'hi/abunchofcharacter', execute({:action => 'abunchofcharacter'}, {})
+    assert_equal '/hi/abunchofcharacter', execute({:action => 'abunchofcharacter'}, {})
     assert_nil execute({}, {})
   end
   
@@ -393,25 +393,25 @@ not_expired = true
     c = [Static.new("hi"), Dynamic.new(:action, :default => 'index', :condition => /^[a-z]+$/)]
     go c
     
-    assert_equal 'hi', execute({:action => 'index'}, {:action => 'index'})
+    assert_equal '/hi', execute({:action => 'index'}, {:action => 'index'})
     assert_nil execute({:action => 'fox5'}, {:action => 'index'})
     assert_nil execute({:action => 'something_is_up'}, {:action => 'index'})
     assert_nil execute({}, {:action => 'list people'})
-    assert_equal 'hi/abunchofcharacter', execute({:action => 'abunchofcharacter'}, {})
-    assert_equal 'hi', execute({}, {})
+    assert_equal '/hi/abunchofcharacter', execute({:action => 'abunchofcharacter'}, {})
+    assert_equal '/hi', execute({}, {})
   end
 
   def test_path
     c = [Static.new("hi"), Path.new(:file)]
     go c
     
-    assert_equal 'hi', execute({:file => []}, {})
-    assert_equal 'hi/books/agile_rails_dev.pdf', execute({:file => %w(books agile_rails_dev.pdf)}, {})
-    assert_equal 'hi/books/development%26whatever/agile_rails_dev.pdf', execute({:file => %w(books development&whatever agile_rails_dev.pdf)}, {})
+    assert_equal '/hi', execute({:file => []}, {})
+    assert_equal '/hi/books/agile_rails_dev.pdf', execute({:file => %w(books agile_rails_dev.pdf)}, {})
+    assert_equal '/hi/books/development%26whatever/agile_rails_dev.pdf', execute({:file => %w(books development&whatever agile_rails_dev.pdf)}, {})
     
-    assert_equal 'hi', execute({:file => ''}, {})
-    assert_equal 'hi/books/agile_rails_dev.pdf', execute({:file => 'books/agile_rails_dev.pdf'}, {})
-    assert_equal 'hi/books/development%26whatever/agile_rails_dev.pdf', execute({:file => 'books/development&whatever/agile_rails_dev.pdf'}, {})
+    assert_equal '/hi', execute({:file => ''}, {})
+    assert_equal '/hi/books/agile_rails_dev.pdf', execute({:file => 'books/agile_rails_dev.pdf'}, {})
+    assert_equal '/hi/books/development%26whatever/agile_rails_dev.pdf', execute({:file => 'books/development&whatever/agile_rails_dev.pdf'}, {})
   end
   
   def test_controller
@@ -419,10 +419,10 @@ not_expired = true
     go c
     
     assert_nil execute({}, {})
-    assert_equal 'hi/content', execute({:controller => 'content'}, {})
-    assert_equal 'hi/admin/user', execute({:controller => 'admin/user'}, {})
-    assert_equal 'hi/content', execute({}, {:controller => 'content'}) 
-    assert_equal 'hi/admin/user', execute({}, {:controller => 'admin/user'})
+    assert_equal '/hi/content', execute({:controller => 'content'}, {})
+    assert_equal '/hi/admin/user', execute({:controller => 'admin/user'}, {})
+    assert_equal '/hi/content', execute({}, {:controller => 'content'}) 
+    assert_equal '/hi/admin/user', execute({}, {:controller => 'admin/user'})
   end
   
   def test_standard_route(time = ::RunTimeTests)
@@ -430,13 +430,13 @@ not_expired = true
     go c
     
     # Make sure we get the right answers
-    assert_equal('content', execute({:action => 'index'}, {:controller => 'content', :action => 'list'}))
-    assert_equal('content/list', execute({:action => 'list'}, {:controller => 'content', :action => 'index'}))
-    assert_equal('content/show/10', execute({:action => 'show', :id => '10'}, {:controller => 'content', :action => 'list'}))
+    assert_equal('/content', execute({:action => 'index'}, {:controller => 'content', :action => 'list'}))
+    assert_equal('/content/list', execute({:action => 'list'}, {:controller => 'content', :action => 'index'}))
+    assert_equal('/content/show/10', execute({:action => 'show', :id => '10'}, {:controller => 'content', :action => 'list'}))
 
-    assert_equal('admin/user', execute({:action => 'index'}, {:controller => 'admin/user', :action => 'list'}))
-    assert_equal('admin/user/list', execute({:action => 'list'}, {:controller => 'admin/user', :action => 'index'}))
-    assert_equal('admin/user/show/10', execute({:action => 'show', :id => '10'}, {:controller => 'admin/user', :action => 'list'}))
+    assert_equal('/admin/user', execute({:action => 'index'}, {:controller => 'admin/user', :action => 'list'}))
+    assert_equal('/admin/user/list', execute({:action => 'list'}, {:controller => 'admin/user', :action => 'index'}))
+    assert_equal('/admin/user/show/10', execute({:action => 'show', :id => '10'}, {:controller => 'admin/user', :action => 'list'}))
 
     if time
       GC.start
@@ -467,9 +467,9 @@ not_expired = true
     assert_nil execute({:controller => 'content', :action => 'elcome'}, {})
     assert_nil execute({:action => 'elcome'}, {:controller => 'content'})
 
-    assert_equal '', execute({:controller => 'content', :action => 'welcome'}, {})
-    assert_equal '', execute({:action => 'welcome'}, {:controller => 'content'})
-    assert_equal '', execute({:action => 'welcome', :id => '10'}, {:controller => 'content'})
+    assert_equal '/', execute({:controller => 'content', :action => 'welcome'}, {})
+    assert_equal '/', execute({:action => 'welcome'}, {:controller => 'content'})
+    assert_equal '/', execute({:action => 'welcome', :id => '10'}, {:controller => 'content'})
   end
 end
 
@@ -509,8 +509,8 @@ class RouteTests < Test::Unit::TestCase
     
     assert_nil gen(:known => 'foo')
     assert_nil gen({})
-    assert_equal 'hello/world', gen(:known => 'known_value')
-    assert_equal 'hello/world', gen(:known => 'known_value', :extra => 'hi')
+    assert_equal '/hello/world', gen(:known => 'known_value')
+    assert_equal '/hello/world', gen(:known => 'known_value', :extra => 'hi')
     assert_equal [:extra], route.extra_keys(:known => 'known_value', :extra => 'hi')
   end
   
@@ -525,8 +525,8 @@ class RouteTests < Test::Unit::TestCase
     assert_nil gen(:controller => 'content', :action => 'show_dude', :name => 'rails')
     assert_nil gen(:controller => 'content', :action => 'show_person')
     assert_nil gen(:controller => 'admin/user', :action => 'show_person', :name => 'rails')
-    assert_equal 'hello/rails', gen(:controller => 'content', :action => 'show_person', :name => 'rails')
-    assert_equal 'hello/Nicholas+Seckar', gen(:controller => 'content', :action => 'show_person', :name => 'Nicholas Seckar')
+    assert_equal '/hello/rails', gen(:controller => 'content', :action => 'show_person', :name => 'rails')
+    assert_equal '/hello/Nicholas+Seckar', gen(:controller => 'content', :action => 'show_person', :name => 'Nicholas Seckar')
   end
   
   def test_typical
@@ -544,14 +544,14 @@ class RouteTests < Test::Unit::TestCase
     assert_equal({:controller => ::Controllers::ContentController, :action => 'show', :id => '10'}, rec('content/show/10'))
     
     
-    assert_equal 'content', gen(:controller => 'content', :action => 'index')
-    assert_equal 'content/list', gen(:controller => 'content', :action => 'list')
-    assert_equal 'content/show/10', gen(:controller => 'content', :action => 'show', :id => '10')
+    assert_equal '/content', gen(:controller => 'content', :action => 'index')
+    assert_equal '/content/list', gen(:controller => 'content', :action => 'list')
+    assert_equal '/content/show/10', gen(:controller => 'content', :action => 'show', :id => '10')
     
-    assert_equal 'admin/user', gen(:controller => 'admin/user', :action => 'index')
-    assert_equal 'admin/user', gen(:controller => 'admin/user')
-    assert_equal 'admin/user', gen({:controller => 'admin/user'}, {:controller => 'content', :action => 'list', :id => '10'})
-    assert_equal 'admin/user/show/10', gen(:controller => 'admin/user', :action => 'show', :id => '10')
+    assert_equal '/admin/user', gen(:controller => 'admin/user', :action => 'index')
+    assert_equal '/admin/user', gen(:controller => 'admin/user')
+    assert_equal '/admin/user', gen({:controller => 'admin/user'}, {:controller => 'content', :action => 'list', :id => '10'})
+    assert_equal '/admin/user/show/10', gen(:controller => 'admin/user', :action => 'show', :id => '10')
   end
 end
 
@@ -569,10 +569,10 @@ class RouteSetTests < Test::Unit::TestCase
     
     assert_equal({:controller => ::Controllers::Admin::UserController, :action => 'show', :id => '10'}.stringify_keys, rs.recognize_path(%w(admin user show 10)))
     
-    assert_equal ['admin/user/show/10', {}], rs.generate({:controller => 'admin/user', :action => 'show', :id => 10})
+    assert_equal ['/admin/user/show/10', {}], rs.generate({:controller => 'admin/user', :action => 'show', :id => 10})
     
-    assert_equal ['admin/user/show', {}], rs.generate({:action => 'show'}, {:controller => 'admin/user', :action => 'list', :id => '10'})
-    assert_equal ['admin/user/list/10', {}], rs.generate({}, {:controller => 'admin/user', :action => 'list', :id => '10'})
+    assert_equal ['/admin/user/show', {}], rs.generate({:action => 'show'}, {:controller => 'admin/user', :action => 'list', :id => '10'})
+    assert_equal ['/admin/user/list/10', {}], rs.generate({}, {:controller => 'admin/user', :action => 'list', :id => '10'})
   end
 
   def test_ignores_leading_slash
@@ -674,7 +674,7 @@ class RouteSetTests < Test::Unit::TestCase
   end
 
   def test_changing_controller
-    assert_equal ['admin/stuff/show/10', {}], rs.generate(
+    assert_equal ['/admin/stuff/show/10', {}], rs.generate(
       {:controller => 'stuff', :action => 'show', :id => 10},
       {:controller => 'admin/user', :action => 'index'}
     )

actionpack/test/controller/test_test.rb

@@ -7,11 +7,11 @@ class TestTest < Test::Unit::TestCase
     end
 
     def test_uri
-      render_text @request.request_uri
+      render :text => request.request_uri
     end
 
     def test_html_output
-      render_text <<HTML
+      render :text => <<HTML
 <html>
   <body>
     <div id="foo">
@@ -54,18 +54,18 @@ HTML
 
   def test_process_with_request_uri_with_no_params
     process :test_uri
-    assert_equal @response.body, "/test_test/test/test_uri"
+    assert_equal "/test_test/test/test_uri", @response.body
   end
 
   def test_process_with_request_uri_with_params
     process :test_uri, :id => 7
-    assert_equal @response.body, "/test_test/test/test_uri/7"
+    assert_equal "/test_test/test/test_uri/7", @response.body
   end
 
   def test_process_with_request_uri_with_params_with_explicit_uri
     @request.set_REQUEST_URI "/explicit/uri"
     process :test_uri, :id => 7
-    assert_equal @response.body, "/explicit/uri"
+    assert_equal "/explicit/uri", @response.body
   end
 
   def test_assert_tag