Improve tests and code for Rails::InfoController and Rails::Info [#2411 state:resolved]

- Refactor Rails::InfoController tests.
- Return forbidden status from the InfoController for remote requests instead of a 500.
- Add tests for displaying middleware in Rails::Info.

Signed-off-by: Joshua Peek <josh@joshpeek.com>

3 files changed, 41 insertions, 32 deletions

diff --git a/railties/builtin/rails_info/rails/info_controller.rb b/railties/builtin/rails_info/rails/info_controller.rb
index 05745d606d..47e87c5bf5 100644
--- a/railties/builtin/rails_info/rails/info_controller.rb
+++ b/railties/builtin/rails_info/rails/info_controller.rb
@@ -3,7 +3,7 @@ class Rails::InfoController < ActionController::Base
     if consider_all_requests_local || local_request?
       render :inline => Rails::Info.to_html
     else
-      render :text => '<p>For security purposes, this information is only available to local requests.</p>', :status => 500
+      render :text => '<p>For security purposes, this information is only available to local requests.</p>', :status => :forbidden
     end
   end
 end
diff --git a/railties/test/rails_info_controller_test.rb b/railties/test/rails_info_controller_test.rb
index e274e1aa6e..607ece99a9 100644
--- a/railties/test/rails_info_controller_test.rb
+++ b/railties/test/rails_info_controller_test.rb
@@ -2,51 +2,45 @@ require 'abstract_unit'
 require 'action_controller'
 require 'action_controller/test_process'
 
-module Rails; end
 require 'rails/info'
 require 'rails/info_controller'
 
-class Rails::InfoController < ActionController::Base
-  @local_request = false
-  class << self
-    cattr_accessor :local_request
-  end
-  
-  # Re-raise errors caught by the controller.
-  def rescue_action(e) raise e end;
-  
-protected
-  def local_request?
-    self.class.local_request
-  end
-end
-
 ActionController::Routing::Routes.draw do |map|
   map.connect ':controller/:action/:id'
 end
 
-class Rails::InfoControllerTest < ActionController::TestCase
+class InfoControllerTest < ActionController::TestCase
+  tests Rails::InfoController
+
   def setup
-    @controller = Rails::InfoController.new
-    @request    = ActionController::TestRequest.new
-    @response   = ActionController::TestResponse.new
+    @controller.stubs(:consider_all_requests_local => false, :local_request? => true)
+  end
+
+  test "info controller does not allow remote requests" do
+    @controller.stubs(:consider_all_requests_local => false, :local_request? => false)
+    get :properties
+    assert_response :forbidden
+  end
 
-    ActionController::Base.consider_all_requests_local = true
+  test "info controller renders an error message when request was forbidden" do
+    @controller.stubs(:consider_all_requests_local => false, :local_request? => false)
+    get :properties
+    assert_select 'p'
+  end
+
+  test "info controller allows requests when all requests are considered local" do
+    @controller.stubs(:consider_all_requests_local => true, :local_request? => false)
+    get :properties
+    assert_response :success
   end
 
-  def test_rails_info_properties_table_rendered_for_local_request
-    Rails::InfoController.local_request = true
+  test "info controller allows local requests" do
     get :properties
-    assert_tag :tag => 'table'
     assert_response :success
   end
-  
-  def test_rails_info_properties_error_rendered_for_non_local_request
-    Rails::InfoController.local_request = false
-    ActionController::Base.consider_all_requests_local = false
 
+  test "info controller renders a table with properties" do
     get :properties
-    assert_tag :tag => 'p'
-    assert_response 500
+    assert_select 'table'
   end
 end
diff --git a/railties/test/rails_info_test.rb b/railties/test/rails_info_test.rb
index 9befd44a58..971cba89d0 100644
--- a/railties/test/rails_info_test.rb
+++ b/railties/test/rails_info_test.rb
@@ -1,9 +1,12 @@
 $:.unshift File.dirname(__FILE__) + "/../lib"
 $:.unshift File.dirname(__FILE__) + "/../builtin/rails_info"
 $:.unshift File.dirname(__FILE__) + "/../../activesupport/lib"
+$:.unshift File.dirname(__FILE__) + "/../../actionpack/lib"
 
 require 'test/unit'
 require 'active_support'
+require 'active_support/test_case'
+require 'action_controller'
 
 unless defined?(Rails) && defined?(Rails::Info)
   module Rails
@@ -11,7 +14,7 @@ unless defined?(Rails) && defined?(Rails::Info)
   end
 end
 
-class InfoTest < Test::Unit::TestCase
+class InfoTest < ActiveSupport::TestCase
   def setup
     Rails.send :remove_const, :Info
     silence_warnings { load 'rails/info.rb' }
@@ -72,6 +75,18 @@ EOS
     end
   end
 
+  def test_middleware_property
+    assert property_defined?('Middleware')
+  end
+
+  def test_html_includes_middleware
+    html = Rails::Info.to_html
+    assert html.include?('<tr><td class="name">Middleware</td>')
+    properties.value_for('Middleware').each do |value|
+      assert html.include?("<li>#{CGI.escapeHTML(value)}</li>")
+    end
+  end
+
   protected
     def svn_info=(info)
       Rails::Info.module_eval do