From 8c1889c926326ca97cd9b4b1fb0017d13f208b03 Mon Sep 17 00:00:00 2001
From: claudiob <claudiob@gmail.com>
Date: Sun, 14 Dec 2014 17:55:30 -0800
Subject: [PATCH] Add test for `:skip_after_callbacks_if_terminated`

`define_callbacks` from `ActiveSupport::Callbacks` accepts the
`:skip_after_callbacks_if_terminated` option since #4866 but the option
is not tested anywhere.

This commit adds tests and fixes documentation for the option, making it clear
that halting a callback chain only stops following `before_` and `around_`
callbacks by default.
---
 activesupport/lib/active_support/callbacks.rb |  9 ++--
 activesupport/test/callbacks_test.rb          | 48 +++++++++++++------
 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index 24c702b602..95dbc9a0cb 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -659,16 +659,17 @@ module ActiveSupport
       # ===== Options
       #
       # * <tt>:terminator</tt> - Determines when a before filter will halt the
-      #   callback chain, preventing following callbacks from being called and
-      #   the event from being triggered. This should be a lambda to be executed.
+      #   callback chain, preventing following before and around callbacks from
+      #   being called and the event from being triggered.
+      #   This should be a lambda to be executed.
       #   The current object and the return result of the callback will be called
       #   with the lambda.
       #
       #     define_callbacks :validate, terminator: ->(target, result) { result == false }
       #
       #   In this example, if any before validate callbacks returns +false+,
-      #   other callbacks are not executed. Defaults to +false+, meaning no value
-      #   halts the chain.
+      #   any successive before and around callback is not executed.
+      #   Defaults to +false+, meaning no value halts the chain.
       #
       # * <tt>:skip_after_callbacks_if_terminated</tt> - Determines if after
       #   callbacks should be terminated by the <tt>:terminator</tt> option. By
diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb
index 32c2dfdfc0..d19e5fd6e7 100644
--- a/activesupport/test/callbacks_test.rb
+++ b/activesupport/test/callbacks_test.rb
@@ -49,7 +49,7 @@ module CallbacksTest
     def self.before(model)
       model.history << [:before_save, :class]
     end
-    
+
     def self.after(model)
       model.history << [:after_save, :class]
     end
@@ -501,21 +501,20 @@ module CallbacksTest
     end
   end
 
-  class CallbackTerminator
+  class AbstractCallbackTerminator
     include ActiveSupport::Callbacks
 
-    define_callbacks :save, :terminator => ->(_,result) { result == :halt }
-
-    set_callback :save, :before, :first
-    set_callback :save, :before, :second
-    set_callback :save, :around, :around_it
-    set_callback :save, :before, :third
-    set_callback :save, :after, :first
-    set_callback :save, :around, :around_it
-    set_callback :save, :after, :second
-    set_callback :save, :around, :around_it
-    set_callback :save, :after, :third
-
+    def self.set_save_callbacks
+      set_callback :save, :before, :first
+      set_callback :save, :before, :second
+      set_callback :save, :around, :around_it
+      set_callback :save, :before, :third
+      set_callback :save, :after, :first
+      set_callback :save, :around, :around_it
+      set_callback :save, :after, :second
+      set_callback :save, :around, :around_it
+      set_callback :save, :after, :third
+    end
 
     attr_reader :history, :saved, :halted
     def initialize
@@ -552,6 +551,17 @@ module CallbacksTest
     end
   end
 
+  class CallbackTerminator < AbstractCallbackTerminator
+    define_callbacks :save, terminator: ->(_,result) { result == :halt }
+    set_save_callbacks
+  end
+
+  class CallbackTerminatorSkippingAfterCallbacks < AbstractCallbackTerminator
+    define_callbacks :save, terminator: ->(_,result) { result == :halt },
+                            skip_after_callbacks_if_terminated: true
+    set_save_callbacks
+  end
+
   class CallbackObject
     def before(caller)
       caller.record << "before"
@@ -688,7 +698,7 @@ module CallbacksTest
   end
 
   class CallbackTerminatorTest < ActiveSupport::TestCase
-    def test_termination
+    def test_termination_skips_following_before_and_around_callbacks
       terminator = CallbackTerminator.new
       terminator.save
       assert_equal ["first", "second", "third", "second", "first"], terminator.history
@@ -707,6 +717,14 @@ module CallbacksTest
     end
   end
 
+  class CallbackTerminatorSkippingAfterCallbacksTest < ActiveSupport::TestCase
+    def test_termination_skips_after_callbacks
+      terminator = CallbackTerminatorSkippingAfterCallbacks.new
+      terminator.save
+      assert_equal ["first", "second"], terminator.history
+    end
+  end
+
   class HyphenatedKeyTest < ActiveSupport::TestCase
     def test_save
       obj = HyphenatedCallbacks.new