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

Timezone test case is broken #433

Open
dmytro-strukov opened this issue Dec 19, 2024 · 8 comments
Open

Timezone test case is broken #433

dmytro-strukov opened this issue Dec 19, 2024 · 8 comments

Comments

@dmytro-strukov
Copy link

dmytro-strukov commented Dec 19, 2024

It does not change the time zone:

  # https://github.com/travisjeffery/timecop/blob/master/test/time_stack_item_test.rb#L278
  # 
  def test_freezing_a_time_with_zone_returns_proper_zones
    Time.zone = "Hawaii"
    t = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 1), ActiveSupport::TimeZone['Tokyo'])
    Timecop.freeze(t) do
      local_now = Time.now
      assert_equal t, local_now
      assert_equal t.getlocal.zone, local_now.zone

      zoned_now = Time.zone.now
      assert_equal t, zoned_now
      assert_equal 'HST', zoned_now.zone
    end
  end
[26] pry(#<TestTimeStackItem>)> t.zone
=> "JST"
[27] pry(#<TestTimeStackItem>)> local_now.zone
=> "+04"
[28] pry(#<TestTimeStackItem>)> t.getlocal.zone
=> "+04"

@travisjeffery @joshuacronemeyer Is it expected behavior? I wanted to use in my test cases Time.utc(2024, 1, 1), but because we use localtime here https://github.com/travisjeffery/timecop/blob/master/lib/timecop/time_stack_item.rb#L78 it does not work

@joshuacronemeyer
Copy link
Collaborator

Hey @dmytro-strukov i'm not sure I understand the problem here. When i try your code the behavior is the same with timecop or without. It's not clear to me from your question why you can't use Time.utc(2024, 1, 1). I'm not sure where you're trying to use it.

Your issue says "It does not change the time zone" but i'm not sure what timezone you expect to change.

@dmytro-strukov
Copy link
Author

@joshuacronemeyer Let me clarify, In the test case t variable was declared with the timezone Tokyo. In the test case, we don't test that the current timezone is Tokyo.

Regarding Time.utc(2024, 1, 1), let's say I want to use it:

Timecop.freeze(Time.utc(2000, 1, 1))
puts Time.now

=> 2000-01-01 04:00:00 +0400 # I suppose it should be UTC timezone, not +0400 (my current local timezone)

And let's compare it with the next:

utc_tz = ActiveSupport::TimeZone.find_tzinfo('UTC') 
=> #<TZInfo::DataTimezone: Etc/UTC>

Timecop.freeze(Time.new(2024, 1, 1, 0, 0, 0, utc_tz))
puts Time.now

=> 2024-01-01 00:00:00 +0000

@joshuacronemeyer
Copy link
Collaborator

@dmytro-strukov Are you running these commands from an environment where Rails is included? Rails does a lot of fancy stuff with timezones so that the timezone set in configuration is respected but can be overridden for the entire thread running a request as well. I think a nice-but-kind-of-old summary of Rails view of this is https://thoughtbot.com/blog/its-about-time-zones

When I run your code in a non-rails environment: Timecop.freeze(Time.utc(2000, 1, 1)) it will work as expected.

josh@MacBook-Pro-2 ~/workspace/timecop_sandbox $ irb                                                                                                                                        [ruby-3.2.2]
3.2.2 :001 > require 'Timecop'
 => true
3.2.2 :002 > Timecop.freeze(Time.utc(2000, 1, 1))
 => 2000-01-01 00:00:00 UTC
3.2.2 :003 > puts Time.now
2000-01-01 00:00:00 UTC

@dmytro-strukov
Copy link
Author

@joshuacronemeyer I tried in the IRB console your example. Yes, this issue occurs only in the Rails environment, I tested on a couple of projects Rails 7.x.x and Ruby 3.x, and on the oldest versions like Rails 5.x.x and Ruby 2.x

@joshuacronemeyer
Copy link
Collaborator

Have you looked at using Time.current instead of Time.now? I think that is the intended way to get current time in a rails app. That method will respect all the layers of rails timezone configuration.

3.2.2 :002 > Timecop.freeze(Time.utc(2000, 1, 1))
 => 1999-12-31 16:00:00 -0800
3.2.2 :003 > Time.current
 => Sat, 01 Jan 2000 00:00:00.000000000 UTC +00:00
3.2.2 :004 > Time.current.zone
 => "UTC"

@dmytro-strukov
Copy link
Author

@joshuacronemeyer Yes, sure 👍 . Do you want to add info regarding this behavior into README and work on the test case?

@joshuacronemeyer
Copy link
Collaborator

Yea it would be nice to have a section in the README about rails best practices. I'm not sure what you mean about the test case but i'm open to whatever you think will help make behavior clear.

@dmytro-strukov
Copy link
Author

@joshuacronemeyer In method time, we have the condition to check if we pass ActiveSupport::TimeWithZone (tied to Rails) or some plain Ruby Time object.

    def time(time_klass = Time) #:nodoc:
      if @time.respond_to?(:in_time_zone)
        time = time_klass.at(@time.dup.localtime)
      else
        time = time_klass.at(@time)
      end

In the test case, verifications seem unuseful, and very unclear, let me explain:

t = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 1), ActiveSupport::TimeZone['Tokyo'])
Timecop.freeze(t) do
  local_now = Time.now
  # => 2000-01-01 04:00:00 +0400
  assert_equal t, local_now
  • Why Time.now and not Time.current if we test Rails behavior.
  • The comparison of dates is very implicit. Under the hood, it uses method to_i, inside to_i method Rails call utc.to_i. So, as I mentioned it is not clear for me what we check here
assert_equal t.getlocal.zone, local_now.zone

Ok, here I suppose we want to check that local timezone wasn't changed.

      Time.zone = "Hawaii"
      ...
      zoned_now = Time.zone.now
      # => 1999-12-31 14:00:00.000000000 HST -10:00
      assert_equal t, zoned_now # very implicit
      assert_equal 'HST', zoned_now.zone

So..if we before globally set Time.zone and after this try to use Timecop.freeze it should not change the timezone, right? Time.zone has higher priority, but I don't see documentation regarding this. @joshuacronemeyer Please correct me if I'm wrong on something and share your thoughts

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

2 participants