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

Calling Time.zone.at on a TimeWithZone can have different behavior than calling Time.zone.at on a Time #40413

Closed
LandonSchropp opened this issue Oct 19, 2020 · 5 comments

Comments

@LandonSchropp
Copy link

Steps to reproduce

We ran across this tricky bug in our application. The behavior for a time at the end of a month changes based upon whether it's a Time or a TimeWithZone.

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{ repo }.git" }

  # Activate the gem you are reporting the issue against.
  gem "activesupport", "6.0.3"
end

require "active_support"
require "active_support/core_ext"
require "minitest/autorun"

class BugTest < Minitest::Test
  def test_stuff
    Time.zone = "Etc/UTC"

    end_of_january_without_time_zone = "2019-01-01 00:00:00Z".to_time.end_of_month
    end_of_january_with_time_zone = end_of_january_without_time_zone.in_time_zone

    puts "Time objects:"
    p end_of_january_with_time_zone
    p end_of_january_without_time_zone
    puts

    puts "Calling Time.zone.at on time objects:"
    p Time.zone.at(end_of_january_with_time_zone)
    p Time.zone.at(end_of_january_without_time_zone)
    puts

    assert_equal(
      Time.zone.at(end_of_january_with_time_zone),
      Time.zone.at(end_of_january_without_time_zone)
    )
  end
end

Expected behavior

The two times returned from Time.zone.at should be identical.

Actual behavior

Somewhere along the way, the TimeWithZone is getting rounded up.

System configuration

Rails version: 6.0.3
Ruby version: 2.7.1p83

@LandonSchropp
Copy link
Author

(I added the print statements to help clarify the problem. They're not necessary for the spec to fail.)

@LandonSchropp
Copy link
Author

LandonSchropp commented Oct 22, 2020

@BKSpurgeon Yep, all of that looks correct to me. I realize this seems like a strange test case to bring up, but we ran into a real-world scenario that's triggered by this. Our tests were passing for Time objects that we were creating, but the code was failing from the API because it was using TimeWithZone.

The test case I posted above was my attempt to simplify this problem so it was reproducible.

@benkoshy
Copy link
Contributor

benkoshy commented Oct 24, 2020

This is where the rounding off seems to occur:

at_without_coercion(time_or_number.to_f).getlocal  
# time_or_number.to_f => 1548939600.0   # which is not what we want, per se.

def at_with_coercion(*args)
     # details skipped
      if time_or_number.is_a?(ActiveSupport::TimeWithZone) || time_or_number.is_a?(DateTime)
        at_without_coercion(time_or_number.to_f).getlocal    # with TimeWithZone, things get rounded up here
      else
        at_without_coercion(time_or_number) # when dealing with a Time object we hit this branch, and everything is fine
      end
end

I don't think this is or should be the intended behaviour - that extra Rational should be preserved as much as possible. Perhaps a new object should be passed into at_without_coercion? Will have to think about a solution to this one.

@eugeneius
Copy link
Member

Fixed by #40448.

@LandonSchropp
Copy link
Author

@eugeneius Awesome! Thank you so much. 🙂

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

3 participants