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

Time.now sometimes has greater-than-nanosecond precision when called under Timecop.travel #420

Open
dgholz opened this issue May 14, 2024 · 9 comments · Fixed by #421
Open

Comments

@dgholz
Copy link
Contributor

dgholz commented May 14, 2024

Hello, I have a test that looks like:

Timecop.travel(some_date)
company = create_company
[…]
Timecop.freeze
EmailPolicy::Actions::Block(company)
expect(Preferences::EmailRateLimit.for(company).blocked_until).to eq(6.hours.from_now)

Where EmailPolicy::Actions::Block calls Time.now, and serialised the date to interact with other systems, and Preferences::EmailRateLimit deserialised the date. Times are serialised down to the nanosecond.

It failed with:

       Failure/Error: expect(Preferences::EmailRateLimit.for(company).blocked_until).to eq(6.hours.from_now)
  
         expected: 2024-05-14 19:42:47.696001634 +0000
              got: 2024-05-14 19:42:47.696001634 +0000

Which baffled me a bit, since I didn't consider that the Time used to calculate 6.hours.from_now would have a fraction of a nanosecond difference to the deserialised Time from the Preferences system.

Previously, the test only had the Timecop.freeze line & worked great. It was only after adding the Timecop.travel that the two identical-looking Times started comparing as unequal. So that made me look at how the travel_offset was added to create the Time.now used for Timecop.freeze, and I saw it was a Float with a much higher precision than was possible with a computer clock.

I have some ideas on how to change my test to pass, but thought I'd raise this issue to highlight that the Times returned under Timecop.travel may sometimes have more precision than you might expect, and shouldn't be compared to Times created from other sources.

@dgholz
Copy link
Contributor Author

dgholz commented May 14, 2024

Times returned under Timecop.travel may sometimes have more precision than you might expect, and shouldn't be compared to Times created from other sources.

I opened #421 as a possible approach to removing this caveat, though I do worry that the test doesn't cover the case when Timecop.travel is called with a Time created with fractions of a nanosecond.

@jasonkingPNM
Copy link

Nice, just stumbled over this today.

@dgholz
Copy link
Contributor Author

dgholz commented Jun 27, 2024

the test doesn't cover the case when Timecop.travel is called with a Time created with fractions of a nanosecond

Resolved, I added a test explicitly for this.

@joshuacronemeyer
Copy link
Collaborator

Reopening this issue since i had to revert the PR

@jasonkingPNM
Copy link

What was the issue?

@joshuacronemeyer
Copy link
Collaborator

@jasonkingPNM tests were not passing. I didn't realize the test suite didn't run on this PR when I merged it in it broke tests and I had to revert it because it wasn't obvious what was wrong #430

@jasonkingPNM
Copy link

Ah, cool. Thanks.

@jasonkingPNM
Copy link

Wow, hard to repro:

~/work/timecop ruby test/time_stack_item_test.rb                                                                                                                                                                  HEAD e05c2c0 16:01:29
Run options: --seed 6834

# Running:

..DEPRECATION WARNING: to_time will always preserve the timezone offset of the receiver in Rails 8.0. To opt in to the new behavior, set `ActiveSupport.to_time_preserves_timezone = true`. (called from parse_time at /Users/jasonking/work/timecop/lib/timecop/time_stack_item.rb:136)
.....................F........

Finished in 0.128306s, 249.4038 runs/s, 623.5094 assertions/s.

  1) Failure:
TestTimeStackItem#test_travel_offset_aligns_to_travel_time [test/time_stack_item_test.rb:314]:
travel offset precision (36028797018963968000000) does not align with travel time precision (1441151880758558720000).
Expected: 0
  Actual: 1441151880758558720000

32 runs, 80 assertions, 1 failures, 0 errors, 0 skips
~/work/timecop ruby test/time_stack_item_test.rb --seed 6834                                                                                                                                                      HEAD e05c2c0 16:01:30
Run options: --seed 6834

# Running:

..DEPRECATION WARNING: to_time will always preserve the timezone offset of the receiver in Rails 8.0. To opt in to the new behavior, set `ActiveSupport.to_time_preserves_timezone = true`. (called from parse_time at /Users/jasonking/work/timecop/lib/timecop/time_stack_item.rb:136)
..............................

Finished in 0.135477s, 236.2025 runs/s, 590.5061 assertions/s.

32 runs, 80 assertions, 0 failures, 0 errors, 0 skips

@jasonkingPNM
Copy link

Oh, these are all Time.now sensitive, of course :)

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

Successfully merging a pull request may close this issue.

3 participants