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
Added time helper method freeze_time
which is an alias for travel_to Time.now
#29681
Conversation
# Time.now # => 2017-07-05 11:01:38 +0530 | ||
# Time.now # => 2017-07-05 11:01:38 +0530 | ||
def freeze_time | ||
travel_to Time.now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be Time.current
though, just so we can take Time.zone
into account?
I wonder if it matters ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went by the existing definition of travel
method which uses Time.now
.
# freeze_time | ||
# Time.now # => 2017-07-05 11:01:38 +0530 | ||
# Time.now # => 2017-07-05 11:01:38 +0530 | ||
def freeze_time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice if this also worked with a block, like travel does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dhh Added support for block as well.
@@ -162,4 +162,13 @@ def test_time_helper_travel_with_time_subclass | |||
assert_equal DateTime.now.to_s, DateTimeSubclass.now.to_s | |||
end | |||
end | |||
|
|||
def test_time_helper_freeze_time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a deterministic test. This can still pass without the correct implementation, you can get same formatted time 3 times.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, updated the test to be non-deterministic. Please check.
6821dcc
to
f154c45
Compare
f154c45
to
248749f
Compare
Excellent, thanks! |
r? @dhh