1 files changed, 39 insertions, 23 deletions

diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb
index b2b01b3fa9..b9fb6be4e3 100644
--- a/actionpack/test/controller/filters_test.rb
+++ b/actionpack/test/controller/filters_test.rb
@@ -10,7 +10,7 @@ class ActionController::Base
 
     def before_actions
       filters = _process_action_callbacks.select { |c| c.kind == :before }
-      filters.map! { |c| c.raw_filter }
+      filters.map!(&:raw_filter)
     end
   end
 
@@ -225,6 +225,30 @@ class FilterTest < ActionController::TestCase
     skip_before_action :clean_up_tmp, if: -> { true }
   end
 
+  class SkipFilterUsingOnlyAndIf < ConditionalFilterController
+    before_action :clean_up_tmp
+    before_action :ensure_login
+
+    skip_before_action :ensure_login, only: :login, if: -> { false }
+    skip_before_action :clean_up_tmp, only: :login, if: -> { true }
+
+    def login
+      render text: 'ok'
+    end
+  end
+
+  class SkipFilterUsingIfAndExcept < ConditionalFilterController
+    before_action :clean_up_tmp
+    before_action :ensure_login
+
+    skip_before_action :ensure_login, if: -> { false }, except: :login
+    skip_before_action :clean_up_tmp, if: -> { true }, except: :login
+
+    def login
+      render text: 'ok'
+    end
+  end
+
   class ClassController < ConditionalFilterController
     before_action ConditionalClassFilter
   end
@@ -504,7 +528,6 @@ class FilterTest < ActionController::TestCase
 
       def non_yielding_action
         @filters  << "it didn't yield"
-        @filter_return_value
       end
 
       def action_three
@@ -528,32 +551,15 @@ class FilterTest < ActionController::TestCase
     end
   end
 
-  def test_non_yielding_around_actions_not_returning_false_do_not_raise
+  def test_non_yielding_around_actions_do_not_raise
     controller = NonYieldingAroundFilterController.new
-    controller.instance_variable_set "@filter_return_value", true
     assert_nothing_raised do
       test_process(controller, "index")
     end
   end
 
-  def test_non_yielding_around_actions_returning_false_do_not_raise
-    controller = NonYieldingAroundFilterController.new
-    controller.instance_variable_set "@filter_return_value", false
-    assert_nothing_raised do
-      test_process(controller, "index")
-    end
-  end
-
-  def test_after_actions_are_not_run_if_around_action_returns_false
-    controller = NonYieldingAroundFilterController.new
-    controller.instance_variable_set "@filter_return_value", false
-    test_process(controller, "index")
-    assert_equal ["filter_one", "it didn't yield"], controller.assigns['filters']
-  end
-
   def test_after_actions_are_not_run_if_around_action_does_not_yield
     controller = NonYieldingAroundFilterController.new
-    controller.instance_variable_set "@filter_return_value", true
     test_process(controller, "index")
     assert_equal ["filter_one", "it didn't yield"], controller.assigns['filters']
   end
@@ -614,6 +620,16 @@ class FilterTest < ActionController::TestCase
     assert_equal %w( ensure_login ), assigns["ran_filter"]
   end
 
+  def test_if_is_ignored_when_used_with_only
+    test_process(SkipFilterUsingOnlyAndIf, 'login')
+    assert_nil assigns['ran_filter']
+  end
+
+  def test_except_is_ignored_when_used_with_if
+    test_process(SkipFilterUsingIfAndExcept, 'login')
+    assert_equal %w(ensure_login), assigns["ran_filter"]
+  end
+
   def test_skipping_class_actions
     test_process(ClassController)
     assert_equal true, assigns["ran_class_action"]
@@ -1021,21 +1037,21 @@ class YieldingAroundFiltersTest < ActionController::TestCase
   def test_first_action_in_multiple_before_action_chain_halts
     controller = ::FilterTest::TestMultipleFiltersController.new
     response = test_process(controller, 'fail_1')
-    assert_equal ' ', response.body
+    assert_equal '', response.body
     assert_equal 1, controller.instance_variable_get(:@try)
   end
 
   def test_second_action_in_multiple_before_action_chain_halts
     controller = ::FilterTest::TestMultipleFiltersController.new
     response = test_process(controller, 'fail_2')
-    assert_equal ' ', response.body
+    assert_equal '', response.body
     assert_equal 2, controller.instance_variable_get(:@try)
   end
 
   def test_last_action_in_multiple_before_action_chain_halts
     controller = ::FilterTest::TestMultipleFiltersController.new
     response = test_process(controller, 'fail_3')
-    assert_equal ' ', response.body
+    assert_equal '', response.body
     assert_equal 3, controller.instance_variable_get(:@try)
   end