Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #5617 (closed defect: fixed)

Opened 2 years ago

Last modified 2 years ago

[PATCH] daylight savings time awareness for since/ago of at least 1.day (with tests)

Reported by: rob@AgileConsultingLLC.com Assigned to: David
Priority: normal Milestone:
Component: ActiveSupport Version: edge
Severity: minor Keywords:
Cc: michael@koziarski.com

Description

In response to a ML post time.next_week bug? and related to tickets 2353, 2509, 4551, the attached patch causes Time#since to be aware of dst? changes and adjust the result accordingly if the requested delta was 86400 (1.day) or more.

It would be better if the semantics were better defined for sub-day or non-integral-days deltas, but the behavior will be closer to what many expect for things like:

Time.local(2005,10,30).since(1.day) => Mon Oct 31 00:00:00 EST 2005

Unfortunately, I don't think it right to give this knowledge of DST to the Numeric extension so the unexpected behavior of:

1.day.since(Time.local(2005,10,30)) => Sun Oct 30 23:00:00 EST 2005

remains.

Comments on better semantics for <1.day changes or on Numeric#since acquiring this instead are welcomed. (As the Time#since delegated to Numeric#since which just lets the Time class handle the change in seconds, it seems like a coin flip, but I could accept that either the changes belong in Numeric or that Numeric ought to delegate to Time to restore the symmetry.)

-Rob

Attachments

handle_dst_changes_for_large_deltas.txt (3.5 kB) - added by rob@AgileConsultingLLC.com on 07/06/06 02:23:09.
handle_dst_changes_for_large_deltas_NEW_tests1.patch (0.8 kB) - added by rabiedenharn on 10/30/06 21:21:17.
Tests that show #6483 to be an incomplete fix
handle_dst_changes_for_large_deltas_NEW_tests2.patch (3.6 kB) - added by rabiedenharn on 10/30/06 21:22:54.
Additional tests to show behavior required of *since* and *seconds_since_midnight*
handle_dst_changes_for_large_deltas_NEW_code.patch (2.3 kB) - added by rabiedenharn on 10/30/06 21:24:18.
Reverses the code change to *next_week* from #6483 and applies changes to *since* (and *ago*) and *seconds_since_midnight*
include_dst_tests_for_multiple_timezones.patch (8.2 kB) - added by rabiedenharn on 11/03/06 00:18:03.

Change History

07/06/06 02:23:09 changed by rob@AgileConsultingLLC.com

  • attachment handle_dst_changes_for_large_deltas.txt added.

10/30/06 21:20:22 changed by rabiedenharn

  • version changed from 1.1.1 to edge.
  • type changed from enhancement to defect.
  • severity changed from normal to minor.

I've included additional tests in response to the accepted ticket #6483 ([PATCH] next_week method fails during DST change) because that fix only works properly during the switch from Daylight to Standard time (i.e., back an hour) and still fails during the last hour of the last day of the week.

I've broken the new files up into:

  • handle_dst_changes_for_large_deltas_NEW_tests1.patch - which has the tests to show that the patch from #6483 (accepted into [5363]) is incomplete
  • handle_dst_changes_for_large_deltas_NEW_tests2.patch - which has the tests to handle general DST crossings of at least 1.day and seconds_since_midnight always
  • handle_dst_changes_for_large_deltas_NEW_code.patch - which reverses the change from ticket #6483 and adds my changes for *since* and *seconds_since_midnight*

All patches are from the top-level directory.

10/30/06 21:21:17 changed by rabiedenharn

  • attachment handle_dst_changes_for_large_deltas_NEW_tests1.patch added.

Tests that show #6483 to be an incomplete fix

10/30/06 21:22:54 changed by rabiedenharn

  • attachment handle_dst_changes_for_large_deltas_NEW_tests2.patch added.

Additional tests to show behavior required of *since* and *seconds_since_midnight*

10/30/06 21:24:18 changed by rabiedenharn

  • attachment handle_dst_changes_for_large_deltas_NEW_code.patch added.

Reverses the code change to *next_week* from #6483 and applies changes to *since* (and *ago*) and *seconds_since_midnight*

11/02/06 01:52:13 changed by bitsweat

  • status changed from new to closed.
  • resolution set to fixed.

(In [5388]) next_week respects DST changes. Closes #5617, closes #2353, closes #2509, references #4551.

11/02/06 20:21:46 changed by nzkoz

  • cc set to michael@koziarski.com.
  • status changed from closed to reopened.
  • resolution deleted.

The attached tests are incorrect for the entire southern hemisphere and any country in the northern hemisphere which does daylight changes on a different date than the united states. Specifically: 

test_seconds_since_midnight_at_daylight_savings_time_start
test_seconds_since_midnight_at_daylight_savings_time_end

Could you provide updated tests which take this into account?

11/02/06 20:36:07 changed by bitsweat

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [5418]) Use US timezone for DST tests. Closes #5617.

11/03/06 00:00:53 changed by rabiedenharn

  • status changed from closed to reopened.
  • resolution deleted.

Added tests so that both US/Eastern and NZ are represented when DST is concerned.

The attached 

include_dst_tests_for_multiple_timezones.patch

is a diff against [5419]

11/03/06 00:18:03 changed by rabiedenharn

  • attachment include_dst_tests_for_multiple_timezones.patch added.

11/03/06 00:40:35 changed by bitsweat

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [5420]) Test multiple timezones' DST. Closes #5617.