ActiveSupport::TimeWithZone and Time mixins cause incorrect behaviour

[wiggly]

Hello,

I have been hunting a bug in my code and eventually tracked it down to the following problem and minimal script that demonstrates the issue.

I develop on 3.2.13 but I have run this script against 4-1-stable and the problem exists there as well.

While it is a problem for me I am wondering if this is expected behaviour, I hope not.

The following code demonstrates the problem;

https://gist.github.com/wiggly/6997961

If you add a mixin to the core Time class that provides before?/after? functionality then it causes the method to be called via method_missing in TimeWithZone. Doing this passes the result of TimeWithZone.time to the method in Time.

The issue, I believe, is with the output of TimeWithZone.time. It takes the internal time and uses the time zone period to convert it to local time. This generates a new Time object that has a different internal epoch-seconds than the TimeWithZone.

The subsequent comparison now fails even when the method is passed the receiver as the second argument, see gist above.

I have figured things out to this point and am wondering if I can get someone within the rails team to give their thoughts. I can fix this by applying the mixin to the TimeWithZone class so my problem is solved for now but I think that the behaviour is incorrect and leads to very difficult to track down bugs. This had me baffled for a long time before I started looking at rails/activesupport for the culprit.

NB: This was previously opened as #12551. Closed due to lack of comment. I have done as the emails suggested and tested this against 4-1-stable and the problem still exists. Nothing has changed. I submit this is still an issue.

[pixeltrix]

The behaviour of ActiveSupport::TimeWithZone#time is dependent on how it has been initialised - if it's generated from a local time then nothing is done to the time value as the @time instance variable is defined. If however the TimeWithZone instance has been generated using UTC then we convert it to a local time as you point out. Since this is used to support features that cross DST boundaries then I'm not sure what else you'd want us to do as just returning the UTC time will break a lot of tests.

As you say, including the module in TimeWithZone makes it work and that's essentially what we have to do internally since we define TimeWithZone#<=> as a comparison with UTC time. So to answer your question, yes this is expected behaviour - sorry.