timeinterval: fix negative days_of_month across DST transitions - #5447
timeinterval: fix negative days_of_month across DST transitions#5447sueun-dev wants to merge 1 commit into
Conversation
…ions daysInMonth derived the month length by dividing the elapsed duration between the first of the month and the first of the next month by 24 hours. In a location whose month contains a spring-forward daylight saving transition that span is 743 wall-clock hours, not 744, so the truncating division returns 30 for a 31-day month. ContainsTime feeds that count into negative days_of_month resolution (for example -1 for the last day of the month), so with a non-UTC location set, a "last day" mute resolves to the 30th during a spring-forward month: it fails to fire on the true last day (31st) and fires a day early on the 30th. Compute the count with calendar arithmetic instead: day 0 of the following month normalizes to the last day of the current month, which is DST-independent. Add a regression case covering -1 in America/New_York across the March spring-forward transition. Signed-off-by: Sueun Cho <sueun.dev@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesCalendar Month Length
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This change corrects negative month-day resolution across spring-forward DST transitions and adds regression coverage; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 participants
daysInMonthcomputes a month's length by dividing the elapsed duration between the first of the month and the first of the next month by 24 hours. When alocationis set and that month contains a spring-forward DST transition, the span is 743 wall-clock hours rather than 744, so the truncating division returns 30 for a 31-day month.ContainsTimeuses that count to resolve negativedays_of_monthentries such as-1(the last day of the month). So a "last day of the month" mute with a non-UTC location resolves to the 30th during a spring-forward month: it does not fire on the actual last day (the 31st) and fires a day early on the 30th.Reproduced with
days_of_month: ['-1']andlocation: America/New_Yorkfor March 2021 (spring-forward on the 14th): before the changeContainsTime(Mar 31)is false andContainsTime(Mar 30)is true; after, it is the reverse. A non-DST month (May) is unaffected either way.The fix computes the count with calendar arithmetic — day 0 of the following month is the last day of the current month — which does not depend on DST. I added a regression case to
timeIntervalTestCasescovering-1inAmerica/New_Yorkacross the March transition; it fails before and passes after.Tested with
go test ./timeinterval/...plus the dependentnotify,config, anddispatchpackages.