From 31e48939b13b5960eec0167ef0b71cd65ed36ed4 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Wed, 29 Jun 2005 02:41:00 +0000 Subject: [PATCH] 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 | 4 + .../lib/action_controller/assertions.rb | 1 + .../lib/action_controller/code_generation.rb | 2 +- .../lib/action_controller/test_process.rb | 25 ++--- .../lib/action_controller/url_rewriter.rb | 1 - actionpack/test/controller/routing_test.rb | 94 +++++++++---------- actionpack/test/controller/test_test.rb | 10 +- 7 files changed, 71 insertions(+), 66 deletions(-) diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index b3e1425843d..55cf3a0ff65 100644 --- a/actionpack/CHANGELOG +++ b/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] diff --git a/actionpack/lib/action_controller/assertions.rb b/actionpack/lib/action_controller/assertions.rb index b19ccc3b2b8..c8245c7a3a1 100644 --- a/actionpack/lib/action_controller/assertions.rb +++ b/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? diff --git a/actionpack/lib/action_controller/code_generation.rb b/actionpack/lib/action_controller/code_generation.rb index de11d4a4962..a4800037203 100644 --- a/actionpack/lib/action_controller/code_generation.rb +++ b/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) diff --git a/actionpack/lib/action_controller/test_process.rb b/actionpack/lib/action_controller/test_process.rb index 5248cd1e90d..60c8e1a02c9 100644 --- a/actionpack/lib/action_controller/test_process.rb +++ b/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 diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb index c9fd60880c5..8ae6f3e57b5 100644 --- a/actionpack/lib/action_controller/url_rewriter.rb +++ b/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 diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index d7b10f69d9f..f5149c84bcd 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/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'} ) diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb index ec067cc6c0f..c754db70eda 100644 --- a/actionpack/test/controller/test_test.rb +++ b/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 < <
@@ -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