From e61afed6f8d2ef6a580ba00a5beaaaf0bf2ddb9a Mon Sep 17 00:00:00 2001 From: Jarl Friis Date: Mon, 11 May 2009 14:09:22 +0200 Subject: My suggestion to fix ticket 2401 [#2401 state:resolved] Signed-off-by: Yehuda Katz + Carl Lerche --- actionpack/lib/action_view/helpers/form_helper.rb | 10 ++- actionpack/test/template/form_helper_test.rb | 92 +++++++++++++++++++++++ 2 files changed, 98 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 8ecec87b10..6d6d623938 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -926,6 +926,7 @@ module ActionView attr_accessor :object_name, :object, :options def initialize(object_name, object, template, options, proc) + @nested_child_index = {} @object_name, @object, @template, @options, @proc = object_name, object, template, options, proc @default_options = @options ? @options.slice(:index) : {} if @object_name.to_s.match(/\[\]$/) @@ -1028,7 +1029,7 @@ module ActionView explicit_child_index = args.last[:child_index] if args.last.is_a?(Hash) children.map do |child| - fields_for_nested_model("#{name}[#{explicit_child_index || nested_child_index}]", child, args, block) + fields_for_nested_model("#{name}[#{explicit_child_index || nested_child_index(name)}]", child, args, block) end.join else fields_for_nested_model(name, explicit_object || association, args, block) @@ -1046,9 +1047,9 @@ module ActionView end end - def nested_child_index - @nested_child_index ||= -1 - @nested_child_index += 1 + def nested_child_index(name) + @nested_child_index[name] ||= -1 + @nested_child_index[name] += 1 end end end @@ -1056,5 +1057,6 @@ module ActionView class << Base attr_accessor :default_form_builder end + Base.default_form_builder = ::ActionView::Helpers::FormBuilder end diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index 56cdf6df9a..515f73c339 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -21,6 +21,9 @@ silence_warnings do attr_accessor :comments def comments_attributes=(attributes); end + + attr_accessor :tags + def tags_attributes=(attributes); end end class Comment @@ -33,6 +36,50 @@ silence_warnings do def name @id.nil? ? "new #{self.class.name.downcase}" : "#{self.class.name.downcase} ##{@id}" end + + attr_accessor :relevances + def relevances_attributes=(attributes); end + + end + + class Tag + attr_reader :id + attr_reader :post_id + def initialize(id = nil, post_id = nil); @id, @post_id = id, post_id end + def save; @id = 1; @post_id = 1 end + def new_record?; @id.nil? end + def to_param; @id; end + def value + @id.nil? ? "new #{self.class.name.downcase}" : "#{self.class.name.downcase} ##{@id}" + end + + attr_accessor :relevances + def relevances_attributes=(attributes); end + + end + + class CommentRelevance + attr_reader :id + attr_reader :comment_id + def initialize(id = nil, comment_id = nil); @id, @comment_id = id, comment_id end + def save; @id = 1; @comment_id = 1 end + def new_record?; @id.nil? end + def to_param; @id; end + def value + @id.nil? ? "new #{self.class.name.downcase}" : "#{self.class.name.downcase} ##{@id}" + end + end + + class TagRelevance + attr_reader :id + attr_reader :tag_id + def initialize(id = nil, tag_id = nil); @id, @tag_id = id, tag_id end + def save; @id = 1; @tag_id = 1 end + def new_record?; @id.nil? end + def to_param; @id; end + def value + @id.nil? ? "new #{self.class.name.downcase}" : "#{self.class.name.downcase} ##{@id}" + end end class Author < Comment @@ -740,6 +787,51 @@ class FormHelperTest < ActionView::TestCase assert_dom_equal expected, output_buffer end + def test_nested_fields_uses_unique_indices_for_different_collection_associations + @post.comments = [Comment.new(321)] + @post.tags = [Tag.new(123), Tag.new(456)] + @post.comments[0].relevances = [] + @post.tags[0].relevances = [] + @post.tags[1].relevances = [] + form_for(:post, @post) do |f| + f.fields_for(:comments, @post.comments[0]) do |cf| + concat cf.text_field(:name) + cf.fields_for(:relevances, CommentRelevance.new(314)) do |crf| + concat crf.text_field(:value) + end + end + f.fields_for(:tags, @post.tags[0]) do |tf| + concat tf.text_field(:value) + tf.fields_for(:relevances, TagRelevance.new(3141)) do |trf| + concat trf.text_field(:value) + end + end + f.fields_for('tags', @post.tags[1]) do |tf| + concat tf.text_field(:value) + tf.fields_for(:relevances, TagRelevance.new(31415)) do |trf| + concat trf.text_field(:value) + end + end + end + + expected = '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + + assert_dom_equal expected, output_buffer + end + def test_fields_for fields_for(:post, @post) do |f| concat f.text_field(:title) -- cgit v1.2.3 From d03689971758b905fa0087bc93cf474a9d0585f5 Mon Sep 17 00:00:00 2001 From: Brian Abreu Date: Wed, 24 Jun 2009 10:51:20 -0700 Subject: Fixed ActiveSupport::OrderedHash::[] work identically to ::Hash::[] in ruby 1.8.7 [#2832 state:resolved] Signed-off-by: Yehuda Katz + Carl Lerche --- activesupport/lib/active_support/ordered_hash.rb | 22 +++++++++++++++++---- activesupport/test/ordered_hash_test.rb | 25 +++++++++++++++++++++++- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index 8d1c0f5160..4324e40cbb 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -12,11 +12,25 @@ module ActiveSupport def self.[](*args) ordered_hash = new - args.each_with_index { |val,ind| - # Only every second value is a key. - next if ind % 2 != 0 + + if (args.length == 1 && args.first.is_a?(Array)) + args.first.each do |key_value_pair| + next unless (key_value_pair.is_a?(Array)) + ordered_hash[key_value_pair[0]] = key_value_pair[1] + end + + return ordered_hash + end + + unless (args.size % 2 == 0) + raise ArgumentError.new("odd number of arguments for Hash") + end + + args.each_with_index do |val, ind| + next if (ind % 2 != 0) ordered_hash[val] = args[ind + 1] - } + end + ordered_hash end diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb index 647938dd87..15bd57181f 100644 --- a/activesupport/test/ordered_hash_test.rb +++ b/activesupport/test/ordered_hash_test.rb @@ -163,9 +163,32 @@ class OrderedHashTest < Test::Unit::TestCase assert @ordered_hash.inspect.include?(@hash.inspect) end - def test_alternate_initialization + def test_alternate_initialization_with_splat alternate = ActiveSupport::OrderedHash[1,2,3,4] assert_kind_of ActiveSupport::OrderedHash, alternate assert_equal [1, 3], alternate.keys end + + def test_alternate_initialization_with_array + alternate = ActiveSupport::OrderedHash[ [ + [1, 2], + [3, 4], + "bad key value pair", + [ 'missing value' ] + ]] + + assert_kind_of ActiveSupport::OrderedHash, alternate + assert_equal [1, 3, 'missing value'], alternate.keys + assert_equal [2, 4, nil ], alternate.values + end + + def test_alternate_initialization_raises_exception_on_odd_length_args + begin + alternate = ActiveSupport::OrderedHash[1,2,3,4,5] + flunk "Hash::[] should have raised an exception on initialization " + + "with an odd number of parameters" + rescue + assert_equal "odd number of arguments for Hash", $!.message + end + end end -- cgit v1.2.3 From 0fbf458b6c7992a1626282727a70636af19450d5 Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Thu, 2 Jul 2009 12:14:23 -0700 Subject: Removed unnecessary calls to image_path and hash lookups [#2827 state:resolved] --- actionpack/lib/action_view/helpers/asset_tag_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index a3b99327ce..6d2c28f969 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -505,8 +505,8 @@ module ActionView def image_tag(source, options = {}) options.symbolize_keys! - options[:src] = path_to_image(source) - options[:alt] ||= File.basename(options[:src], '.*').split('.').first.to_s.capitalize + src = options[:src] = path_to_image(source) + options[:alt] ||= File.basename(src, '.*').split('.').first.to_s.capitalize if size = options.delete(:size) options[:width], options[:height] = size.split("x") if size =~ %r{^\d+x\d+$} @@ -514,7 +514,7 @@ module ActionView if mouseover = options.delete(:mouseover) options[:onmouseover] = "this.src='#{image_path(mouseover)}'" - options[:onmouseout] = "this.src='#{image_path(options[:src])}'" + options[:onmouseout] = "this.src='#{src}'" end tag("img", options) -- cgit v1.2.3 From fc6077d76ecbf20d29e9963afcabfd613d839091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Garc=C3=ADa=20S=C3=A1ez?= Date: Tue, 23 Jun 2009 22:54:32 +0200 Subject: Allow symbols on routes declaration (:controller and :action values) [#2828 state:resolved] Signed-off-by: Yehuda Katz + Carl Lerche --- actionpack/lib/action_controller/routing/route_set.rb | 1 + actionpack/test/controller/routing_test.rb | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/actionpack/lib/action_controller/routing/route_set.rb b/actionpack/lib/action_controller/routing/route_set.rb index 87b4b0571c..f5a4b1e1db 100644 --- a/actionpack/lib/action_controller/routing/route_set.rb +++ b/actionpack/lib/action_controller/routing/route_set.rb @@ -305,6 +305,7 @@ module ActionController end def add_route(path, options = {}) + options.each { |k, v| options[k] = v.to_s if [:controller, :action].include?(k) && v.is_a?(Symbol) } route = builder.build(path, options) routes << route route diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index 16d7df4843..fb83dba395 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -2492,6 +2492,16 @@ class RouteSetTest < Test::Unit::TestCase end assert_equal({:controller => 'pages', :action => 'show', :name => 'JAMIS'}, set.recognize_path('/page/JAMIS')) end + + def test_routes_with_symbols + set.draw do |map| + map.connect 'unnamed', :controller => :pages, :action => :show, :name => :as_symbol + map.named 'named', :controller => :pages, :action => :show, :name => :as_symbol + end + assert_equal({:controller => 'pages', :action => 'show', :name => :as_symbol}, set.recognize_path('/unnamed')) + assert_equal({:controller => 'pages', :action => 'show', :name => :as_symbol}, set.recognize_path('/named')) + end + end class RouteLoadingTest < Test::Unit::TestCase -- cgit v1.2.3 From 913bb2f4c2feb79dcbc9ed2c0fb1ef6d436f7d02 Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Thu, 2 Jul 2009 15:47:11 -0700 Subject: Modify the Rails::Application::Path object to allow for more concise path definition. --- railties/lib/rails/paths.rb | 40 ++++++++++++-------- railties/test/paths_test.rb | 92 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 113 insertions(+), 19 deletions(-) diff --git a/railties/lib/rails/paths.rb b/railties/lib/rails/paths.rb index d2f6d83659..ca56199cbc 100644 --- a/railties/lib/rails/paths.rb +++ b/railties/lib/rails/paths.rb @@ -6,8 +6,8 @@ module Rails def method_missing(id, *args) name = id.to_s - if name =~ /^(.*)=$/ - @children[$1] = Path.new(args.first, @root) + if name =~ /^(.*)=$/ || args.any? + @children[$1 || name] = Path.new(@root, *args) elsif path = @children[name] path else @@ -28,17 +28,15 @@ module Rails # TODO: Move logic from set_root_path initializer @path = File.expand_path(path) @root = self - @load_once, @eager_load, @all_paths = [], [], [] + @all_paths = [] end def load_once - @load_once.uniq! - @load_once + all_paths.map { |path| path.paths if path.load_once? }.compact.flatten.uniq end def eager_load - @eager_load.uniq! - @eager_load + all_paths.map { |path| path.paths if path.eager_load? }.compact.flatten.uniq end def all_paths @@ -47,7 +45,7 @@ module Rails end def load_paths - all_paths.map { |path| path.paths }.flatten + all_paths.map { |path| path.paths if path.load_path? }.compact.flatten.uniq end def add_to_load_path @@ -55,6 +53,14 @@ module Rails $LOAD_PATH.unshift(path) if File.directory?(path) end end + + def push(*) + raise "Application root can only have one physical path" + end + + alias unshift push + alias << push + alias concat push end class Path @@ -63,11 +69,18 @@ module Rails attr_reader :path attr_accessor :glob - def initialize(path, root) + def initialize(root, *paths) + @options = paths.extract_options! @children = {} @root = root - @paths = [path].flatten - @glob = "**/*.rb" + @paths = paths.flatten + @glob = @options[:glob] || "**/*.rb" + + @load_once = @options[:load_once] + @eager_load = @options[:eager_load] + @load_path = @options[:load_path] || @eager_load + + @root.all_paths << self end def push(path) @@ -86,7 +99,6 @@ module Rails def load_once! @load_once = true - @root.load_once.push *self.paths end def load_once? @@ -95,8 +107,7 @@ module Rails def eager_load! @eager_load = true - @root.all_paths << self - @root.eager_load.push *self.paths + @load_path = true end def eager_load? @@ -105,7 +116,6 @@ module Rails def load_path! @load_path = true - @root.all_paths << self end def load_path? diff --git a/railties/test/paths_test.rb b/railties/test/paths_test.rb index fa2f6ceee2..d50882110f 100644 --- a/railties/test/paths_test.rb +++ b/railties/test/paths_test.rb @@ -17,17 +17,37 @@ class PathsTest < ActiveSupport::TestCase assert_equal ["/foo/bar"], @root.app.to_a end + test "creating a root level path without assignment" do + @root.app "/foo/bar" + assert_equal ["/foo/bar"], @root.app.to_a + end + + test "trying to access a path that does not exist raises NoMethodError" do + assert_raises(NoMethodError) { @root.app } + end + test "relative paths are relative to the paths root" do @root.app = "app" assert_equal ["/foo/bar/app"], @root.app.to_a end + test "relative paths are relative to the paths root without assignment" do + @root.app "app" + assert_equal ["/foo/bar/app"], @root.app.to_a + end + test "creating a child level path" do @root.app = "/foo/bar" @root.app.models = "/foo/bar/baz" assert_equal ["/foo/bar/baz"], @root.app.models.to_a end + test "creating a child level path without assignment" do + @root.app = "/foo/bar" + @root.app.models "/foo/bar/baz" + assert_equal ["/foo/bar/baz"], @root.app.models.to_a + end + test "child level paths are relative from the root" do @root.app = "/app" @root.app.models = "baz" @@ -40,6 +60,11 @@ class PathsTest < ActiveSupport::TestCase assert_equal ["/app", "/app2"], @root.app.to_a end + test "adding multiple physical paths as an array without assignment" do + @root.app "/app", "/app2" + assert_equal ["/app", "/app2"], @root.app.to_a + end + test "adding multiple physical paths using #push" do @root.app = "/app" @root.app.push "/app2" @@ -66,10 +91,10 @@ class PathsTest < ActiveSupport::TestCase test "the root can only have one physical path" do assert_raise(RuntimeError) { Rails::Application::Root.new(["/fiz", "/biz"]) } - assert_raise(NoMethodError) { @root.push "/biz" } - assert_raise(NoMethodError) { @root.unshift "/biz" } - assert_raise(NoMethodError) { @root.concat ["/biz"]} - assert_raise(NoMethodError) { @root << "/biz" } + assert_raise(RuntimeError) { @root.push "/biz" } + assert_raise(RuntimeError) { @root.unshift "/biz" } + assert_raise(RuntimeError) { @root.concat ["/biz"]} + assert_raise(RuntimeError) { @root << "/biz" } end test "it is possible to add a path that should be loaded only once" do @@ -79,6 +104,19 @@ class PathsTest < ActiveSupport::TestCase assert @root.load_once.include?(@root.app.paths.first) end + test "it is possible to add a path without assignment and specify it should be loaded only once" do + @root.app "/app", :load_once => true + assert @root.app.load_once? + assert @root.load_once.include?("/app") + end + + test "it is possible to add multiple paths without assignment and specify it should be loaded only once" do + @root.app "/app", "/app2", :load_once => true + assert @root.app.load_once? + assert @root.load_once.include?("/app") + assert @root.load_once.include?("/app2") + end + test "making a path load_once more than once only includes it once in @root.load_once" do @root.app = "/app" @root.app.load_once! @@ -86,6 +124,13 @@ class PathsTest < ActiveSupport::TestCase assert_equal 1, @root.load_once.select {|p| p == @root.app.paths.first }.size end + test "paths added to a load_once path should be added to the load_once collection" do + @root.app = "/app" + @root.app.load_once! + @root.app << "/app2" + assert_equal 2, @root.load_once.size + end + test "it is possible to mark a path as eager" do @root.app = "/app" @root.app.eager_load! @@ -93,6 +138,27 @@ class PathsTest < ActiveSupport::TestCase assert @root.eager_load.include?(@root.app.paths.first) end + test "it is possible to add a path without assignment and mark it as eager" do + @root.app "/app", :eager_load => true + assert @root.app.eager_load? + assert @root.eager_load.include?("/app") + end + + test "it is possible to add multiple paths without assignment and mark them as eager" do + @root.app "/app", "/app2", :eager_load => true + assert @root.app.eager_load? + assert @root.eager_load.include?("/app") + assert @root.eager_load.include?("/app2") + end + + test "it is possible to create a path without assignment and mark it both as eager and load once" do + @root.app "/app", :eager_load => true, :load_once => true + assert @root.app.eager_load? + assert @root.app.load_once? + assert @root.eager_load.include?("/app") + assert @root.load_once.include?("/app") + end + test "making a path eager more than once only includes it once in @root.eager_paths" do @root.app = "/app" @root.app.eager_load! @@ -100,6 +166,13 @@ class PathsTest < ActiveSupport::TestCase assert_equal 1, @root.eager_load.select {|p| p == @root.app.paths.first }.size end + test "paths added to a eager_load path should be added to the eager_load collection" do + @root.app = "/app" + @root.app.eager_load! + @root.app << "/app2" + assert_equal 2, @root.eager_load.size + end + test "a path should have a glob that defaults to **/*.rb" do @root.app = "/app" assert_equal "**/*.rb", @root.app.glob @@ -111,6 +184,11 @@ class PathsTest < ActiveSupport::TestCase assert_equal "*.rb", @root.app.glob end + test "it should be possible to override a path's default glob without assignment" do + @root.app "/app", :glob => "*.rb" + assert_equal "*.rb", @root.app.glob + end + test "a path can be added to the load path" do @root.app = "app" @root.app.load_path! @@ -118,6 +196,12 @@ class PathsTest < ActiveSupport::TestCase assert_equal ["/foo/bar/app"], @root.load_paths end + test "a path can be added to the load path on creation" do + @root.app "/app", :load_path => true + assert @root.app.load_path? + assert_equal ["/app"], @root.load_paths + end + test "adding a path to the eager paths also adds it to the load path" do @root.app = "app" @root.app.eager_load! -- cgit v1.2.3 From 940aad225af0b963f435a0bf015daece2218d502 Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Thu, 2 Jul 2009 15:49:35 -0700 Subject: Compact the way application paths are defined --- railties/lib/rails/configuration.rb | 40 ++++++++++++------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index d877915460..1a2f217d20 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -62,33 +62,19 @@ module Rails end @paths = Rails::Application::Root.new(root_path) - @paths.app = "app" - @paths.app.metals = "app/metal" - @paths.app.models = "app/models" - @paths.app.controllers = "app/controllers" - @paths.app.helpers = "app/helpers" - @paths.app.services = "app/services" - @paths.lib = "lib" - @paths.vendor = "vendor" - @paths.vendor.plugins = "vendor/plugins" - @paths.cache = "tmp/cache" - @paths.config = "config" - @paths.config.locales = "config/locales" - @paths.config.environments = "config/environments" - - @paths.app.controllers.concat builtin_directories - - @paths.app.load_path! - @paths.app.metals.load_path! - @paths.app.models.eager_load! - @paths.app.controllers.eager_load! - @paths.app.helpers.eager_load! - @paths.app.services.load_path! - @paths.app.metals.eager_load! - @paths.lib.load_path! - @paths.vendor.load_path! - - @paths.config.environments.glob = "#{RAILS_ENV}.rb" + @paths.app "app", :load_path => true + @paths.app.metals "app/metal", :eager_load => true + @paths.app.models "app/models", :eager_load => true + @paths.app.controllers "app/controllers", builtin_directories, :eager_load => true + @paths.app.helpers "app/helpers", :eager_load => true + @paths.app.services "app/services", :load_path => true + @paths.lib "lib", :load_path => true + @paths.vendor "vendor", :load_path => true + @paths.vendor.plugins "vendor/plugins" + @paths.cache "tmp/cache" + @paths.config "config" + @paths.config.locales "config/locales" + @paths.config.environments "config/environments", :glob => "#{RAILS_ENV}.rb" RAILS_ROOT.replace root_path end -- cgit v1.2.3 From 1d280e21a19aff74e1b35779be2633e6efa511f0 Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Thu, 2 Jul 2009 16:03:41 -0700 Subject: Adds support for def self.setup in isolation tests for setup that should be run only once in the parent --- activesupport/lib/active_support/testing/isolation.rb | 5 +++++ activesupport/test/isolation_test.rb | 17 +++++++++++++++-- railties/test/initializer/path_test.rb | 11 ++++++----- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/activesupport/lib/active_support/testing/isolation.rb b/activesupport/lib/active_support/testing/isolation.rb index 090e0c5c89..dd13abcd5d 100644 --- a/activesupport/lib/active_support/testing/isolation.rb +++ b/activesupport/lib/active_support/testing/isolation.rb @@ -21,6 +21,11 @@ module ActiveSupport::Testing end def run(result) + unless defined?(@@ran_class_setup) + self.class.setup + @@ran_class_setup = true + end + yield(Test::Unit::TestCase::STARTED, name) @_result = result diff --git a/activesupport/test/isolation_test.rb b/activesupport/test/isolation_test.rb index b844bbb673..5a1f285476 100644 --- a/activesupport/test/isolation_test.rb +++ b/activesupport/test/isolation_test.rb @@ -5,6 +5,12 @@ if ENV['CHILD'] class ChildIsolationTest < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation + def self.setup + File.open(File.join(File.dirname(__FILE__), "fixtures", "isolation_test"), "a") do |f| + f.puts "hello" + end + end + def setup @instance = "HELLO" end @@ -64,6 +70,8 @@ if ENV['CHILD'] else class ParentIsolationTest < ActiveSupport::TestCase + File.open(File.join(File.dirname(__FILE__), "fixtures", "isolation_test"), "w") {} + ENV["CHILD"] = "1" OUTPUT = `#{Gem.ruby} -I#{File.dirname(__FILE__)} #{File.expand_path(__FILE__)} -v` ENV.delete("CHILD") @@ -131,12 +139,17 @@ else test "backtrace is printed for errors" do assert_equal 'Error', @backtraces["test_captures_errors"][:type] - assert_match %{isolation_test.rb:21:in `test_captures_errors'}, @backtraces["test_captures_errors"][:output] + assert_match %r{isolation_test.rb:\d+:in `test_captures_errors'}, @backtraces["test_captures_errors"][:output] end test "backtrace is printed for failures" do assert_equal 'Failure', @backtraces["test_captures_failures"][:type] - assert_match %{isolation_test.rb:25:in `test_captures_failures'}, @backtraces["test_captures_failures"][:output] + assert_match %r{isolation_test.rb:\d+:in `test_captures_failures'}, @backtraces["test_captures_failures"][:output] + end + + test "self.setup is run only once" do + text = File.read(File.join(File.dirname(__FILE__), "fixtures", "isolation_test")) + assert_equal "hello\n", text end end diff --git a/railties/test/initializer/path_test.rb b/railties/test/initializer/path_test.rb index 8fbad24a73..db62796ea5 100644 --- a/railties/test/initializer/path_test.rb +++ b/railties/test/initializer/path_test.rb @@ -8,14 +8,15 @@ module Rails def self.vendor_rails? ; false ; end end -# TODO: Can this be reset? -Rails::Initializer.run do |config| - config.frameworks = [:action_controller, :action_view, :action_mailer, :active_record] -end - class PathsTest < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation + def self.setup + Rails::Initializer.run do |config| + config.frameworks = [:action_controller, :action_view, :action_mailer, :active_record] + end + end + def setup @paths = Rails::Initializer.default.config.paths end -- cgit v1.2.3 From d8406f0c20e7809ce797d37c509e2f58f07109d2 Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Thu, 2 Jul 2009 16:51:05 -0700 Subject: Wrote tests for the :check_ruby_version initializer --- .../test/initializer/check_ruby_version_test.rb | 59 ++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 railties/test/initializer/check_ruby_version_test.rb diff --git a/railties/test/initializer/check_ruby_version_test.rb b/railties/test/initializer/check_ruby_version_test.rb new file mode 100644 index 0000000000..12503fb481 --- /dev/null +++ b/railties/test/initializer/check_ruby_version_test.rb @@ -0,0 +1,59 @@ +require 'abstract_unit' +require 'active_support/ruby/shim' +require 'initializer' + +RAILS_ROOT.replace File.join(File.dirname(__FILE__), "root") + +module Rails + def self.vendor_rails? ; false ; end +end + +module InitializerTests + class PathsTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + + test "rails does not initialize with ruby version 1.8.1" do + assert_rails_does_not_boot "1.8.1" + end + + test "rails initializes with ruby version 1.8.2" do + assert_rails_boots "1.8.2" + end + + test "rails does not initialize with ruby version 1.8.3" do + assert_rails_does_not_boot "1.8.3" + end + + test "rails initializes with ruby version 1.8.4" do + assert_rails_boots "1.8.4" + end + + test "rails initializes with ruby version 1.8.5" do + assert_rails_boots "1.8.5" + end + + test "rails initializes with ruby version 1.8.6" do + assert_rails_boots "1.8.6" + end + + def set_ruby_version(version) + $-w = nil + Object.const_set(:RUBY_VERSION, version.freeze) + end + + def assert_rails_boots(version) + set_ruby_version(version) + assert_nothing_raised "It appears that rails does not boot" do + Rails::Initializer.run { |c| c.frameworks = [] } + end + end + + def assert_rails_does_not_boot(version) + set_ruby_version(version) + $stderr = File.open("/dev/null", "w") + assert_raises(SystemExit) do + Rails::Initializer.run { |c| c.frameworks = [] } + end + end + end +end -- cgit v1.2.3 From 378a65a909439ebca909125fdfada23ed89cec63 Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Thu, 2 Jul 2009 17:52:46 -0700 Subject: Added tests for the :install_gem_spec_stubs initializer --- railties/lib/initializer.rb | 7 +- .../test/initializer/check_ruby_version_test.rb | 10 +-- .../initializer/install_gem_spec_stubs_test.rb | 85 ++++++++++++++++++++++ railties/test/initializer/path_test.rb | 10 +-- railties/test/initializer/test_helper.rb | 24 ++++++ railties/test/initializer_test.rb | 2 +- railties/test/plugin_test_helper.rb | 2 +- 7 files changed, 115 insertions(+), 25 deletions(-) create mode 100644 railties/test/initializer/install_gem_spec_stubs_test.rb create mode 100644 railties/test/initializer/test_helper.rb diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index cd23158e98..560105670f 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -125,11 +125,8 @@ module Rails if Rails.vendor_rails? begin; require "rubygems"; rescue LoadError; return; end - stubs = %w(rails activesupport activerecord actionpack actionmailer activeresource) - stubs.reject! { |s| Gem.loaded_specs.key?(s) } - - stubs.each do |stub| - Gem.loaded_specs[stub] = Gem::Specification.new do |s| + %w(rails activesupport activerecord actionpack actionmailer activeresource).each do |stub| + Gem.loaded_specs[stub] ||= Gem::Specification.new do |s| s.name = stub s.version = Rails::VERSION::STRING s.loaded_from = "" diff --git a/railties/test/initializer/check_ruby_version_test.rb b/railties/test/initializer/check_ruby_version_test.rb index 12503fb481..33de653906 100644 --- a/railties/test/initializer/check_ruby_version_test.rb +++ b/railties/test/initializer/check_ruby_version_test.rb @@ -1,12 +1,4 @@ -require 'abstract_unit' -require 'active_support/ruby/shim' -require 'initializer' - -RAILS_ROOT.replace File.join(File.dirname(__FILE__), "root") - -module Rails - def self.vendor_rails? ; false ; end -end +require "initializer/test_helper" module InitializerTests class PathsTest < ActiveSupport::TestCase diff --git a/railties/test/initializer/install_gem_spec_stubs_test.rb b/railties/test/initializer/install_gem_spec_stubs_test.rb new file mode 100644 index 0000000000..2e94c9968f --- /dev/null +++ b/railties/test/initializer/install_gem_spec_stubs_test.rb @@ -0,0 +1,85 @@ +require "initializer/test_helper" + +module InitializerTests + class GemSpecStubsTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + + def setup + $stderr = StringIO.new + end + + test "user has an old boot.rb (defined by having no Rails.vendor_rails?)" do + class << Rails + undef vendor_rails? + end + + assert_stderr(/outdated/) do + assert_raises(SystemExit) do + Rails::Initializer.run { |c| c.frameworks = [] } + end + end + end + + test "requires rubygems" do + Kernel.module_eval do + alias old_require require + def require(name) + $rubygems_required = true if name == "rubygems" + old_require(name) + end + end + + Rails.vendor_rails = true + Rails::Initializer.run { |c| c.frameworks = [] } + assert $rubygems_required + end + + test "does not fail if rubygems does not exist" do + Kernel.module_eval do + alias old_require require + def require(name) + raise LoadError if name == "rubygems" + old_require(name) + end + end + + assert_nothing_raised do + Rails::Initializer.run { |c| c.frameworks = [] } + end + end + + test "adds fake Rubygems stubs if a framework is not loaded in Rubygems and we've vendored" do + Rails.vendor_rails = true + + Rails::Initializer.run { |c| c.frameworks = [] } + + %w(rails activesupport activerecord actionpack actionmailer activeresource).each do |stub| + gem_spec = Gem.loaded_specs[stub] + assert_equal Gem::Version.new(Rails::VERSION::STRING), gem_spec.version + assert_equal stub, gem_spec.name + assert_equal "", gem_spec.loaded_from + end + end + + test "doesn't replace gem specs that are already loaded" do + Rails.vendor_rails = true + + Gem.loaded_specs["rails"] = Gem::Specification.new do |s| + s.name = "rails" + s.version = Rails::VERSION::STRING + s.loaded_from = "/foo/bar/baz" + end + + Rails::Initializer.run { |c| c.frameworks = [] } + + assert_equal "/foo/bar/baz", Gem.loaded_specs["rails"].loaded_from + + %w(activesupport activerecord actionpack actionmailer activeresource).each do |stub| + gem_spec = Gem.loaded_specs[stub] + assert_equal Gem::Version.new(Rails::VERSION::STRING), gem_spec.version + assert_equal stub, gem_spec.name + assert_equal "", gem_spec.loaded_from + end + end + end +end \ No newline at end of file diff --git a/railties/test/initializer/path_test.rb b/railties/test/initializer/path_test.rb index db62796ea5..26f796f93d 100644 --- a/railties/test/initializer/path_test.rb +++ b/railties/test/initializer/path_test.rb @@ -1,12 +1,4 @@ -require 'abstract_unit' -require 'active_support/ruby/shim' -require 'initializer' - -RAILS_ROOT.replace File.join(File.dirname(__FILE__), "root") - -module Rails - def self.vendor_rails? ; false ; end -end +require "initializer/test_helper" class PathsTest < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation diff --git a/railties/test/initializer/test_helper.rb b/railties/test/initializer/test_helper.rb new file mode 100644 index 0000000000..ddb03397ab --- /dev/null +++ b/railties/test/initializer/test_helper.rb @@ -0,0 +1,24 @@ +require 'abstract_unit' +require 'active_support/ruby/shim' +require 'initializer' + +RAILS_ROOT.replace File.join(File.dirname(__FILE__), "root") + +module Rails + class << self + attr_accessor :vendor_rails + def vendor_rails?() @vendor_rails end + end +end + +class ActiveSupport::TestCase + def assert_stderr(match) + $stderr = StringIO.new + yield + $stderr.rewind + err = $stderr.read + assert_match match, err + ensure + $stderr = STDERR + end +end \ No newline at end of file diff --git a/railties/test/initializer_test.rb b/railties/test/initializer_test.rb index 5caa5858a4..550cb7de76 100644 --- a/railties/test/initializer_test.rb +++ b/railties/test/initializer_test.rb @@ -178,7 +178,7 @@ class ConfigurationFrameworkPathsTests < Test::Unit::TestCase end end -require File.dirname(__FILE__) + '/plugin_test_helper' +require 'plugin_test_helper' class InitializerPluginLoadingTests < Test::Unit::TestCase def setup diff --git a/railties/test/plugin_test_helper.rb b/railties/test/plugin_test_helper.rb index 55d1a1fa96..893095fa66 100644 --- a/railties/test/plugin_test_helper.rb +++ b/railties/test/plugin_test_helper.rb @@ -4,7 +4,7 @@ $:.unshift File.dirname(__FILE__) + "/../../activesupport/lib" require 'test/unit' require 'active_support' require 'initializer' -require File.join(File.dirname(__FILE__), 'abstract_unit') +require 'abstract_unit' # We need to set RAILS_ROOT if it isn't already set RAILS_ROOT = '.' unless defined?(RAILS_ROOT) -- cgit v1.2.3 From 3c1dab72259d01c6335bf359d7f9b3af69d45bb4 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 3 Jul 2009 12:23:57 +0100 Subject: Revert "Modify the Rails::Application::Path object to allow for more concise path definition." This reverts commit 913bb2f4c2feb79dcbc9ed2c0fb1ef6d436f7d02. Reason : The server does not start --- railties/lib/rails/paths.rb | 40 ++++++++------------ railties/test/paths_test.rb | 92 ++------------------------------------------- 2 files changed, 19 insertions(+), 113 deletions(-) diff --git a/railties/lib/rails/paths.rb b/railties/lib/rails/paths.rb index ca56199cbc..d2f6d83659 100644 --- a/railties/lib/rails/paths.rb +++ b/railties/lib/rails/paths.rb @@ -6,8 +6,8 @@ module Rails def method_missing(id, *args) name = id.to_s - if name =~ /^(.*)=$/ || args.any? - @children[$1 || name] = Path.new(@root, *args) + if name =~ /^(.*)=$/ + @children[$1] = Path.new(args.first, @root) elsif path = @children[name] path else @@ -28,15 +28,17 @@ module Rails # TODO: Move logic from set_root_path initializer @path = File.expand_path(path) @root = self - @all_paths = [] + @load_once, @eager_load, @all_paths = [], [], [] end def load_once - all_paths.map { |path| path.paths if path.load_once? }.compact.flatten.uniq + @load_once.uniq! + @load_once end def eager_load - all_paths.map { |path| path.paths if path.eager_load? }.compact.flatten.uniq + @eager_load.uniq! + @eager_load end def all_paths @@ -45,7 +47,7 @@ module Rails end def load_paths - all_paths.map { |path| path.paths if path.load_path? }.compact.flatten.uniq + all_paths.map { |path| path.paths }.flatten end def add_to_load_path @@ -53,14 +55,6 @@ module Rails $LOAD_PATH.unshift(path) if File.directory?(path) end end - - def push(*) - raise "Application root can only have one physical path" - end - - alias unshift push - alias << push - alias concat push end class Path @@ -69,18 +63,11 @@ module Rails attr_reader :path attr_accessor :glob - def initialize(root, *paths) - @options = paths.extract_options! + def initialize(path, root) @children = {} @root = root - @paths = paths.flatten - @glob = @options[:glob] || "**/*.rb" - - @load_once = @options[:load_once] - @eager_load = @options[:eager_load] - @load_path = @options[:load_path] || @eager_load - - @root.all_paths << self + @paths = [path].flatten + @glob = "**/*.rb" end def push(path) @@ -99,6 +86,7 @@ module Rails def load_once! @load_once = true + @root.load_once.push *self.paths end def load_once? @@ -107,7 +95,8 @@ module Rails def eager_load! @eager_load = true - @load_path = true + @root.all_paths << self + @root.eager_load.push *self.paths end def eager_load? @@ -116,6 +105,7 @@ module Rails def load_path! @load_path = true + @root.all_paths << self end def load_path? diff --git a/railties/test/paths_test.rb b/railties/test/paths_test.rb index d50882110f..fa2f6ceee2 100644 --- a/railties/test/paths_test.rb +++ b/railties/test/paths_test.rb @@ -17,37 +17,17 @@ class PathsTest < ActiveSupport::TestCase assert_equal ["/foo/bar"], @root.app.to_a end - test "creating a root level path without assignment" do - @root.app "/foo/bar" - assert_equal ["/foo/bar"], @root.app.to_a - end - - test "trying to access a path that does not exist raises NoMethodError" do - assert_raises(NoMethodError) { @root.app } - end - test "relative paths are relative to the paths root" do @root.app = "app" assert_equal ["/foo/bar/app"], @root.app.to_a end - test "relative paths are relative to the paths root without assignment" do - @root.app "app" - assert_equal ["/foo/bar/app"], @root.app.to_a - end - test "creating a child level path" do @root.app = "/foo/bar" @root.app.models = "/foo/bar/baz" assert_equal ["/foo/bar/baz"], @root.app.models.to_a end - test "creating a child level path without assignment" do - @root.app = "/foo/bar" - @root.app.models "/foo/bar/baz" - assert_equal ["/foo/bar/baz"], @root.app.models.to_a - end - test "child level paths are relative from the root" do @root.app = "/app" @root.app.models = "baz" @@ -60,11 +40,6 @@ class PathsTest < ActiveSupport::TestCase assert_equal ["/app", "/app2"], @root.app.to_a end - test "adding multiple physical paths as an array without assignment" do - @root.app "/app", "/app2" - assert_equal ["/app", "/app2"], @root.app.to_a - end - test "adding multiple physical paths using #push" do @root.app = "/app" @root.app.push "/app2" @@ -91,10 +66,10 @@ class PathsTest < ActiveSupport::TestCase test "the root can only have one physical path" do assert_raise(RuntimeError) { Rails::Application::Root.new(["/fiz", "/biz"]) } - assert_raise(RuntimeError) { @root.push "/biz" } - assert_raise(RuntimeError) { @root.unshift "/biz" } - assert_raise(RuntimeError) { @root.concat ["/biz"]} - assert_raise(RuntimeError) { @root << "/biz" } + assert_raise(NoMethodError) { @root.push "/biz" } + assert_raise(NoMethodError) { @root.unshift "/biz" } + assert_raise(NoMethodError) { @root.concat ["/biz"]} + assert_raise(NoMethodError) { @root << "/biz" } end test "it is possible to add a path that should be loaded only once" do @@ -104,19 +79,6 @@ class PathsTest < ActiveSupport::TestCase assert @root.load_once.include?(@root.app.paths.first) end - test "it is possible to add a path without assignment and specify it should be loaded only once" do - @root.app "/app", :load_once => true - assert @root.app.load_once? - assert @root.load_once.include?("/app") - end - - test "it is possible to add multiple paths without assignment and specify it should be loaded only once" do - @root.app "/app", "/app2", :load_once => true - assert @root.app.load_once? - assert @root.load_once.include?("/app") - assert @root.load_once.include?("/app2") - end - test "making a path load_once more than once only includes it once in @root.load_once" do @root.app = "/app" @root.app.load_once! @@ -124,13 +86,6 @@ class PathsTest < ActiveSupport::TestCase assert_equal 1, @root.load_once.select {|p| p == @root.app.paths.first }.size end - test "paths added to a load_once path should be added to the load_once collection" do - @root.app = "/app" - @root.app.load_once! - @root.app << "/app2" - assert_equal 2, @root.load_once.size - end - test "it is possible to mark a path as eager" do @root.app = "/app" @root.app.eager_load! @@ -138,27 +93,6 @@ class PathsTest < ActiveSupport::TestCase assert @root.eager_load.include?(@root.app.paths.first) end - test "it is possible to add a path without assignment and mark it as eager" do - @root.app "/app", :eager_load => true - assert @root.app.eager_load? - assert @root.eager_load.include?("/app") - end - - test "it is possible to add multiple paths without assignment and mark them as eager" do - @root.app "/app", "/app2", :eager_load => true - assert @root.app.eager_load? - assert @root.eager_load.include?("/app") - assert @root.eager_load.include?("/app2") - end - - test "it is possible to create a path without assignment and mark it both as eager and load once" do - @root.app "/app", :eager_load => true, :load_once => true - assert @root.app.eager_load? - assert @root.app.load_once? - assert @root.eager_load.include?("/app") - assert @root.load_once.include?("/app") - end - test "making a path eager more than once only includes it once in @root.eager_paths" do @root.app = "/app" @root.app.eager_load! @@ -166,13 +100,6 @@ class PathsTest < ActiveSupport::TestCase assert_equal 1, @root.eager_load.select {|p| p == @root.app.paths.first }.size end - test "paths added to a eager_load path should be added to the eager_load collection" do - @root.app = "/app" - @root.app.eager_load! - @root.app << "/app2" - assert_equal 2, @root.eager_load.size - end - test "a path should have a glob that defaults to **/*.rb" do @root.app = "/app" assert_equal "**/*.rb", @root.app.glob @@ -184,11 +111,6 @@ class PathsTest < ActiveSupport::TestCase assert_equal "*.rb", @root.app.glob end - test "it should be possible to override a path's default glob without assignment" do - @root.app "/app", :glob => "*.rb" - assert_equal "*.rb", @root.app.glob - end - test "a path can be added to the load path" do @root.app = "app" @root.app.load_path! @@ -196,12 +118,6 @@ class PathsTest < ActiveSupport::TestCase assert_equal ["/foo/bar/app"], @root.load_paths end - test "a path can be added to the load path on creation" do - @root.app "/app", :load_path => true - assert @root.app.load_path? - assert_equal ["/app"], @root.load_paths - end - test "adding a path to the eager paths also adds it to the load path" do @root.app = "app" @root.app.eager_load! -- cgit v1.2.3 From a4bdc00fec623f72592e663e6d7830eea0bc6ea4 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 3 Jul 2009 12:24:26 +0100 Subject: Revert "Compact the way application paths are defined" This reverts commit 940aad225af0b963f435a0bf015daece2218d502. Reason : The server does not start --- railties/lib/rails/configuration.rb | 40 +++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index 1a2f217d20..d877915460 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -62,19 +62,33 @@ module Rails end @paths = Rails::Application::Root.new(root_path) - @paths.app "app", :load_path => true - @paths.app.metals "app/metal", :eager_load => true - @paths.app.models "app/models", :eager_load => true - @paths.app.controllers "app/controllers", builtin_directories, :eager_load => true - @paths.app.helpers "app/helpers", :eager_load => true - @paths.app.services "app/services", :load_path => true - @paths.lib "lib", :load_path => true - @paths.vendor "vendor", :load_path => true - @paths.vendor.plugins "vendor/plugins" - @paths.cache "tmp/cache" - @paths.config "config" - @paths.config.locales "config/locales" - @paths.config.environments "config/environments", :glob => "#{RAILS_ENV}.rb" + @paths.app = "app" + @paths.app.metals = "app/metal" + @paths.app.models = "app/models" + @paths.app.controllers = "app/controllers" + @paths.app.helpers = "app/helpers" + @paths.app.services = "app/services" + @paths.lib = "lib" + @paths.vendor = "vendor" + @paths.vendor.plugins = "vendor/plugins" + @paths.cache = "tmp/cache" + @paths.config = "config" + @paths.config.locales = "config/locales" + @paths.config.environments = "config/environments" + + @paths.app.controllers.concat builtin_directories + + @paths.app.load_path! + @paths.app.metals.load_path! + @paths.app.models.eager_load! + @paths.app.controllers.eager_load! + @paths.app.helpers.eager_load! + @paths.app.services.load_path! + @paths.app.metals.eager_load! + @paths.lib.load_path! + @paths.vendor.load_path! + + @paths.config.environments.glob = "#{RAILS_ENV}.rb" RAILS_ROOT.replace root_path end -- cgit v1.2.3