end_travel to remove stubs from travel and travel_to

[senny]

This is a follow up to #12824.

This patch introduces a new method called end_travel to remove the stubs from Time.now and Date.today. This is useful to set your tim in setup hooks and return to the original state in teardown.

To illustrate I updated a test within AR that used some custom stubbing logic.

I'm opening the PR to discuss and find the best name for the method end_travel. While brainstorming we thought about untravel, return, go_back, back and settled for end_travel.

[senny]

@fxn as discussed.

/cc @sikachu @rafaelfranca @dhh

[senny]

Reading this comment I realize that the stubs are cleaned after every run anyway and my example with setup / teardown is not really a valid use case for end_travel.

Should we still provide this functionality to return back? If we decide not to include the method we should at least document that the stubs are cleared after every test.

[fxn]

I'd prefer to have a use-case that justifies it (as this one seemed to).

[dmathieu]

I don't see any use of end_travel which can't just be replaced with a block, which I would believe to be a better way of doing things.

[fxn]

By the way, this tells us it should be explained in the docs. It works that way because of the implementation, but that behaviour belongs to the public interface.

[senny]

Agreed. I'm replacing the end_travel commit with documentation.

[carlosantoniodasilva]

If we are allowing users to "travel"/stub at any time in a given test, I think we should allow them to stop that at any time too, either using a block or being explicit. Just my 2 cents.

[sikachu]

Since this has been a while, and seems like the use case can be justified by using a block version, I'm closing this PR for now.

[chancancode]

In the original discussion, one of the arguments against adding it is that stubs will be automatically cleaned after each test anyway. However, that seems to be a mocha feature. Now that we are removing the mocha dependency for this feature, does that still work?

[rafaelfranca]

Both my implementations have one hook to make this feature work but I'd love to remove it. The reason is, right now the hook only work with minitest. If the project is using cucumber or rspec users will have to know and use an internal feature of Rails to avoid stubs to leak between tests cases.

[chancancode]

Right, so if we don't want to make people mess with private APIs to make alternative test frameworks work, we would have to expose one of internal_unstub or end_travel, correct? In that case I think we should expose the latter.

[carlosantoniodasilva]

Maybe we should just move that to the way we expect it to be used: either use a block, which unstub at the end for you, or make sure you clean the stub yourself on an after hook. Nothing automatic, and docs would explain that. Other Ruby APIs work similarly right?

[rafaelfranca]

Right. But I don't think we should expose internal_unstub. I prefer to have an end_travel to let users make sure they cleaned the stub themself.

[chancancode]

👍—
Sent from Mailbox for iPhone

On Fri, Jan 24, 2014 at 8:50 AM, Rafael Mendonça França
notifications@github.com wrote:

Right. But I don't think we should expose internal_unstub. I prefer to have an end_travel to let users make sure they cleaned the stub theyself.

Reply to this email directly or view it on GitHub:
#12966 (comment)

[carlosantoniodasilva]

Yup, end_travel it is. 👍

[senny]

Closing in favor of #13884