From a91ea1d51048342d13fc73f9b09ce4cfd086bb34 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Wed, 15 Mar 2017 14:38:21 +0000 Subject: Remove implicit coercion deprecation of durations In #28204 we deprecated implicit conversion of durations to a numeric which represented the number of seconds in the duration because of unwanted side effects with calculations on durations and dates. This unfortunately had the side effect of forcing a explicit cast when configuring third-party libraries like expiration in Redis, e.g: redis.expire("foo", 5.minutes) To work around this we've removed the deprecation and added a private class that wraps the numeric and can perform calculation involving durations and ensure that they remain a duration irrespective of the order of operations. --- activesupport/test/core_ext/duration_test.rb | 142 ++++++++++++++++++++++----- 1 file changed, 117 insertions(+), 25 deletions(-) (limited to 'activesupport/test/core_ext/duration_test.rb') diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 2b1a715b7a..52fdbe677d 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -96,24 +96,20 @@ class DurationTest < ActiveSupport::TestCase assert_instance_of ActiveSupport::Duration, 2.seconds - 1.second assert_equal 1.second, 2.seconds - 1 assert_instance_of ActiveSupport::Duration, 2.seconds - 1 + assert_equal 1.second, 2 - 1.second + assert_instance_of ActiveSupport::Duration, 2.seconds - 1 end def test_multiply assert_equal 7.days, 1.day * 7 assert_instance_of ActiveSupport::Duration, 1.day * 7 - - assert_deprecated do - assert_equal 86400, 1.day * 1.second - end + assert_equal 86400, 1.day * 1.second end def test_divide assert_equal 1.day, 7.days / 7 assert_instance_of ActiveSupport::Duration, 7.days / 7 - - assert_deprecated do - assert_equal 1, 1.day / 1.day - end + assert_equal 1, 1.day / 1.day end def test_date_added_with_multiplied_duration @@ -121,9 +117,7 @@ class DurationTest < ActiveSupport::TestCase end def test_plus_with_time - assert_deprecated do - assert_equal 1 + 1.second, 1.second + 1, "Duration + Numeric should == Numeric + Duration" - end + assert_equal 1 + 1.second, 1.second + 1, "Duration + Numeric should == Numeric + Duration" end def test_time_plus_duration_returns_same_time_datatype @@ -142,13 +136,6 @@ class DurationTest < ActiveSupport::TestCase assert_equal 'expected a time or date, got ""', e.message, "ensure ArgumentError is not being raised by dependencies.rb" end - def test_implicit_coercion_is_deprecated - assert_deprecated { 1 + 1.second } - assert_deprecated { 1 - 1.second } - assert_deprecated { 1 * 1.second } - assert_deprecated { 1 / 1.second } - end - def test_fractional_weeks assert_equal((86400 * 7) * 1.5, 1.5.weeks) assert_equal((86400 * 7) * 1.7, 1.7.weeks) @@ -286,20 +273,125 @@ class DurationTest < ActiveSupport::TestCase def test_comparable assert_equal(-1, (0.seconds <=> 1.second)) assert_equal(-1, (1.second <=> 1.minute)) - - assert_deprecated do - assert_equal(-1, (1 <=> 1.minute)) - end - + assert_equal(-1, (1 <=> 1.minute)) assert_equal(0, (0.seconds <=> 0.seconds)) assert_equal(0, (0.seconds <=> 0.minutes)) assert_equal(0, (1.second <=> 1.second)) assert_equal(1, (1.second <=> 0.second)) assert_equal(1, (1.minute <=> 1.second)) + assert_equal(1, (61 <=> 1.minute)) + end - assert_deprecated do - assert_equal(1, (61 <=> 1.minute)) + def test_implicit_coercion + assert_equal 2.days, 2 * 1.day + assert_instance_of ActiveSupport::Duration, 2 * 1.day + assert_equal Time.utc(2017, 1, 3), Time.utc(2017, 1, 1) + 2 * 1.day + assert_equal Date.civil(2017, 1, 3), Date.civil(2017, 1, 1) + 2 * 1.day + end + + def test_scalar_coerce + scalar = ActiveSupport::Duration::Scalar.new(10) + assert_instance_of ActiveSupport::Duration::Scalar, 10 + scalar + assert_instance_of ActiveSupport::Duration, 10.seconds + scalar + end + + def test_scalar_delegations + scalar = ActiveSupport::Duration::Scalar.new(10) + assert_kind_of Float, scalar.to_f + assert_kind_of Integer, scalar.to_i + assert_kind_of String, scalar.to_s + end + + def test_scalar_unary_minus + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal -10, -scalar + assert_instance_of ActiveSupport::Duration::Scalar, -scalar + end + + def test_scalar_compare + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal 1, scalar <=> 5 + assert_equal 0, scalar <=> 10 + assert_equal -1, scalar <=> 15 + assert_equal nil, scalar <=> "foo" + end + + def test_scalar_plus + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal 20, 10 + scalar + assert_instance_of ActiveSupport::Duration::Scalar, 10 + scalar + assert_equal 20, scalar + 10 + assert_instance_of ActiveSupport::Duration::Scalar, scalar + 10 + assert_equal 20, 10.seconds + scalar + assert_instance_of ActiveSupport::Duration, 10.seconds + scalar + assert_equal 20, scalar + 10.seconds + assert_instance_of ActiveSupport::Duration, scalar + 10.seconds + + exception = assert_raises(TypeError) do + scalar + "foo" + end + + assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message + end + + def test_scalar_minus + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal 10, 20 - scalar + assert_instance_of ActiveSupport::Duration::Scalar, 20 - scalar + assert_equal 5, scalar - 5 + assert_instance_of ActiveSupport::Duration::Scalar, scalar - 5 + assert_equal 10, 20.seconds - scalar + assert_instance_of ActiveSupport::Duration, 20.seconds - scalar + assert_equal 5, scalar - 5.seconds + assert_instance_of ActiveSupport::Duration, scalar - 5.seconds + + exception = assert_raises(TypeError) do + scalar - "foo" + end + + assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message + end + + def test_scalar_multiply + scalar = ActiveSupport::Duration::Scalar.new(5) + + assert_equal 10, 2 * scalar + assert_instance_of ActiveSupport::Duration::Scalar, 2 * scalar + assert_equal 10, scalar * 2 + assert_instance_of ActiveSupport::Duration::Scalar, scalar * 2 + assert_equal 10, 2.seconds * scalar + assert_instance_of ActiveSupport::Duration, 2.seconds * scalar + assert_equal 10, scalar * 2.seconds + assert_instance_of ActiveSupport::Duration, scalar * 2.seconds + + exception = assert_raises(TypeError) do + scalar * "foo" end + + assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message + end + + def test_scalar_divide + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal 10, 100 / scalar + assert_instance_of ActiveSupport::Duration::Scalar, 100 / scalar + assert_equal 5, scalar / 2 + assert_instance_of ActiveSupport::Duration::Scalar, scalar / 2 + assert_equal 10, 100.seconds / scalar + assert_instance_of ActiveSupport::Duration, 2.seconds * scalar + assert_equal 5, scalar / 2.seconds + assert_instance_of ActiveSupport::Duration, scalar / 2.seconds + + exception = assert_raises(TypeError) do + scalar / "foo" + end + + assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message end def test_twelve_months_equals_one_year -- cgit v1.2.3 From b9c399de22aa84c94bb6b2edf6f8e88b88ac5f4a Mon Sep 17 00:00:00 2001 From: Andrew White Date: Thu, 16 Mar 2017 09:16:20 +0000 Subject: Fix test warnings --- activesupport/test/core_ext/duration_test.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'activesupport/test/core_ext/duration_test.rb') diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 52fdbe677d..1648a9b270 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -305,17 +305,17 @@ class DurationTest < ActiveSupport::TestCase def test_scalar_unary_minus scalar = ActiveSupport::Duration::Scalar.new(10) - assert_equal -10, -scalar + assert_equal(-10, -scalar) assert_instance_of ActiveSupport::Duration::Scalar, -scalar end def test_scalar_compare scalar = ActiveSupport::Duration::Scalar.new(10) - assert_equal 1, scalar <=> 5 - assert_equal 0, scalar <=> 10 - assert_equal -1, scalar <=> 15 - assert_equal nil, scalar <=> "foo" + assert_equal(1, scalar <=> 5) + assert_equal(0, scalar <=> 10) + assert_equal(-1, scalar <=> 15) + assert_equal(nil, scalar <=> "foo") end def test_scalar_plus -- cgit v1.2.3 From 28938dd64c8b4fa1943d0b878d3d832b94fa12a3 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Sat, 20 May 2017 16:33:09 +0100 Subject: Fix implicit calculations with scalars and durations Previously calculations where the scalar is first would be converted to a duration of seconds but this causes issues with dates being converted to times, e.g: Time.zone = "Beijing" # => Asia/Shanghai date = Date.civil(2017, 5, 20) # => Mon, 20 May 2017 2 * 1.day # => 172800 seconds date + 2 * 1.day # => Mon, 22 May 2017 00:00:00 CST +08:00 Now the `ActiveSupport::Duration::Scalar` calculation methods will try to maintain the part structure of the duration where possible, e.g: Time.zone = "Beijing" # => Asia/Shanghai date = Date.civil(2017, 5, 20) # => Mon, 20 May 2017 2 * 1.day # => 2 days date + 2 * 1.day # => Mon, 22 May 2017 Fixes #29160, #28970. --- activesupport/test/core_ext/duration_test.rb | 34 ++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) (limited to 'activesupport/test/core_ext/duration_test.rb') diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 1648a9b270..3108f24f21 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -337,6 +337,13 @@ class DurationTest < ActiveSupport::TestCase assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message end + def test_scalar_plus_parts + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal({ days: 1, seconds: 10 }, (scalar + 1.day).parts) + assert_equal({ days: -1, seconds: 10 }, (scalar + -1.day).parts) + end + def test_scalar_minus scalar = ActiveSupport::Duration::Scalar.new(10) @@ -349,6 +356,9 @@ class DurationTest < ActiveSupport::TestCase assert_equal 5, scalar - 5.seconds assert_instance_of ActiveSupport::Duration, scalar - 5.seconds + assert_equal({ days: -1, seconds: 10 }, (scalar - 1.day).parts) + assert_equal({ days: 1, seconds: 10 }, (scalar - -1.day).parts) + exception = assert_raises(TypeError) do scalar - "foo" end @@ -356,6 +366,13 @@ class DurationTest < ActiveSupport::TestCase assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message end + def test_scalar_minus_parts + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal({ days: -1, seconds: 10 }, (scalar - 1.day).parts) + assert_equal({ days: 1, seconds: 10 }, (scalar - -1.day).parts) + end + def test_scalar_multiply scalar = ActiveSupport::Duration::Scalar.new(5) @@ -375,6 +392,14 @@ class DurationTest < ActiveSupport::TestCase assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message end + def test_scalar_multiply_parts + scalar = ActiveSupport::Duration::Scalar.new(1) + assert_equal({ days: 2 }, (scalar * 2.days).parts) + assert_equal(172800, (scalar * 2.days).value) + assert_equal({ days: -2 }, (scalar * -2.days).parts) + assert_equal(-172800, (scalar * -2.days).value) + end + def test_scalar_divide scalar = ActiveSupport::Duration::Scalar.new(10) @@ -394,6 +419,15 @@ class DurationTest < ActiveSupport::TestCase assert_equal "no implicit conversion of String into ActiveSupport::Duration::Scalar", exception.message end + def test_scalar_divide_parts + scalar = ActiveSupport::Duration::Scalar.new(10) + + assert_equal({ days: 2 }, (scalar / 5.days).parts) + assert_equal(172800, (scalar / 5.days).value) + assert_equal({ days: -2 }, (scalar / -5.days).parts) + assert_equal(-172800, (scalar / -5.days).value) + end + def test_twelve_months_equals_one_year assert_equal 12.months, 1.year end -- cgit v1.2.3 From 6631bc569f3f5cd5bef764a167ba1c9587d80fb7 Mon Sep 17 00:00:00 2001 From: utilum Date: Tue, 20 Jun 2017 06:49:49 +0200 Subject: prepare for Minitest 6 --- activesupport/test/core_ext/duration_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activesupport/test/core_ext/duration_test.rb') diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 3108f24f21..cd1b505c34 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -315,7 +315,7 @@ class DurationTest < ActiveSupport::TestCase assert_equal(1, scalar <=> 5) assert_equal(0, scalar <=> 10) assert_equal(-1, scalar <=> 15) - assert_equal(nil, scalar <=> "foo") + assert_nil(scalar <=> "foo") end def test_scalar_plus -- cgit v1.2.3 From cfade1ec7ee7b5b51f3c1578e3474f9c156f2971 Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Thu, 22 Jun 2017 22:59:18 -0400 Subject: Enforce frozen string in Rubocop --- activesupport/test/core_ext/duration_test.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'activesupport/test/core_ext/duration_test.rb') diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index cd1b505c34..32c0ce4317 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true require "abstract_unit" require "active_support/inflector" require "active_support/time" -- cgit v1.2.3 From 87b3e226d65ac1ed371620bfdcd2f950c87cfece Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Sun, 2 Jul 2017 02:15:17 +0930 Subject: Revert "Merge pull request #29540 from kirs/rubocop-frozen-string" This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa. --- activesupport/test/core_ext/duration_test.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'activesupport/test/core_ext/duration_test.rb') diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 32c0ce4317..cd1b505c34 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -1,4 +1,3 @@ -# frozen_string_literal: true require "abstract_unit" require "active_support/inflector" require "active_support/time" -- cgit v1.2.3 From 72950568dde05bfe8a69ce4bbf6338fdebf3062f Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Sun, 9 Jul 2017 15:06:36 +0300 Subject: Use frozen-string-literal in ActiveSupport --- activesupport/test/core_ext/duration_test.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'activesupport/test/core_ext/duration_test.rb') diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index cd1b505c34..32c0ce4317 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true require "abstract_unit" require "active_support/inflector" require "active_support/time" -- cgit v1.2.3 From ac717d65a31d05458588b78ea7719b79f8ea69e5 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 10 Jul 2017 22:39:13 +0900 Subject: [Active Support] `rubocop -a --only Layout/EmptyLineAfterMagicComment` --- activesupport/test/core_ext/duration_test.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'activesupport/test/core_ext/duration_test.rb') diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index 32c0ce4317..c655f466f2 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require "abstract_unit" require "active_support/inflector" require "active_support/time" -- cgit v1.2.3