Merge pull request #16662 from robin850/duration-instance-of

Fix `ActiveSupport::Duration#instance_of?` to behave like `#is_a?`

3 files changed, 14 insertions, 9 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 88d6e09d4b..e0d154271f 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,5 +1,6 @@
-*   Add the `Duration#instance_of?` method that was previously delegated to the
-    internal `value` attribute.
+*   Fix the `ActiveSupport::Duration#instance_of?` method to return the right
+    value with the class itself since it was previously delegated to the
+    internal value.
 
     *Robin Dupret*
 
diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb
index 084d13a9e3..1a6c02a39b 100644
--- a/activesupport/lib/active_support/duration.rb
+++ b/activesupport/lib/active_support/duration.rb
@@ -35,12 +35,12 @@ module ActiveSupport
     end
 
     def is_a?(klass) #:nodoc:
-      instance_of?(klass) || value.is_a?(klass)
+      Duration == klass || value.is_a?(klass)
     end
     alias :kind_of? :is_a?
 
     def instance_of?(klass) # :nodoc:
-      Duration == klass
+      Duration == klass || value.instance_of?(klass)
     end
 
     # Returns +true+ if +other+ is also a Duration instance with the
diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb
index 330d995b7c..a89202f518 100644
--- a/activesupport/test/core_ext/duration_test.rb
+++ b/activesupport/test/core_ext/duration_test.rb
@@ -20,6 +20,12 @@ class DurationTest < ActiveSupport::TestCase
     assert !d.is_a?(k)
   end
 
+  def test_instance_of
+    assert 1.minute.instance_of?(Fixnum)
+    assert 2.days.instance_of?(ActiveSupport::Duration)
+    assert !3.second.instance_of?(Numeric)
+  end
+
   def test_threequals
     assert ActiveSupport::Duration === 1.day
     assert !(ActiveSupport::Duration === 1.day.to_i)
@@ -35,17 +41,15 @@ class DurationTest < ActiveSupport::TestCase
   end
 
   def test_eql
+    rubinius_skip "Rubinius' #eql? definition relies on #instance_of? " \
+                  "which behaves oddly for the sake of backward-compatibility."
+
     assert 1.minute.eql?(1.minute)
     assert 2.days.eql?(48.hours)
     assert !1.second.eql?(1)
     assert !1.eql?(1.second)
   end
 
-  def test_instance_of
-    assert !1.minute.instance_of?(Fixnum)
-    assert !2.days.instance_of?(Fixnum)
-  end
-
   def test_inspect
     assert_equal '0 seconds',                       0.seconds.inspect
     assert_equal '1 month',                         1.month.inspect