period.AddTo revised to reduce the impact of subtle behaviours of tim…

…e.AddDate
rickb777 · Sep 19, 2023 · b6690e4 · b6690e4
1 parent ad3aa70
commit b6690e4
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 39 deletions.
diff --git a/period/arithmetic.go b/period/arithmetic.go
@@ -39,8 +39,13 @@ func (period Period) AddTo(t time.Time) (time.Time, bool) {
 	wholeDays := (period.days % 10) == 0
 
 	if wholeYears && wholeMonths && wholeDays {
-		// in this case, time.AddDate provides an exact solution
+		// in this case, time.AddDate(...).Add(...) provides an exact solution
 		stE3 := totalSecondsE3(period)
+		if period.years == 0 && period.months == 0 && period.days == 0 {
+			// AddDate (below) normalises its result, so we don't call it unless needed
+			return t.Add(stE3 * time.Millisecond), true
+		}
+
 		t1 := t.AddDate(int(period.years/10), int(period.months/10), int(period.days/10))
 		return t1.Add(stE3 * time.Millisecond), true
 	}

diff --git a/period/arithmetic_test.go b/period/arithmetic_test.go
@@ -89,45 +89,56 @@ func TestPeriodAddToTime(t *testing.T) {
 	const min = 60 * sec
 	const hr = 60 * min
 
-	// A conveniently round number (14 July 2017 @ 2:40am UTC)
-	var t0 = time.Unix(1500000000, 0).UTC()
-
-	cases := []struct {
-		value   string
-		result  time.Time
-		precise bool
-	}{
-		// precise cases
-		{"P0D", t0, true},
-		{"PT1S", t0.Add(sec), true},
-		{"PT0.1S", t0.Add(100 * ms), true},
-		{"-PT0.1S", t0.Add(-100 * ms), true},
-		{"PT3276S", t0.Add(3276 * sec), true},
-		{"PT1M", t0.Add(60 * sec), true},
-		{"PT0.1M", t0.Add(6 * sec), true},
-		{"PT3276M", t0.Add(3276 * min), true},
-		{"PT1H", t0.Add(hr), true},
-		{"PT0.1H", t0.Add(6 * min), true},
-		{"PT3276H", t0.Add(3276 * hr), true},
-		{"P1D", t0.AddDate(0, 0, 1), true},
-		{"P3276D", t0.AddDate(0, 0, 3276), true},
-		{"P1M", t0.AddDate(0, 1, 0), true},
-		{"P3276M", t0.AddDate(0, 3276, 0), true},
-		{"P1Y", t0.AddDate(1, 0, 0), true},
-		{"-P1Y", t0.AddDate(-1, 0, 0), true},
-		{"P3276Y", t0.AddDate(3276, 0, 0), true},   // near the upper limit of range
-		{"-P3276Y", t0.AddDate(-3276, 0, 0), true}, // near the lower limit of range
-		// approximate cases
-		{"P0.1D", t0.Add(144 * min), false},
-		{"-P0.1D", t0.Add(-144 * min), false},
-		{"P0.1M", t0.Add(oneMonthApprox / 10), false},
-		{"P0.1Y", t0.Add(oneYearApprox / 10), false},
+	est, err := time.LoadLocation("America/New_York")
+	g.Expect(err).NotTo(HaveOccurred())
+
+	times := []time.Time{
+		// A conveniently round number but with non-zero nanoseconds (14 July 2017 @ 2:40am UTC)
+		time.Unix(1500000000, 1).UTC(),
+		// This specific time fails for EST due behaviour of Time.AddDate
+		time.Date(2020, 11, 1, 1, 0, 0, 0, est),
 	}
-	for i, c := range cases {
-		p := MustParse(c.value)
-		t1, prec := p.AddTo(t0)
-		g.Expect(t1).To(Equal(c.result), info(i, c.value))
-		g.Expect(prec).To(Equal(c.precise), info(i, c.value))
+
+	for _, t0 := range times {
+		cases := []struct {
+			value   string
+			result  time.Time
+			precise bool
+		}{
+			// precise cases
+			{"P0D", t0, true},
+			{"PT1S", t0.Add(sec), true},
+			{"PT0.1S", t0.Add(100 * ms), true},
+			{"-PT0.1S", t0.Add(-100 * ms), true},
+			{"PT3276S", t0.Add(3276 * sec), true},
+			{"PT1M", t0.Add(60 * sec), true},
+			{"PT0.1M", t0.Add(6 * sec), true},
+			{"PT3276M", t0.Add(3276 * min), true},
+			{"PT1H", t0.Add(hr), true},
+			{"PT0.1H", t0.Add(6 * min), true},
+			{"PT3276H", t0.Add(3276 * hr), true},
+			{"P1D", t0.AddDate(0, 0, 1), true},
+			{"P3276D", t0.AddDate(0, 0, 3276), true},
+			{"P1M", t0.AddDate(0, 1, 0), true},
+			{"P3276M", t0.AddDate(0, 3276, 0), true},
+			{"P1Y", t0.AddDate(1, 0, 0), true},
+			{"-P1Y", t0.AddDate(-1, 0, 0), true},
+			{"P3276Y", t0.AddDate(3276, 0, 0), true},   // near the upper limit of range
+			{"-P3276Y", t0.AddDate(-3276, 0, 0), true}, // near the lower limit of range
+			// approximate cases
+			{"P0.1D", t0.Add(144 * min), false},
+			{"-P0.1D", t0.Add(-144 * min), false},
+			{"P0.1M", t0.Add(oneMonthApprox / 10), false},
+			{"P0.1Y", t0.Add(oneYearApprox / 10), false},
+		}
+		for i, c := range cases {
+			p, err := ParseWithNormalise(c.value, false)
+			g.Expect(err).NotTo(HaveOccurred())
+
+			t1, prec := p.AddTo(t0)
+			g.Expect(t1).To(Equal(c.result), info(i, c.value, t0))
+			g.Expect(prec).To(Equal(c.precise), info(i, c.value, t0))
+		}
 	}
 }