Skip to content

Commit

Permalink
travel/travel_to travel time helpers, now raise on nested calls,
Browse files Browse the repository at this point in the history
     as this can lead to confusing time stubbing.

     Instead of:

         travel_to 2.days.from_now do
           # 2 days from today
           travel_to 3.days.from_now do
             # 5 days from today
           end
         end

     preferred way to achieve above is:

         travel_to 2.days.from_now
         # 2 days from today

         travel_back
         travel_to 5.days.from_now
         # 5 days from today

Closes rails#24690
Fixes rails#24689
  • Loading branch information
vipulnsward committed Jul 2, 2016
1 parent 173bf35 commit 919e705
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 14 deletions.
11 changes: 5 additions & 6 deletions activerecord/test/cases/mixin_test.rb
Expand Up @@ -41,13 +41,12 @@ def test_many_updates

old_updated_at = stamped.updated_at

travel 5.minutes do
stamped.lft_will_change!
stamped.save
travel 5.minutes
stamped.lft_will_change!
stamped.save

assert_equal Time.now, stamped.updated_at
assert_equal old_updated_at, stamped.created_at
end
assert_equal Time.now, stamped.updated_at
assert_equal old_updated_at, stamped.created_at
end

def test_create_turned_off
Expand Down
25 changes: 25 additions & 0 deletions activesupport/CHANGELOG.md
@@ -1,3 +1,28 @@
* `travel/travel_to` travel time helpers, now raise on nested calls,
as this can lead to confusing time stubbing.

Instead of:

travel_to 2.days.from_now do
# 2 days from today
travel_to 3.days.from_now do
# 5 days from today
end
end

preferred way to achieve above is:

travel 2.days do
# 2 days from today
end
travel 5.days do
# 5 days from today
end
*Vipul A M*


* Support parsing JSON time in ISO8601 local time strings in
`ActiveSupport::JSON.decode` when `parse_json_times` is enabled.
Strings in the format of `YYYY-MM-DD hh:mm:ss` (without a `Z` at
Expand Down
51 changes: 43 additions & 8 deletions activesupport/lib/active_support/testing/time_helpers.rb
@@ -1,32 +1,39 @@
require 'active_support/core_ext/string/strip' # for strip_heredoc
require 'concurrent/map'

module ActiveSupport
module Testing
class SimpleStubs # :nodoc:
Stub = Struct.new(:object, :method_name, :original_method)

def initialize
@stubs = {}
@stubs = Concurrent::Map.new { |h, k| h[k] = {} }
end

def stub_object(object, method_name, return_value)
key = [object.object_id, method_name]

if stub = @stubs[key]
if stub = stubbing(object, method_name)
unstub_object(stub)
end

new_name = "__simple_stub__#{method_name}"

@stubs[key] = Stub.new(object, method_name, new_name)
@stubs[object.object_id][method_name] = Stub.new(object, method_name, new_name)

object.singleton_class.send :alias_method, new_name, method_name
object.define_singleton_method(method_name) { return_value }
end

def unstub_all!
@stubs.each_value do |stub|
unstub_object(stub)
@stubs.each_value do |object_stubs|
object_stubs.each_value do |stub|
unstub_object(stub)
end
end
@stubs = {}
@stubs.clear
end

def stubbing(object, method_name)
@stubs[object.object_id][method_name]
end

private
Expand Down Expand Up @@ -93,6 +100,34 @@ def travel(duration, &block)
# end
# Time.current # => Sat, 09 Nov 2013 15:34:49 EST -05:00
def travel_to(date_or_time)
if block_given? && simple_stubs.stubbing(Time, :now)
travel_to_nested_block_call = <<-MSG.strip_heredoc
Calling `travel_to` with a block, when we have previously already made a call to `travel_to`, can lead to confusing time stubbing.
Instead of:
travel_to 2.days.from_now do
# 2 days from today
travel_to 3.days.from_now do
# 5 days from today
end
end
preferred way to achieve above is:
travel 2.days do
# 2 days from today
end
travel 5.days do
# 5 days from today
end
MSG
raise travel_to_nested_block_call
end

if date_or_time.is_a?(Date) && !date_or_time.is_a?(DateTime)
now = date_or_time.midnight.to_time
else
Expand Down
43 changes: 43 additions & 0 deletions activesupport/test/time_travel_test.rb
Expand Up @@ -77,6 +77,49 @@ def test_time_helper_travel_back
end
end

def test_time_helper_travel_to_with_nested_calls_with_blocks
Time.stub(:now, Time.now) do
outer_expected_time = Time.new(2004, 11, 24, 01, 04, 44)
inner_expected_time = Time.new(2004, 10, 24, 01, 04, 44)
travel_to outer_expected_time do
assert_raises(RuntimeError, /Calling `travel_to` with a block, when we have previously already made a call to `travel_to`, can lead to confusing time stubbing./) do
travel_to(inner_expected_time) do
#noop
end
end
end
end
end

def test_time_helper_travel_to_with_nested_calls
Time.stub(:now, Time.now) do
outer_expected_time = Time.new(2004, 11, 24, 01, 04, 44)
inner_expected_time = Time.new(2004, 10, 24, 01, 04, 44)
travel_to outer_expected_time do
assert_nothing_raised do
travel_to(inner_expected_time)

assert_equal inner_expected_time, Time.now
end
end
end
end

def test_time_helper_travel_to_with_subsequent_calls
Time.stub(:now, Time.now) do
initial_expected_time = Time.new(2004, 11, 24, 01, 04, 44)
subsequent_expected_time = Time.new(2004, 10, 24, 01, 04, 44)
assert_nothing_raised do
travel_to initial_expected_time
travel_to subsequent_expected_time

assert_equal subsequent_expected_time, Time.now

travel_back
end
end
end

def test_travel_to_will_reset_the_usec_to_avoid_mysql_rouding
Time.stub(:now, Time.now) do
travel_to Time.utc(2014, 10, 10, 10, 10, 50, 999999) do
Expand Down

0 comments on commit 919e705

Please sign in to comment.