Support nested time travel helpers #24690
Conversation
|
r? @senny |
vipulnsward
force-pushed
the
24689-allow-nested-travel-to
branch
3 times, most recently
from
3d4c85b
to
26178fb
Compare
|
Sorry @senny, I don't remember why I assigned this to you. Going to assign to RF, since he originally wrote it. |
| stub = stubs.first | ||
| unstub_object(stub) if stub | ||
| end | ||
| @stubs = Hash.new { |h,k| h[k] = [] } |
There was a problem hiding this comment.
Could mutate the hash inline: @stubs.clear.
…if we have called them in a row, or in a nested fashion.
We simply unstub all previous occurrences. Consider following scenario:
travel_to outer_expected_time do
travel_to inner_expected_time do
#perform some operations from some state here
end
#perform some operations from some outer state here
end
This will fail for outer scenario in that, we simply just unstub all stubbed objects.
Here onwards we also take into account the object into account to create alias, to be more safe in returning back to previous states.
Fixes rails#24689
vipulnsward
force-pushed
the
24689-allow-nested-travel-to
branch
from
26178fb
to
204c041
Compare
| @stubs.each_value do |stubs| | ||
| stub = stubs.first | ||
| unstub_object(stub) if stub | ||
| end |
There was a problem hiding this comment.
Could be a good case for splat:
@stubs.each_value do |(stub_masking_original_methods, *)|
|
Are we sure we want Not sure what's intuitive here. |
| stub_object(stub.object, stub.method_name, stub.return_value) | ||
| else | ||
| unstub_object(current_stub) | ||
| end |
There was a problem hiding this comment.
Could do with a better method name 😁
Since the stubs per method is essentially turned into a stack we could go with pop_stub.
Here's my take:
def pop_stub(object, method_name)
key = [ object.object_id, method_name ]
currently_stubbed_to = @stubs[key].pop
if stub_next_in_stack = @stubs[key].last
stub_object(object, method_name, stub_next_in_stack.return_value)
else
unstub_object(currently_stubbed_to)
end
endRenamed some variables because I found them hard to keep track of.
If it's called inside of a |
|
That sounds reasonable, yeah 👍 |
|
Closing in favor of #24890 |
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
Currently travel time helpers don't restore to proper previous state if we have called them in a row, or in a nested fashion.
We simply unstub all previous occurrences. Consider following scenario:
This will fail for outer scenario in that, we simply just unstub all stubbed objects.
Here onwards we also take into account the object into account to create alias, to be more safe in returning back to previous states.
Fixes #24689