Skip to content
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 `travel_back` to remove stubs from `travel` and `travel_to` and remove auto-rollback after each test case #13884

Merged
merged 3 commits into from Jan 30, 2014

Conversation

@rafaelfranca
Copy link
Member

@rafaelfranca rafaelfranca commented Jan 30, 2014

This is a alternative implementation of #12966.

The reason is the same plus, I'm removing the auto-rollback after each test case. The reason is this auto-rollback will only work with minitest out of the box and it may confuse the users.

Closes #12966

@sikachu
sikachu reviewed Jan 30, 2014
View changes
activesupport/lib/active_support/testing/time_helpers.rb Outdated
@@ -68,8 +62,7 @@ def travel(duration, &block)
end

# Change current time to the given time by stubbing +Time.now+ and +Date.today+ to return the
# time or date passed into this method. Note that the stubs are automatically removed
# at the end of each test.
# time or date passed into this method.

This comment has been minimized.

@sikachu

sikachu Jan 30, 2014
Member

You'll want to keep the doc on this method intact, as we really travelling back on the block form.

This comment has been minimized.

@rafaelfranca

rafaelfranca Jan 30, 2014
Author Member

This is documented below:

      # This method also accepts a block, which will return the current time back to its original
      # state at the end of the block:

What I removed is the automatically removal of the stubs in the non-block version.

This comment has been minimized.

@sikachu

sikachu Jan 30, 2014
Member

Oh, ha. I read it wrong. This is good to go 👍

@sikachu
sikachu reviewed Jan 30, 2014
View changes
activesupport/lib/active_support/testing/time_helpers.rb Outdated
#
# Time.current # => Sat, 09 Nov 2013 15:34:49 EST -05:00
# travel_to Time.new(2004, 11, 24, 01, 04, 44)
# User.create.created_at # => Wed, 24 Nov 2004 01:04:44 EST -05:00

This comment has been minimized.

@sikachu

sikachu Jan 30, 2014
Member

I think this example could be just a simple Time.current call.

This comment has been minimized.

@rafaelfranca

rafaelfranca Jan 30, 2014
Author Member

👍, I just used the same example of the other methods. I'll change all

@fxn
fxn reviewed Jan 30, 2014
View changes
guides/source/4_1_release_notes.md Outdated
@@ -607,6 +607,9 @@ for detailed changes.
`Time.now` and
`Date.today`. ([Pull Request](https://github.com/rails/rails/pull/12824))

* Added `ActiveSupport::Testing::TimeHelpers#travel_back`. This method return

This comment has been minimized.

@fxn

fxn Jan 30, 2014
Member

"returns" here

This comment has been minimized.

@rafaelfranca

rafaelfranca Jan 30, 2014
Author Member

👍

@carlosantoniodasilva
carlosantoniodasilva reviewed Jan 30, 2014
View changes
activesupport/lib/active_support/testing/time_helpers.rb Outdated
end
end

# Return the current time back to its original state.

This comment has been minimized.

@carlosantoniodasilva

carlosantoniodasilva Jan 30, 2014
Member

Might be interesting to comment that it removes the stubs:

Return the current time back to its original state, by removing the stubs added by `travel` and `travel_to`.

This comment has been minimized.

@fxn

fxn Jan 30, 2014
Member

Detail: the doc guidelines say start in 3rd person "Returns the current time...".

This comment has been minimized.

@rafaelfranca

rafaelfranca Jan 30, 2014
Author Member

Fixed. Thanks

@senny
Copy link
Member

@senny senny commented Jan 30, 2014

@rafaelfranca looking good 👍

This behavior is only work out-of-box with minitest and also add a
downside to run after each test case, even if we don't used the travel
or travel_to methods
rafaelfranca added a commit that referenced this pull request Jan 30, 2014
Add `travel_back` to remove stubs from `travel` and `travel_to` and remove auto-rollback after each test case
@rafaelfranca rafaelfranca merged commit db6f69f into rails:master Jan 30, 2014
@rafaelfranca rafaelfranca deleted the rafaelfranca:rm-travel-back branch Jan 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.