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

Feature/timezone support test cases #16

Conversation

bgentry
Copy link

@bgentry bgentry commented Sep 2, 2018

This adds additional test cases to #15 to verify the correct behavior around Daylight Saving Time boundaries.

Two of these tests are currently broken. They relate to times that would occur twice during the transition from daylight to standard time, or times that don't exist during the transition from standard to daylight time. These are explicitly detailed by RFC5545:

If, based on the definition of the referenced time zone, the local
time described occurs more than once (when changing from daylight
to standard time), the DATE-TIME value refers to the first
occurrence of the referenced time. Thus, TZID=America/
New_York:20071104T013000 indicates November 4, 2007 at 1:30 A.M.
EDT (UTC-04:00). If the local time described does not occur (when
changing from standard to daylight time), the DATE-TIME value is
interpreted using the UTC offset before the gap in local times.
Thus, TZID=America/New_York:20070311T023000 indicates March 11,
2007 at 3:30 A.M. EDT (UTC-04:00), one hour after 1:30 A.M. EST
(UTC-05:00).

Here is the spring behavior correctly implemented in Google Calendar:

screen shot 2018-09-02 at 11 14 27 am

Here's the failure in this test as of now:

  1) test should handle timezones at the spring DST boundary for a 2:30 AM event (RecurringEvents.TimezoneTest)
     test/timezone_test.exs:70
     Assertion with == failed
     code:  assert result == date_tz_expand([{{2019, 3, 8}, {5, 0, 0}}, {{2019, 3, 9}, {2, 30, 0}}, {{2019, 3, 10}, {3, 30, 0}}, {{2019, 3, 11..12}, {2, 30, 0}}], "America/New_York")
     left:  [
              #DateTime<2019-03-08 05:00:00-05:00 EST America/New_York>,
              #DateTime<2019-03-09 02:30:00-05:00 EST America/New_York>,
              #DateTime<2019-03-10 02:30:00-04:00 EDT America/New_York>,
              #DateTime<2019-03-11 02:30:00-04:00 EDT America/New_York>,
              #DateTime<2019-03-12 02:30:00-04:00 EDT America/New_York>]
     right: [
              #DateTime<2019-03-08 05:00:00-05:00 EST America/New_York>,
              #DateTime<2019-03-09 02:30:00-05:00 EST America/New_York>,
              #DateTime<2019-03-10 03:30:00-04:00 EDT America/New_York>,
              #DateTime<2019-03-11 02:30:00-04:00 EDT America/New_York>,
              #DateTime<2019-03-12 02:30:00-04:00 EDT America/New_York>]
     stacktrace:
       test/timezone_test.exs:80: (test)

And the failure for the fall DST issue:

  2) test should handle timezones at the fall DST boundary for a 1:30 AM event (RecurringEvents.TimezoneTest)
     test/timezone_test.exs:24
     Assertion with == failed
     code:  assert result == date_tz_expand([{{2018, 11, 2}, {5, 0, 0}}, {{2018, 11, 3..6}, {1, 30, 0}}], "America/Los_Angeles")
     left:  [
              #DateTime<2018-11-02 05:00:00-07:00 PDT America/Los_Angeles>,
              #DateTime<2018-11-03 01:30:00-07:00 PDT America/Los_Angeles>,
              #DateTime<2018-11-04 01:30:00-08:00 PST America/Los_Angeles>,
              #DateTime<2018-11-05 01:30:00-08:00 PST America/Los_Angeles>,
              #DateTime<2018-11-06 01:30:00-08:00 PST America/Los_Angeles>]
     right: [
              #DateTime<2018-11-02 05:00:00-07:00 PDT America/Los_Angeles>,
              #DateTime<2018-11-03 01:30:00-07:00 PDT America/Los_Angeles>,
              #<Ambiguous(#DateTime<2018-11-04 01:30:00-07:00 PDT America/Los_Angeles> ~ #DateTime<2018-11-04 01:30:00-08:00 PST America/Los_Angeles>)>,
              #DateTime<2018-11-05 01:30:00-08:00 PST America/Los_Angeles>,
              #DateTime<2018-11-06 01:30:00-08:00 PST America/Los_Angeles>]
     stacktrace:
       test/timezone_test.exs:34: (test)

Note that the expected value here is also wrong because it has an ambiguous timezone result (since 1:30AM occurs twice). We likely need to explicitly choose the expected timezone for this result to make sure it is the UTC-7 / PDT value, not the UTC-8 PST value (or ambiguous).

@coveralls
Copy link

coveralls commented Sep 2, 2018

Pull Request Test Coverage Report for Build 73

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 94.915%

Totals Coverage Status
Change from base Build 71: 0.0%
Covered Lines: 280
Relevant Lines: 295

💛 - Coveralls

@bgentry bgentry mentioned this pull request Sep 2, 2018
2 tasks
@pbogut pbogut merged commit 82bf612 into pbogut:feature/timezone-support Sep 3, 2018
@pbogut
Copy link
Owner

pbogut commented Sep 3, 2018

Thank you very much.

@bgentry bgentry deleted the feature/timezone-support-test-cases branch September 3, 2018 16:45
@voughtdq
Copy link
Contributor

So the TimezoneInfo struct that Timex makes might be helpful. It gives date info like this:

iex(67)> Timex.Timezone.get("America/New_York", DateTime.utc_now) |> Map.from_struct()   
%{
  abbreviation: "EST",
  from: {:sunday, {{2018, 11, 4}, {1, 0, 0}}},
  full_name: "America/New_York",
  offset_std: 0,
  offset_utc: -18000,
  until: {:sunday, {{2019, 3, 10}, {2, 0, 0}}}
}

So I guess we can extrapolate that any time between :until and :until + 1 should be time + 1?

For the ambiguous case, I'll have to do a little more research on what the best practice is.

The dateutil.rrule tests for the Python dateutil.rrule class has extensive tests, so maybe we can borrow some of those (with attribution of course :)

https://www.timeanddate.com/time/change/usa/new-york?year=2019

And yes, I did have to look at this visual to understand what's going on. 😂

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

Successfully merging this pull request may close these issues.

4 participants