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

Timecop should automaticaly handle returning time back to normal #5

Closed
richievos opened this issue Dec 8, 2009 · 8 comments
Closed

Comments

@richievos
Copy link

I just tracked down a bug in our tests where intermittent failures were occurring (really scared to see how many fail when I fix this bug). I got burned by the fact that Timecop doesn't always return time back to normal.

Example:
Timecop.freeze { Timecop.travel(1.second) }

After running that, all subsequent tests will have Time.now frozen (curiously to 2000-1-1). From reading the api again, I see there's the Timecop.return method that will fix this issue for us.

2 issues/opinions:

  1. when that block closes timecop should see there are no more time blocks around, and reset time back to normal right away
  2. after the test with that line finishes, it seems like a disaster waiting to happen for that time to not be automatically reset back to normal

For 2 I think if possible Timecop should be managing resetting that. All other mocking libraries out there would implicitly handle that, so it seems fair for a user to assume Timecop would as well.

If that's not possible, or overly complicated it seems deserving of a big, flashing, red section of the Readme that says "watch out, you better add an after(:each) hook which calls Timecop.return or your time will be frozen for-ev-er"

@richievos
Copy link
Author

A workaround for this if you're using RSpec is to add to your spec_helper:

config.before(:each) do
  Timecop.return
end

@jtrupiano
Copy link
Collaborator

@JerryVos I think this is user error. Your code snippet should be:

Timecop.freeze(a_time) { Timecop.travel(1.second) { # code here } }

The version of the call where a block is not passed is a feature that I use in several places and won't seriously consider removing.

For #1, I see what you're saying and would consider a patch to correct it. Timecop essentially uses a stack (implemented by native arrays) of TimeStackItem's. When the block returns, it's just a matter of popping every TimeStackItem off the stack that is above itself.

For #2, I had always considered this to be impossible....am I supposed to detect that Timecop is being run from within Test::Unit?? How should I know when/if to implicitly return when I'm using Timecop e.g. from irb? Could you point me to actual code samples from other mocking libraries where they handle this particular problem?

@richievos
Copy link
Author

I agree having to pass a block to all calls would be annoying, I don't want to pass a block to travel when I am in a freeze.

For #2, happy to provide examples. My main thought is I have never typed Mocha.return or RSpec.return in a test or teardown/after block, and I would not expect to need to do that with another mocking library.

Towards the implementation, I would assume there probably would have to be some detection of some sort to handle all apis. Realistically though, if Test::Unit was supported along with RSpec a majority of users would be covered (RSpec support might even be free if Test::Unit is done [at least in rails apps]). The libraries I am familiar with take this approach:

  • mocha - if you look at mocha/lib/mocha/integration, mocha monkey patches itself into Test::Unit::TestCase and MiniTest::Unit::TestCase
  • rspec - the library itself has a lib/interop folder where it's Test::Unit::TestCase interoperability lives
  • shoulda - lib/shoulda.rb has the triggering of the RSpec/TestUnit interoperability
    if defined? Spec
    require 'shoulda/rspec'
    else
    require 'shoulda/test_unit'
    end
  • spork - when you run spork --bootstrap it detects if you are using RSpec, and updates the proper files

Let me know if you'd like more or can't find those files

@jtrupiano
Copy link
Collaborator

Hey Richie,

Haven't forgotten about this. Thanks for supplying direct references to the other mocking libraries for this functionality. I'm hoping to have some time over the holiday to look further into this.

@richievos
Copy link
Author

No problem. While non-optimal, I think putting a note in the readme would go a long way.

@clarkdevis
Copy link

Timecop.return
ap Time.zone.now
Timecop.freeze(1.day.ago) {ap Time.zone.now}
ap Time.zone.now

=> Wed, 31 Aug 2011 09:52:50 UTC +00:00

Timecop.return
ap Time.zone.now
Timecop.freeze 1.day.ago {ap Time.zone.now}
ap Time.zone.now

=> Tue, 30 Aug 2011 09:53:22 UTC +00:00

Timecop.return
ap Time.zone.now
Timecop.freeze 1.day.ago do ap Time.zone.now end
ap Time.zone.now

=> Wed, 31 Aug 2011 09:59:41 UTC +00:00

Such behavior is not obvious!

@pcasaretto
Copy link

Did the first thing (block auto-return) ever get fixed?

@travisjeffery
Copy link
Owner

@pcasaretto yep, it's fixed.

[19] pry(main)> Time.now
=> 2012-07-26 15:58:13 -0500
[20] pry(main)> Timecop.freeze { Timecop.travel(10.days) { puts Time.now } }
2012-08-05 15:58:31 -0500
=> 2012-07-26 15:58:31 -0500
[21] pry(main)> Time.now
=> 2012-07-26 15:58:38 -0500

If there's another problem please make a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants