Daylight Saving Time Issues - Yet another Time

[MarkusHarmsen]

First of all, many thanks for your awesome gem!

I would love to use it in a project, but sadly our test suite failed when checking some nasty DST changes.
After digging deeper, I was able to build a minimal failing example:

require 'ice_cube'
require 'active_support/time'

# Set a fixed timezone
Time.zone = 'Berlin'

# Setup required dates (daily from 16 to 9 o'clock the next day)
start_time = Time.zone.parse '2017-10-01 16:00' # Within DST
end_time   = Time.zone.parse '2017-10-02 09:00' # Within DST

# Build schedule
schedule = IceCube::Schedule.new(start_time, end_time: end_time)
schedule.add_recurrence_rule IceCube::Rule.daily

# Set reference date, which is at 8 o'clock and should be within the schedule
now = Time.zone.parse '2017-10-29 08:00' # Not within DST (nasty stuff here)

puts schedule.next_occurrence now, spans: true

# Result
#  2017-10-29 16:00:00 +0100 - 2017-10-30 09:00:00 +0100
# but actually the "real" next occurrence should be
#  2017-10-28 16:00:00 +0200 - 2017-10-29 09:00:00 +0100

This happens on master as well as on the full_tz branch.

There some related (closed) issues, but I could not see if this is the expected behavior or a concrete "bug".
An attempt to fix that issue has been done here: MarkusHarmsen@0d6f6ef
but although it works "correcting" the DST change for the end_time in an occurrence, it fails to catch the edge case seen above (it seems that the ValidatedRule skips the required date).

Greetings,

Markus

[avit]

Thanks @MarkusHarmsen,

Your assessment looks correct. Is the spans: true option relevant to this? I could not see a difference in the output.

We did recently fix an issue with doubled occurrences over DST, and removed some complicated code that was handling it. (See: #404) We previously had a similar method there to wind_back_dst around these scenarios, similar to what you wrote. I will take a closer look and determine if we need to reverse some of those changes.

[MarkusHarmsen]

Hey @avit,

I'm not 100% sure about the next_occurrence implementation, but I understood that spans: true can be set if a "overlapping" occurrence is fine as well (i.e start_time is before - and end_time is after reference date) or if the next occurrence must start after given reference date (i.e. both start_time and end_time have to be after the reference date).

If that understanding is correct, spans: true would be required to return the expected occurrence from the example above (2017-10-28 16:00:00 +0200 - 2017-10-29 09:00:00 +0100). spans: false should return the current result of the example above (2017-10-29 16:00:00 +0100 - 2017-10-30 09:00:00 +0100).

[avit]

Ok, I looked into this further and confirmed it's not a bug.

Your example schedule has a duration of 17 hours: that means each occurrence must have the same length.

Because the end of DST adds one hour, that occurrence actually ends at 08:00, not at 09:00 like on other days, so the selection of the "next occurrence" is correct.

I considered whether the occurrence should have a different length in this situation, but based on the spec, it should not. Internally, we specify the duration in seconds so it should always be an exact period. However, the spec does allow for setting duration in different amounts, e.g. "1D" should mean a day, regardless of whether it is 24 or 25 hours... IceCube currently doesn't support this, but it could be added without changing the current meaning of duration.