fix(datetime): subtract 1 from ACARS month before setUTCMonth (#424)#434
fix(datetime): subtract 1 from ACARS month before setUTCMonth (#424)#434SAY-5 wants to merge 3 commits into
Conversation
…mesio#424) Signed-off-by: SAY-5 <say.apm35@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughBuilds UTC Date objects using Date.UTC(year, month-1, day, hour, minute, second) to fix an off-by-one month bug and adds deterministic Jest tests verifying DDMMYY parsing, a 28-Feb regression case, and HHMMSS (seconds) parsing. ChangesUTC date parsing and tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
| // (28 Feb 2026) used to roll forward to March because the order of | ||
| // setUTC* calls applied "set day=28" to a Date already at the 31st. | ||
| jest.useFakeTimers(); | ||
| jest.setSystemTime(new Date(Date.UTC(2026, 2, 31, 0, 0, 0))); |
There was a problem hiding this comment.
I don't understand why we're setting system time. Can you explain?
There was a problem hiding this comment.
Pull request overview
Fixes an off-by-one bug in DateTimeUtils.UTCDateTimeToString where the ACARS 1–12 month value was passed directly to setUTCMonth (which expects 0–11), shifting every decoded date one month forward. The fix also restructures the date construction into a single new Date(Date.UTC(...)) call to avoid order-dependent rollover when the host clock is on a day that doesn't exist in the target month.
Changes:
- Subtract 1 from the parsed month to align ACARS (1–12) with JS Date (0–11).
- Build the UTC date in one
Date.UTC(...)call instead of incrementalsetUTC*mutations. - Add unit tests covering the month convention, system-clock rollover edge case, and HHMMSS time parsing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/DateTimeUtils.ts | Fixes month off-by-one and refactors construction into a single Date.UTC call. |
| lib/DateTimeUtils.test.ts | Adds regression tests for the month fix, system-time rollover, and seconds handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks @SAY-5 for the contribution! It looks like @makrsmark has a question for you to address. |
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
|
@makrsmark good point - the implementation builds the date in a single |
Summary
Fixes #424.
UTCDateTimeToStringpassed the raw ACARS month digit (1-12) toDate.prototype.setUTCMonth, which expects 0-11. Every decoded date came out one month forward,150226(15 Feb 2026) decoded to 15 March 2026.While addressing the off-by-one I also folded the construction into a single
new Date(Date.UTC(...))call. The original incrementalsetUTC*sequence is fragile when the system clock falls on a day that doesn't exist in the target month: settingday=28on a Date already at the 31st silently rolls forward, so encoding280226while the host clock said 31 March would have decoded to 28 March.Test plan
npx jest, full suite: 410 passed, 9 skipped, 89 suitesDateTimeUtils.test.ts:decodes a DDMMYY date with the ACARS 1-12 month convention, direct regression for Bug: DateTimeUtils.UTCDateTimeToString month is off-by-one #424does not roll the month forward when the system date is later than the target month-end, usesjest.setSystemTimeto pin a 31-day-month system clock and verifies the order-fragility fixhandles a six-digit time with seconds, exercises theHHMMSSbranchSummary by CodeRabbit
Bug Fixes
Tests