New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add `before?` and `after?` methods to date and time classes #32185

Merged
merged 1 commit into from Mar 26, 2018
Jump to file or symbol
Failed to load files and symbols.
+63 −0
Diff settings

Always

Just for now

Copy path View file
@@ -1,5 +1,10 @@
## Rails 6.0.0.alpha (Unreleased) ##
* Add `before?` and `after?` methods to `Date`, `DateTime`,
`Time`, and `TimeWithZone`.
*Nick Holden*
* Add `:private` option to ActiveSupport's `Module#delegate`
in order to delegate methods as private:
@@ -142,4 +142,6 @@ def compare_with_coercion(other)
end
alias_method :compare_without_coercion, :<=>
alias_method :<=>, :compare_with_coercion
alias_method :before?, :<
alias_method :after?, :>
end
@@ -208,4 +208,6 @@ def <=>(other)
super
end
end
alias_method :before?, :<
alias_method :after?, :>
end
@@ -302,6 +302,8 @@ def compare_with_coercion(other)
end
alias_method :compare_without_coercion, :<=>
alias_method :<=>, :compare_with_coercion
alias_method :before?, :<
alias_method :after?, :>
# Layers additional behavior on Time#eql? so that ActiveSupport::TimeWithZone instances
# can be eql? to an equivalent Time
@@ -225,6 +225,8 @@ def strftime(format)
def <=>(other)
utc <=> other
end
alias_method :before?, :<
alias_method :after?, :>
# Returns true if the current object's time is within the specified
# +min+ and +max+ time.
@@ -360,6 +360,18 @@ def test_future
end
end
def test_before
assert_equal false, Date.new(2017, 3, 6).before?(Date.new(2017, 3, 5))
assert_equal false, Date.new(2017, 3, 6).before?(Date.new(2017, 3, 6))
assert_equal true, Date.new(2017, 3, 6).before?(Date.new(2017, 3, 7))
end
def test_after
assert_equal true, Date.new(2017, 3, 6).after?(Date.new(2017, 3, 5))
assert_equal false, Date.new(2017, 3, 6).after?(Date.new(2017, 3, 6))
assert_equal false, Date.new(2017, 3, 6).after?(Date.new(2017, 3, 7))
end
def test_current_returns_date_today_when_zone_not_set
with_env_tz "US/Central" do
Time.stub(:now, Time.local(1999, 12, 31, 23)) do
@@ -285,6 +285,18 @@ def test_future_without_offset
end
end
def test_before
assert_equal false, DateTime.civil(2017, 3, 6, 12, 0, 0).before?(DateTime.civil(2017, 3, 6, 11, 59, 59))
assert_equal false, DateTime.civil(2017, 3, 6, 12, 0, 0).before?(DateTime.civil(2017, 3, 6, 12, 0, 0))
assert_equal true, DateTime.civil(2017, 3, 6, 12, 0, 0).before?(DateTime.civil(2017, 3, 6, 12, 00, 1))
end
def test_after
assert_equal true, DateTime.civil(2017, 3, 6, 12, 0, 0).after?(DateTime.civil(2017, 3, 6, 11, 59, 59))
assert_equal false, DateTime.civil(2017, 3, 6, 12, 0, 0).after?(DateTime.civil(2017, 3, 6, 12, 0, 0))
assert_equal false, DateTime.civil(2017, 3, 6, 12, 0, 0).after?(DateTime.civil(2017, 3, 6, 12, 00, 1))
end
def test_current_returns_date_today_when_zone_is_not_set
with_env_tz "US/Eastern" do
Time.stub(:now, Time.local(1999, 12, 31, 23, 59, 59)) do
@@ -736,6 +736,18 @@ def test_future_with_time_current_as_time_with_zone
end
end
def test_before
assert_equal false, Time.utc(2017, 3, 6, 12, 0, 0).before?(Time.utc(2017, 3, 6, 11, 59, 59))
assert_equal false, Time.utc(2017, 3, 6, 12, 0, 0).before?(Time.utc(2017, 3, 6, 12, 0, 0))
assert_equal true, Time.utc(2017, 3, 6, 12, 0, 0).before?(Time.utc(2017, 3, 6, 12, 00, 1))
end

This comment has been minimized.

@kerrizor

kerrizor Mar 26, 2018

Contributor

Instead of testing behavior (since what you're really testing is the underlying implementation of :> and :<

time = Time.now # or whatever instance flavor you're testing
time.method(:before) == time.method(:<)
time.method(:after) == time.method(:>)

This comment has been minimized.

@nholden

nholden Mar 26, 2018

Contributor

Interesting, I hadn't considered tests like that. Even though before? and after? are simply alias methods, I think it's valuable to have tests that look at actual dates and times since we'll always expect them to pass regardless of any changes to < or >. What do you think?

This comment has been minimized.

@rafaelfranca

rafaelfranca Mar 26, 2018

Member

We prefer to test behavior in Rails, not the implementation. If the implementation changes we don't have to change the test. So this test is actually nice.

def test_after
assert_equal true, Time.utc(2017, 3, 6, 12, 0, 0).after?(Time.utc(2017, 3, 6, 11, 59, 59))
assert_equal false, Time.utc(2017, 3, 6, 12, 0, 0).after?(Time.utc(2017, 3, 6, 12, 0, 0))
assert_equal false, Time.utc(2017, 3, 6, 12, 0, 0).after?(Time.utc(2017, 3, 6, 12, 00, 1))
end
def test_acts_like_time
assert_predicate Time.new, :acts_like_time?
end
@@ -289,6 +289,20 @@ def test_future_with_time_current_as_time_with_zone
end
end
def test_before
twz = ActiveSupport::TimeWithZone.new(Time.utc(2017, 3, 6, 12, 0, 0), @time_zone)
assert_equal false, twz.before?(ActiveSupport::TimeWithZone.new(Time.utc(2017, 3, 6, 11, 59, 59), @time_zone))
assert_equal false, twz.before?(ActiveSupport::TimeWithZone.new(Time.utc(2017, 3, 6, 12, 0, 0), @time_zone))
assert_equal true, twz.before?(ActiveSupport::TimeWithZone.new(Time.utc(2017, 3, 6, 12, 00, 1), @time_zone))
end
def test_after
twz = ActiveSupport::TimeWithZone.new(Time.utc(2017, 3, 6, 12, 0, 0), @time_zone)
assert_equal true, twz.after?(ActiveSupport::TimeWithZone.new(Time.utc(2017, 3, 6, 11, 59, 59), @time_zone))
assert_equal false, twz.after?(ActiveSupport::TimeWithZone.new(Time.utc(2017, 3, 6, 12, 0, 0), @time_zone))
assert_equal false, twz.after?(ActiveSupport::TimeWithZone.new(Time.utc(2017, 3, 6, 12, 00, 1), @time_zone))
end
def test_eql?
assert_equal true, @twz.eql?(@twz.dup)
assert_equal true, @twz.eql?(Time.utc(2000))
ProTip! Use n and p to navigate between commits in a pull request.