travisjeffery / timecop

A gem providing "time travel", "time freezing", and "time acceleration" capabilities, making it simple to test time-dependent code. It provides a unified method to mock Time.now, Date.today, and DateTime.now in a single call.
MIT License
3.36k stars 223 forks source link

ActiveSupport Date.current differs from Date.today when Time.zone is set #416

Closed jch closed 3 weeks ago

jch commented 5 months ago

Edit: https://github.com/travisjeffery/timecop/pull/416#issuecomment-2289702440 for diagnosis

Original description:

I suspect https://github.com/travisjeffery/timecop/blob/e134761e942c612f8199f2d83793995653f810cb/lib/timecop/time_stack_item.rb#L59 calling localtime to be causing this issue. My system localtime is -7, so this cast is changing the date to the previous day. Diff is a failing test.

joshuacronemeyer commented 3 weeks ago

@jch this test does not fail for me. also checkout the each_timezone test helper we've got. I was careful to make sure it had a timezone where today is tomorrow for me and still cant get this test to fail.

jch commented 3 weeks ago

@joshuacronemeyer thanks for taking a look. how were you running the test? I realized I didn't have the _test suffix on the filename so it wasn't running in rake test. Updated it, and running the specific test file fails for me with the latest master changes merged:

$  ruby test/active_support_date_current_test.rb
Run options: --seed 39850

# Running:

F.

Finished in 0.050811s, 39.3616 runs/s, 39.3616 assertions/s.

  1) Failure:
TestActiveSupportDateCurrent#test_date_current_same_as_date_today_with_time_zone [test/active_support_date_current_test.rb:24]:
--- expected
+++ actual
@@ -1 +1 @@
-#<Date: 2024-03-15 ((2460385j,0s,0n),+0s,2299161j)>
+#<Date: 2024-03-14 ((2460384j,0s,0n),+0s,2299161j)>

2 runs, 2 assertions, 1 failures, 0 errors, 0 skips

The test doesn't fail if I use each_timezone test helper. I suspect this is ENV["TZ"] is used by some railtie to set Time.zone explicitly but haven't confirmed. What is your system timezone set to?

I didn't see this previously, but the README hints at this old issue https://rails.lighthouseapp.com/projects/8994/tickets/6410-dateyesterday-datetoday and describes the unexpected behavior.

Setting breakpoints within the freeze block, Date.current uses the Time extensions, whereas Date.today uses the Date extensions:

        # @time: ActiveSupport::TimeWithZone: Fri, 15 Mar 2024 00:00:00.000000000 UTC +00:00
        # time: Time: 2024-03-14 17:00:00 -0700
        #0  Timecop::TimeStackItem#time(time_klass=Time) at ~/timecop/lib/timecop/time_stack_item.rb:79
        #1  Time.mock_time at ~/timecop/lib/timecop/time_extensions.rb:8
        #2  Time.now at ~/timecop/lib/timecop/time_extensions.rb:14
        #3  TZInfo::Timezone#now at ~/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/gems/tzinfo-2.0.6/lib/tzinfo/timezone.rb:993 (to_local(Time.now))
        #4  ActiveSupport::TimeZone#today at ~/.rbenv/versions/3.3.0/lib/ruby/gems/3.3.0/gems/activesupport-7.1.3.4/lib/active_support/values/time_zone.rb:515 (tzinfo.now.to_date)
        #5  Date.current  3/15 <----- CORRECT
        Date.current

        # @time: ActiveSupport::TimeWithZone: Fri, 15 Mar 2024 00:00:00.000000000 UTC +00:00
        # time: Time 2024-03-14 17:00:00 -0700
        #0  Timecop::TimeStackItem#time(time_klass=Time) at ~/timecop/lib/timecop/time_stack_item.rb:79
        #1  Timecop::TimeStackItem#date(date_klass=Date) at ~/timecop/lib/timecop/time_stack_item.rb:99 (calls to_date)
        #2  Date.mock_date at ~/timecop/lib/timecop/time_extensions.rb:34
        #3  Date.today  3/14 <----- INCORRECT
        Date.today

My original suspicion was wrong because the call to TimeStackItem#time actually sets the same value for @time and the returned time. The difference being Date.current uses casts to the set UTC timezone before calling to_date, whereas Date.today calls to_date on the returned value directly.

I've proposed a change in 603b64dded557e703408c09499f99c015e7e6b99 for Date.today to mirror the behavior of Date.current by changing TimeStackItem#date:

diff --git a/lib/timecop/time_stack_item.rb b/lib/timecop/time_stack_item.rb
index ef49150..0767f71 100644
--- a/lib/timecop/time_stack_item.rb
+++ b/lib/timecop/time_stack_item.rb
@@ -96,7 +96,9 @@ class Timecop
     end

     def date(date_klass = Date)
-      date_klass.jd(time.__send__(:to_date).jd)
+      t = time
+      t = t.respond_to?(:in_time_zone) ? t.in_time_zone : t
+      date_klass.jd(t.__send__(:to_date).jd)
     end
joshuacronemeyer commented 3 weeks ago

@jch can you create a new bundle and reproduce this issue in isolation? Your test is not failing on my machine OR on the time cop build... https://github.com/travisjeffery/timecop/actions/runs/10393723273/job/28788507003?pr=416

jch commented 3 weeks ago

Testing this out in a codespace, the test passes because the system time in the instance is UTC and matches Time.zone in my test. I switched to the test helper each_timezone which will change the system time. This was a weird gotcha, and I realize now I should use Time.zone.today instead of Date.today in my rails app for a time zone aware "today" https://stackoverflow.com/a/19016136

Closing this out as I believe that Timecop shouldn't add additional behavior that differs from ActiveSupport defaults, even if this was confusing. Hopefully this closed issue will help others if they run into a similar issue.

Thanks @joshuacronemeyer for checking me :bow:

joshuacronemeyer commented 3 weeks ago

Been there. Timezones are the worst!