ActiveSupport::TimeWithZone compare fails between Time/DateTime in 3.1 (3.0 works)

[jconley]

test case:

time_string = '2012-02-10T16:55:00+00:00'
t1 = Time.parse(time_string).in_time_zone
t2 = DateTime.parse(time_string).in_time_zone
t1 <=> t2

correct result of 0 in rails 3.0.10
invalid result of nil in rails 3.1.1

t1.eql?(t2) is also affected, returning nil instead of true in 3.1.1

[MDaubs]

I am unable to reproduce the test case. I get the same result below on 3.0.10, 3.1.1, 3-1-stable, and edge. Note that I used a different time string to more clearly see when the time zone is and isn't parsed. My local time zone is UTC-5.

ruby-1.9.2-p290 :001 > require 'active_support/all'
 => true 
ruby-1.9.2-p290 :002 > time_string = '2012-02-10T16:55:00+02:00'
 => "2012-02-10T16:55:00+02:00" 
ruby-1.9.2-p290 :003 > t1 = Time.parse(time_string).in_time_zone
 => 2012-02-10 09:55:00 -0500 
ruby-1.9.2-p290 :004 > t2 = DateTime.parse(time_string).in_time_zone
 => Fri, 10 Feb 2012 16:55:00 +0200 
ruby-1.9.2-p290 :005 > t1 == t2
 => true 
ruby-1.9.2-p290 :006 > t1 <=> t2
 => 0 
ruby-1.9.2-p290 :007 > t1.eql? t2
 => false 

[jconley]

fails for me using:
ruby 1.8.7 (2011-02-18 patchlevel 334) [i686-darwin11.0.0], MBARI 0x6770, Ruby Enterprise Edition 2011.03

works when i use:
ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin11.2.0]

I can only reproduce in a stock rails install console, standalone irb loading with gem+require works on both ruby versions

[MDaubs]

I can confirm, works at irb with require 'active_support/all' but does not work from rails console when using MRI 1.8.7:

Loading development environment (Rails 3.1.1)
:001 > time_string = '2012-02-10T16:55:00+00:00'
 => "2012-02-10T16:55:00+00:00" 
:002 > t1 = Time.parse(time_string).in_time_zone
 => Fri, 10 Feb 2012 16:55:00 UTC +00:00 
:003 > t2 = DateTime.parse(time_string).in_time_zone
 => Fri, 10 Feb 2012 16:55:00 UTC +00:00 
:004 > t1 <=> t2
 => nil 

Works when I use 1.9.2 and rails console. Looks like the time zone parsing is completely different on 1.8.7, not sure if it's related.

[jeremyf]

@jconley Any luck writing an automated test case for this?

[steveklabnik]

Looks like the time zone parsing is completely different on 1.8.7, not sure if it's related.

I know that strftime was re-written for 1.9. In 1.8, it calls to the system implementation, in 1.9, there's an implementation inside of Ruby itself. (in C, of course.)

[steveklabnik]

Since this works with 1.9, it will be fine on master, since Rails 4 will only support 1.9.3.

That said, this could still be fixed for 3-2-stable if anyone is interested.

[chrisrobbie]

I had a look at this one and got a bit further but still not sure what the issue is, so Im noting here what i found.

I noticed that t2 can be any DateTime and the spaceship comparison will return nil.

So using Ruby 1.8.7 with Rails 3.2.8 in the rails console:

1.8.7 :002 > t1 <=> DateTime.now.in_time_zone
 => nil 

While passing a regular DateTime to spaceship, rather than a TimeWithZone, seems to work.

1.8.7 :004 > t1 <=> DateTime.now
 => -1 
1.8.7 :005 > t1 <=> DateTime.now.in_time_zone.to_datetime
 => -1 

So t1 is based on Time, and looking at the ruby Time class spaceship method, it does type checking and returns nil if that check fails.
So this seems to explain why the nil is coming back, in other words, Time.<=> is receiving t2, checking the type of t2, and returning nil.

Also to note, as previously mentioned, the issue happens in rails console, but is fine in irb with active_support/all required.
Ruby 1.9.3 works fine.

[arunagw]

Just found

1.8.7 :060 > t2 <=> t1
 => 0

[unorthodoxgeek]

same for me, MRI 1.8.7, result is as expected on rails 3.2-stable and 3.1-stable.

[steveklabnik]

@unorthodoxgeek please note that 3.1 isn't getting any fixes at all, and 3.2 is only getting security fixes, and 1.8.7 support has been dropped from Rails 4 entirely.

[pftg]

I can confirm that this fixed for ruby 1.9.3, 2.0.0 and Rails 4.0.1.rc1 and master. Should this issue be closed?

[pixeltrix]

Yep, closing because it works correctly on Rails 3.2.15 and Ruby 1.8.7

[why-el]

This does not work in master.