Skip to content

Commit

Permalink
"Handle" Feb 29th rules more cleanly
Browse files Browse the repository at this point in the history
Previously, we were following zic: if a rule says "exactly Feb 29th"
or "at least Feb 29th" it was invalid in a non-leap year. This is
painful due to the way we ask rules for transitions; it's simpler to
ignore the rule, and just treat Feb 29th as Feb 28th in non-leap
years.

It shouldn't make any different to validly-defined rules in TZDB,
although it does make us more lenient with badly-defined fules.
Importantly, it allows us to handle some valid BCL rules.

Fixes #743.
  • Loading branch information
jskeet committed May 4, 2017
1 parent ac90d8a commit bec3276
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 7 deletions.
49 changes: 49 additions & 0 deletions src/NodaTime.Test/TimeZones/BclDateTimeZoneTest.cs
Expand Up @@ -142,6 +142,55 @@ public void DateTimeMinValueStartRuleExtendsToBeginningOfTime()
Assert.AreEqual(Offset.Zero, nodaZone.GetUtcOffset(Instant.FromUtc(-100, 10, 1, 0, 0)));
}

[Test]
public void AwkwardLeapYears()
{
// This mimics the data on Mono on Linux for Europe/Malta, where there's a BCL adjustment rule for
// each rule for quite a long time. One of those years is 1948, and the daylight transition is Feburary
// 29th. That then fails when we try to build a ZoneInterval at the end of that year.
// See https://github.com/nodatime/nodatime/issues/743 for more details. We've simplified this to just
// a single rule here...

// Amusingly, trying to reproduce the test on Mono with a custom time zone causes Mono to throw -
// quite possibly due to the same root cause that we're testing we've fixed in Noda Time.
// See https://bugzilla.xamarin.com/attachment.cgi?id=21192&action=edit
if (TestHelper.IsRunningOnMono)
{
Assert.Ignore("Test skipped on Mono");
}

var rules = new[]
{
TimeZoneInfo.AdjustmentRule.CreateAdjustmentRule(
dateStart: new DateTime(1948, 1, 1),
dateEnd: new DateTime(1949, 1, 1).AddDays(-1),
daylightDelta: TimeSpan.FromHours(1),
daylightTransitionStart: TimeZoneInfo.TransitionTime.CreateFixedDateRule(timeOfDay: new DateTime(1, 1, 1, 2, 0, 0), month: 2, day: 29),
daylightTransitionEnd: TimeZoneInfo.TransitionTime.CreateFixedDateRule(timeOfDay: new DateTime(1, 1, 1, 3, 0, 0), month: 10, day: 3))
};

var bclZone = TimeZoneInfo.CreateCustomTimeZone("Europe/Malta", TimeSpan.Zero, "Malta", "Standard", "Daylight", rules);
var nodaZone = BclDateTimeZone.FromTimeZoneInfo(bclZone);

var expectedTransition1 = Instant.FromUtc(1948, 2, 29, 2, 0, 0);
var expectedTransition2 = Instant.FromUtc(1948, 10, 3, 2, 0, 0); // 3am local time

var zoneIntervalBefore = nodaZone.GetZoneInterval(Instant.FromUtc(1947, 1, 1, 0, 0));
Assert.AreEqual(
new ZoneInterval("Standard", Instant.BeforeMinValue, expectedTransition1, Offset.Zero, Offset.Zero),
zoneIntervalBefore);

var daylightZoneInterval = nodaZone.GetZoneInterval(Instant.FromUtc(1948, 6, 1, 0, 0));
Assert.AreEqual(
new ZoneInterval("Daylight", expectedTransition1, expectedTransition2, Offset.FromHours(1), Offset.FromHours(1)),
daylightZoneInterval);

var zoneIntervalAfter = nodaZone.GetZoneInterval(Instant.FromUtc(1949, 1, 1, 0, 0));
Assert.AreEqual(
new ZoneInterval("Standard", expectedTransition2, Instant.AfterMaxValue, Offset.Zero, Offset.Zero),
zoneIntervalAfter);
}

private void ValidateZoneEquality(Instant instant, DateTimeZone nodaZone, TimeZoneInfo windowsZone)
{
// The BCL is basically broken (up to and including .NET 4.5.1 at least) around its interpretation
Expand Down
8 changes: 6 additions & 2 deletions src/NodaTime.Test/TimeZones/ZoneYearOffsetTest.cs
Expand Up @@ -166,7 +166,9 @@ public void GetOccurrenceForYear_ExactlyFeb29th_LeapYear()
public void GetOccurrenceForYear_ExactlyFeb29th_NotLeapYear()
{
ZoneYearOffset offset = new ZoneYearOffset(TransitionMode.Utc, 2, 29, 0, false, LocalTime.Midnight);
Assert.Throws<InvalidOperationException>(() => offset.GetOccurrenceForYear(2013));
var actual = offset.GetOccurrenceForYear(2013);
var expected = new LocalDateTime(2013, 2, 28, 0, 0).ToLocalInstant(); // For "exact", go to Feb 28th
Assert.AreEqual(expected, actual);
}

[Test]
Expand All @@ -182,7 +184,9 @@ public void GetOccurrenceForYear_AtLeastFeb29th_LeapYear()
public void GetOccurrenceForYear_AtLeastFeb29th_NotLeapYear()
{
ZoneYearOffset offset = new ZoneYearOffset(TransitionMode.Utc, 2, 29, (int) IsoDayOfWeek.Sunday, true, LocalTime.Midnight);
Assert.Throws<InvalidOperationException>(() => offset.GetOccurrenceForYear(2013));
var actual = offset.GetOccurrenceForYear(2013);
var expected = new LocalDateTime(2013, 3, 3, 0, 0).ToLocalInstant(); // March 3rd is the first Sunday after the non-existent 2013-02-29
Assert.AreEqual(expected, actual);
}

[Test]
Expand Down
10 changes: 5 additions & 5 deletions src/NodaTime/TimeZones/ZoneYearOffset.cs
Expand Up @@ -193,11 +193,11 @@ internal LocalInstant GetOccurrenceForYear(int year)
int actualDayOfMonth = dayOfMonth > 0 ? dayOfMonth : CalendarSystem.Iso.GetDaysInMonth(year, monthOfYear) + dayOfMonth + 1;
if (monthOfYear == 2 && dayOfMonth == 29 && !CalendarSystem.Iso.IsLeapYear(year))
{
// This mirrors zic.c. It's an odd rule, but...
if (dayOfWeek == 0 || AdvanceDayOfWeek)
{
throw new InvalidOperationException("Requested transition for a ZoneYearOffset of February 29th in a non-leap year, not moving backwards to find a day-of-week");
}
// In zic.c, this would result in an error if dayOfWeek is 0 or AdvanceDayOfWeek is true.
// However, it's very convenient to be able to ask any rule for its occurrence in any year.
// We rely on genuine rules being well-written - and before releasing an nzd file we always
// check that it's in line with zic anyway. Ignoring the brokenness is simpler than fixing
// rules that are only in force for a single year.
actualDayOfMonth = 28; // We'll now look backwards for the right day-of-week.
}
LocalDate date = new LocalDate(year, monthOfYear, actualDayOfMonth);
Expand Down

0 comments on commit bec3276

Please sign in to comment.