diff --git a/railties/CHANGELOG b/railties/CHANGELOG index 11fa6bdc37..1a64a26e8e 100644 --- a/railties/CHANGELOG +++ b/railties/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Thoroughly test the FCGI dispatcher. #5970 [Kevin Clark] + * Remove Dir.chdir in the Webrick DispatchServlet#initialize method. Fix bad path errors when trying to load config/routes.rb. [Rick Olson] * Tighten rescue clauses. #5985 [james@grayproductions.net] diff --git a/railties/lib/fcgi_handler.rb b/railties/lib/fcgi_handler.rb index 7b5aafe122..80ce1a34f5 100644 --- a/railties/lib/fcgi_handler.rb +++ b/railties/lib/fcgi_handler.rb @@ -50,25 +50,7 @@ class RailsFCGIHandler run_gc! if gc_request_period - provider.each_cgi do |cgi| - process_request(cgi) - - case when_ready - when :reload - reload! - when :restart - close_connection(cgi) - restart! - when :exit - close_connection(cgi) - break - when :breakpoint - close_connection(cgi) - breakpoint! - end - - gc_countdown - end + process_each_request!(provider) GC.enable dispatcher_log :info, "terminated gracefully" @@ -87,9 +69,9 @@ class RailsFCGIHandler dispatcher_error(fcgi_error, "killed by this error") end end - - - private + + + protected def logger @logger ||= Logger.new(@log_file_path) end @@ -145,6 +127,28 @@ class RailsFCGIHandler dispatcher_log :info, "asked to breakpoint ASAP" @when_ready = :breakpoint end + + def process_each_request!(provider) + provider.each_cgi do |cgi| + process_request(cgi) + + case when_ready + when :reload + reload! + when :restart + close_connection(cgi) + restart! + when :exit + close_connection(cgi) + break + when :breakpoint + close_connection(cgi) + breakpoint! + end + + gc_countdown + end + end def process_request(cgi) Dispatcher.dispatch(cgi) @@ -204,4 +208,4 @@ class RailsFCGIHandler def close_connection(cgi) cgi.instance_variable_get("@request").finish end -end \ No newline at end of file +end diff --git a/railties/test/abstract_unit.rb b/railties/test/abstract_unit.rb new file mode 100644 index 0000000000..eac36d41b3 --- /dev/null +++ b/railties/test/abstract_unit.rb @@ -0,0 +1,22 @@ +$:.unshift File.dirname(__FILE__) + "/../../activesupport/lib" +$:.unshift File.dirname(__FILE__) + "/../../actionpack/lib" +$:.unshift File.dirname(__FILE__) + "/../lib" +$:.unshift File.dirname(__FILE__) + "/../builtin/rails_info" + +require 'test/unit' +require 'rubygems' +require 'mocha' +require 'stubba' + +# Needed for the class mock delegation +#require File.dirname(__FILE__) + "/../../activesupport/lib/active_support/core_ext/class/attribute_accessors" + +if defined?(RAILS_ROOT) + RAILS_ROOT.replace File.dirname(__FILE__) +else + RAILS_ROOT = File.dirname(__FILE__) +end + +class Test::Unit::TestCase + # Add stuff here if you need it +end diff --git a/railties/test/fcgi_dispatcher_test.rb b/railties/test/fcgi_dispatcher_test.rb index cc96c2934c..303a4fcaeb 100644 --- a/railties/test/fcgi_dispatcher_test.rb +++ b/railties/test/fcgi_dispatcher_test.rb @@ -1,11 +1,14 @@ -$:.unshift File.dirname(__FILE__) + "/../lib" +require File.dirname(__FILE__) + "/abstract_unit" + $:.unshift File.dirname(__FILE__) + "/mocks" -require 'test/unit' require 'stringio' -require 'fcgi_handler' -RAILS_ROOT = File.dirname(__FILE__) if !defined?(RAILS_ROOT) +# Stubs +require 'fcgi_handler' +require 'routes' +require 'stubbed_breakpoint' +require 'stubbed_kernel' class RailsFCGIHandler attr_reader :exit_code @@ -26,13 +29,8 @@ class RailsFCGIHandler def send_signal(which) @signal_handlers[which].call(which) end - - def restore! - @reloaded = true - end - def reload! - @reloaded = true + def breakpoint end alias_method :old_run_gc!, :run_gc! @@ -53,6 +51,102 @@ class RailsFCGIHandlerTest < Test::Unit::TestCase Dispatcher.raise_exception = nil end + def test_process_restart + @handler.stubs(:when_ready).returns(:restart) + + @handler.expects(:close_connection) + @handler.expects(:restart!) + @handler.process! + end + + def test_process_exit + @handler.stubs(:when_ready).returns(:exit) + + @handler.expects(:close_connection) + @handler.process! + end + + def test_process_breakpoint + @handler.stubs(:when_ready).returns(:breakpoint) + + @handler.expects(:close_connection) + @handler.expects(:breakpoint!) + @handler.process! + end + + def test_process_with_system_exit_exception + @handler.stubs(:process_request).raises(SystemExit) + + @handler.expects(:dispatcher_log).with(:info, "terminated by explicit exit") + @handler.process! + end + + def test_restart_handler + @handler.expects(:dispatcher_log).with(:info, "asked to restart ASAP") + + @handler.send(:restart_handler, nil) + assert_equal :restart, @handler.when_ready + end + + def test_breakpoint_handler + @handler.expects(:dispatcher_log).with(:info, "asked to breakpoint ASAP") + + @handler.send(:breakpoint_handler, nil) + assert_equal :breakpoint, @handler.when_ready + end + + def test_install_signal_handler_should_log_on_bad_signal + @handler.stubs(:trap).raises(ArgumentError) + + @handler.expects(:dispatcher_log).with(:warn, "Ignoring unsupported signal CHEESECAKE.") + @handler.send(:install_signal_handler, "CHEESECAKE", nil) + end + + def test_reload + @handler.expects(:restore!) + @handler.expects(:dispatcher_log).with(:info, "reloaded") + + @handler.send(:reload!) + assert_nil @handler.when_ready + end + + + def test_reload_runs_gc_when_gc_request_period_set + @handler.expects(:run_gc!) + @handler.expects(:restore!) + @handler.expects(:dispatcher_log).with(:info, "reloaded") + @handler.gc_request_period = 10 + @handler.send(:reload!) + end + + def test_reload_doesnt_run_gc_if_gc_request_period_isnt_set + @handler.expects(:run_gc!).never + @handler.expects(:restore!) + @handler.expects(:dispatcher_log).with(:info, "reloaded") + @handler.send(:reload!) + end + + def test_restart! + @handler.expects(:dispatcher_log).with(:info, "restarted") + assert_equal true, @handler.send(:restart!), "Exec wasn't run" + end + + def test_restore! + $".expects(:replace) + Dispatcher.expects(:reset_application!) + ActionController::Routing::Routes.expects(:reload) + @handler.send(:restore!) + end + + def test_breakpoint! + @handler.expects(:require).with('breakpoint') + Breakpoint.expects(:activate_drb) + @handler.expects(:breakpoint) + @handler.expects(:dispatcher_log).with(:info, "breakpointing") + @handler.send(:breakpoint!) + assert_nil @handler.when_ready + end + def test_uninterrupted_processing @handler.process! assert_nil @handler.exit_code @@ -60,6 +154,7 @@ class RailsFCGIHandlerTest < Test::Unit::TestCase end def test_interrupted_via_HUP_when_not_in_request + @handler.expects(:reload!) FCGI.time_to_sleep = 1 @handler.thread = Thread.new { @handler.process! } sleep 0.1 # let the thread get started @@ -67,10 +162,11 @@ class RailsFCGIHandlerTest < Test::Unit::TestCase @handler.thread.join assert_nil @handler.exit_code assert_equal :reload, @handler.when_ready - assert @handler.reloaded end def test_interrupted_via_HUP_when_in_request + @handler.expects(:reload!) + Dispatcher.time_to_sleep = 1 @handler.thread = Thread.new { @handler.process! } sleep 0.1 # let the thread get started @@ -78,7 +174,6 @@ class RailsFCGIHandlerTest < Test::Unit::TestCase @handler.thread.join assert_nil @handler.exit_code assert_equal :reload, @handler.when_ready - assert @handler.reloaded end def test_interrupted_via_USR1_when_not_in_request diff --git a/railties/test/mocks/routes.rb b/railties/test/mocks/routes.rb new file mode 100644 index 0000000000..ea12863683 --- /dev/null +++ b/railties/test/mocks/routes.rb @@ -0,0 +1,6 @@ +module ActionController + module Routing + class Routes + end + end +end diff --git a/railties/test/mocks/stubbed_breakpoint.rb b/railties/test/mocks/stubbed_breakpoint.rb new file mode 100644 index 0000000000..15558282b1 --- /dev/null +++ b/railties/test/mocks/stubbed_breakpoint.rb @@ -0,0 +1,2 @@ +module Breakpoint +end diff --git a/railties/test/mocks/stubbed_kernel.rb b/railties/test/mocks/stubbed_kernel.rb new file mode 100644 index 0000000000..ef9864867c --- /dev/null +++ b/railties/test/mocks/stubbed_kernel.rb @@ -0,0 +1,5 @@ +module Kernel + def exec(*args) + true + end +end