From 9a42e06dd8ac5d9abd50b0e47e8de0ac3ab00a9d Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Mon, 6 Jul 2009 10:04:57 -0700 Subject: [PATCH 1/6] Reapply Rails::Application::Path tweaks Reapplies: a4bdc00fec623f72592e663e6d7830eea0bc6ea4 3c1dab72259d01c6335bf359d7f9b3af69d45bb4 --- railties/lib/rails/configuration.rb | 40 ++++--------- railties/lib/rails/paths.rb | 40 ++++++++----- railties/test/paths_test.rb | 92 +++++++++++++++++++++++++++-- 3 files changed, 126 insertions(+), 46 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 diff --git a/railties/lib/rails/paths.rb b/railties/lib/rails/paths.rb index d2f6d83659..3899b744b0 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.last.is_a?(::Hash) ? paths.pop : {} @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! From 61604feec0fa04810f5903d13b74bad06e67b3bb Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Mon, 6 Jul 2009 12:25:34 -0700 Subject: [PATCH 2/6] Get Initializer tests running without requiring parts of Rails being loaded first --- .../lib/active_support/testing/isolation.rb | 146 +++++++++--------- railties/lib/initializer.rb | 4 + .../initializer/check_ruby_version_test.rb | 2 +- .../install_gem_spec_stubs_test.rb | 29 ++-- railties/test/initializer/path_test.rb | 2 +- railties/test/initializer/test_helper.rb | 48 ++++-- 6 files changed, 131 insertions(+), 100 deletions(-) diff --git a/activesupport/lib/active_support/testing/isolation.rb b/activesupport/lib/active_support/testing/isolation.rb index dd13abcd5d..30e194536b 100644 --- a/activesupport/lib/active_support/testing/isolation.rb +++ b/activesupport/lib/active_support/testing/isolation.rb @@ -1,94 +1,96 @@ -module ActiveSupport::Testing - class ProxyTestResult - def initialize - @calls = [] - end +module ActiveSupport + module Testing + class ProxyTestResult + def initialize + @calls = [] + end - def __replay__(result) - @calls.each do |name, args| - result.send(name, *args) + def __replay__(result) + @calls.each do |name, args| + result.send(name, *args) + end + end + + def method_missing(name, *args) + @calls << [name, args] end end - def method_missing(name, *args) - @calls << [name, args] - end - end - - module Isolation - def self.forking_env? - !ENV["NO_FORK"] && RUBY_PLATFORM !~ /mswin|mingw|java/ - end - - def run(result) - unless defined?(@@ran_class_setup) - self.class.setup - @@ran_class_setup = true + module Isolation + def self.forking_env? + !ENV["NO_FORK"] && RUBY_PLATFORM !~ /mswin|mingw|java/ end - yield(Test::Unit::TestCase::STARTED, name) - - @_result = result - - proxy = run_in_isolation do |proxy| - super(proxy) { } - end - - proxy.__replay__(@_result) - - yield(Test::Unit::TestCase::FINISHED, name) - end - - module Forking - def run_in_isolation(&blk) - read, write = IO.pipe - - pid = fork do - read.close - proxy = ProxyTestResult.new - yield proxy - write.puts [Marshal.dump(proxy)].pack("m") - exit! + def run(result) + unless defined?(@@ran_class_setup) + self.class.setup if self.class.respond_to?(:setup) + @@ran_class_setup = true end - write.close - result = read.read - Process.wait2(pid) - Marshal.load(result.unpack("m")[0]) + yield(Test::Unit::TestCase::STARTED, name) + + @_result = result + + proxy = run_in_isolation do |proxy| + super(proxy) { } + end + + proxy.__replay__(@_result) + + yield(Test::Unit::TestCase::FINISHED, name) end - end - module Subprocess - # Crazy H4X to get this working in windows / jruby with - # no forking. - def run_in_isolation(&blk) - require "tempfile" + module Forking + def run_in_isolation(&blk) + read, write = IO.pipe - if ENV["ISOLATION_TEST"] - proxy = ProxyTestResult.new - yield proxy - File.open(ENV["ISOLATION_OUTPUT"], "w") do |file| - file.puts [Marshal.dump(proxy)].pack("m") + pid = fork do + read.close + proxy = ProxyTestResult.new + yield proxy + write.puts [Marshal.dump(proxy)].pack("m") + exit! end - exit! - else - Tempfile.open("isolation") do |tmpfile| - ENV["ISOLATION_TEST"] = @method_name - ENV["ISOLATION_OUTPUT"] = tmpfile.path - load_paths = $-I.map {|p| "-I\"#{File.expand_path(p)}\"" }.join(" ") - `#{Gem.ruby} #{load_paths} #{$0} #{ORIG_ARGV.join(" ")} -t\"#{self.class}\"` + write.close + result = read.read + Process.wait2(pid) + Marshal.load(result.unpack("m")[0]) + end + end - ENV.delete("ISOLATION_TEST") - ENV.delete("ISOLATION_OUTPUT") + module Subprocess + # Crazy H4X to get this working in windows / jruby with + # no forking. + def run_in_isolation(&blk) + require "tempfile" - return Marshal.load(tmpfile.read.unpack("m")[0]) + if ENV["ISOLATION_TEST"] + proxy = ProxyTestResult.new + yield proxy + File.open(ENV["ISOLATION_OUTPUT"], "w") do |file| + file.puts [Marshal.dump(proxy)].pack("m") + end + exit! + else + Tempfile.open("isolation") do |tmpfile| + ENV["ISOLATION_TEST"] = @method_name + ENV["ISOLATION_OUTPUT"] = tmpfile.path + + load_paths = $-I.map {|p| "-I\"#{File.expand_path(p)}\"" }.join(" ") + `#{Gem.ruby} #{load_paths} #{$0} #{ORIG_ARGV.join(" ")} -t\"#{self.class}\"` + + ENV.delete("ISOLATION_TEST") + ENV.delete("ISOLATION_OUTPUT") + + return Marshal.load(tmpfile.read.unpack("m")[0]) + end end end end - end - include forking_env? ? Forking : Subprocess + include forking_env? ? Forking : Subprocess + end end end diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 560105670f..f0fb78c8f4 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -12,6 +12,10 @@ require 'rails/configuration' RAILS_ENV = (ENV['RAILS_ENV'] || 'development').dup unless defined?(RAILS_ENV) module Rails + # Sanity check to make sure this file is only loaded once + # TODO: Get to the point where this can be removed. + raise "It looks like initializer.rb was required twice" if defined?(Initializer) + class Initializer class Error < StandardError ; end diff --git a/railties/test/initializer/check_ruby_version_test.rb b/railties/test/initializer/check_ruby_version_test.rb index 33de653906..68feba058e 100644 --- a/railties/test/initializer/check_ruby_version_test.rb +++ b/railties/test/initializer/check_ruby_version_test.rb @@ -1,7 +1,7 @@ require "initializer/test_helper" module InitializerTests - class PathsTest < ActiveSupport::TestCase + class PathsTest < Test::Unit::TestCase include ActiveSupport::Testing::Isolation test "rails does not initialize with ruby version 1.8.1" do diff --git a/railties/test/initializer/install_gem_spec_stubs_test.rb b/railties/test/initializer/install_gem_spec_stubs_test.rb index 2e94c9968f..cfb12d7405 100644 --- a/railties/test/initializer/install_gem_spec_stubs_test.rb +++ b/railties/test/initializer/install_gem_spec_stubs_test.rb @@ -1,7 +1,7 @@ require "initializer/test_helper" module InitializerTests - class GemSpecStubsTest < ActiveSupport::TestCase + class GemSpecStubsTest < Test::Unit::TestCase include ActiveSupport::Testing::Isolation def setup @@ -34,19 +34,20 @@ module InitializerTests 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 + # Pending until we're further along + # 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 diff --git a/railties/test/initializer/path_test.rb b/railties/test/initializer/path_test.rb index 26f796f93d..1b73cdc73e 100644 --- a/railties/test/initializer/path_test.rb +++ b/railties/test/initializer/path_test.rb @@ -1,6 +1,6 @@ require "initializer/test_helper" -class PathsTest < ActiveSupport::TestCase +class PathsTest < Test::Unit::TestCase include ActiveSupport::Testing::Isolation def self.setup diff --git a/railties/test/initializer/test_helper.rb b/railties/test/initializer/test_helper.rb index ddb03397ab..9d7dfff1c0 100644 --- a/railties/test/initializer/test_helper.rb +++ b/railties/test/initializer/test_helper.rb @@ -1,17 +1,18 @@ -require 'abstract_unit' -require 'active_support/ruby/shim' -require 'initializer' +# This is a test helper file that simulates a rails application being +# boot from scratch in vendored mode. This file should really only be +# required in test cases that use the isolation helper so that requires +# can be reset correctly. +RAILS_ROOT = File.join(File.dirname(__FILE__), "root") +RAILS_FRAMEWORK_ROOT = File.expand_path(File.join(File.dirname(__FILE__), '..', '..', '..')) -RAILS_ROOT.replace File.join(File.dirname(__FILE__), "root") +require "test/unit" +# We are purposely avoiding adding things to the load path to catch bugs that only happen in the genuine article +require File.join(RAILS_FRAMEWORK_ROOT, 'activesupport', 'lib', 'active_support', 'testing', 'isolation') +require File.join(RAILS_FRAMEWORK_ROOT, 'activesupport', 'lib', 'active_support', 'testing', 'declarative') -module Rails - class << self - attr_accessor :vendor_rails - def vendor_rails?() @vendor_rails end - end -end +class Test::Unit::TestCase + extend ActiveSupport::Testing::Declarative -class ActiveSupport::TestCase def assert_stderr(match) $stderr = StringIO.new yield @@ -21,4 +22,27 @@ class ActiveSupport::TestCase ensure $stderr = STDERR end -end \ No newline at end of file +end + +# Fake boot.rb +module Rails + class << self + attr_accessor :vendor_rails + + def vendor_rails? + @vendor_rails + end + + def boot! + # Require the initializer + require File.join(RAILS_FRAMEWORK_ROOT, 'railties', 'lib', 'initializer') + # Run the initializer the same way boot.rb does it + Rails::Initializer.run(:install_gem_spec_stubs) + Rails::GemDependency.add_frozen_gem_path + Rails::Initializer.run(:set_load_path) + end + end +end + +# All that for this: +Rails.boot! \ No newline at end of file From 1e2d7229602f467cfdc0ef606b5ef8a5566a1501 Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Fri, 3 Jul 2009 09:29:30 -0300 Subject: [PATCH 3/6] Adds a audio_tag helper for the HTML5 audio tag. Fixed video_path docs. HTML attributes values should be true or false not attribute's name. [#2864 state:resolved] Signed-off-by: Yehuda Katz --- .../action_view/helpers/asset_tag_helper.rb | 38 ++++++++++++++++-- .../lib/action_view/helpers/tag_helper.rb | 3 +- .../test/template/asset_tag_helper_test.rb | 40 +++++++++++++++++-- 3 files changed, 72 insertions(+), 9 deletions(-) diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 6d2c28f969..3fde79dfa4 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -462,12 +462,27 @@ module ActionView # video_path("hd") # => /videos/hd # video_path("hd.avi") # => /videos/hd.avi # video_path("trailers/hd.avi") # => /videos/trailers/hd.avi - # video_path("/trailers/hd.avi") # => /videos/hd.avi + # video_path("/trailers/hd.avi") # => /trailers/hd.avi # video_path("http://www.railsapplication.com/vid/hd.avi") # => http://www.railsapplication.com/vid/hd.avi def video_path(source) compute_public_path(source, 'videos') end - alias_method :path_to_video, :video_path # aliased to avoid conflicts with an video_path named route + alias_method :path_to_video, :video_path # aliased to avoid conflicts with a video_path named route + + # Computes the path to an audio asset in the public audios directory. + # Full paths from the document root will be passed through. + # Used internally by +audio_tag+ to build the audio path. + # + # ==== Examples + # audio_path("horse") # => /audios/horse + # audio_path("horse.wav") # => /audios/horse.avi + # audio_path("sounds/horse.wav") # => /audios/sounds/horse.avi + # audio_path("/sounds/horse.wav") # => /sounds/horse.avi + # audio_path("http://www.railsapplication.com/sounds/horse.wav") # => http://www.railsapplication.com/sounds/horse.wav + def audio_path(source) + compute_public_path(source, 'audios') + end + alias_method :path_to_audio, :audio_path # aliased to avoid conflicts with an audio_path named route # Returns an html image tag for the +source+. The +source+ can be a full # path or a file that exists in your public images directory. @@ -542,7 +557,7 @@ module ActionView # video_tag("trailer.ogg") # => #