Skip to content

fix(datetime): subtract 1 from ACARS month before setUTCMonth (#424)#434

Open
SAY-5 wants to merge 3 commits into
airframesio:masterfrom
SAY-5:fix/issue-424-utc-month-off-by-one
Open

fix(datetime): subtract 1 from ACARS month before setUTCMonth (#424)#434
SAY-5 wants to merge 3 commits into
airframesio:masterfrom
SAY-5:fix/issue-424-utc-month-off-by-one

Conversation

@SAY-5
Copy link
Copy Markdown

@SAY-5 SAY-5 commented Apr 30, 2026

Summary

Fixes #424. UTCDateTimeToString passed the raw ACARS month digit (1-12) to Date.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 incremental setUTC* sequence is fragile when the system clock falls on a day that doesn't exist in the target month: setting day=28 on a Date already at the 31st silently rolls forward, so encoding 280226 while the host clock said 31 March would have decoded to 28 March.

Test plan

  • npx jest, full suite: 410 passed, 9 skipped, 89 suites
  • New DateTimeUtils.test.ts:
    • decodes a DDMMYY date with the ACARS 1-12 month convention, direct regression for Bug: DateTimeUtils.UTCDateTimeToString month is off-by-one #424
    • does not roll the month forward when the system date is later than the target month-end, uses jest.setSystemTime to pin a 31-day-month system clock and verifies the order-fragility fix
    • handles a six-digit time with seconds, exercises the HHMMSS branch

Summary by CodeRabbit

  • Bug Fixes

    • Fixed UTC date/time parsing and formatting so displayed dates are correct across month boundaries, with proper month indexing and reliable year fallback.
    • Ensures seconds are handled consistently in formatted times.
  • Tests

    • Added deterministic tests for date/time utilities covering month-end cases and time strings (including seconds).

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fc7fab6b-a2f2-421c-b896-2c3571d94f96

📥 Commits

Reviewing files that changed from the base of the PR and between 11211f7 and 374cc5e.

📒 Files selected for processing (1)
  • lib/DateTimeUtils.test.ts

Walkthrough

Builds 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.

Changes

UTC date parsing and tests

Layer / File(s) Summary
UTC construction and parsing
lib/DateTimeUtils.ts
Constructs UTC Date with Date.UTC(...), subtracts 1 from parsed month, uses current UTC year when dateString isn't length 6, and parses seconds only when timeString.length === 6.
Deterministic Jest tests
lib/DateTimeUtils.test.ts
Adds tests: decode DDMMYY with 1–12 month convention (asserts month/day/year), regression asserting 280226 yields 28 Feb 2026 using a Date.UTC-computed expected string, and a test verifying HHMMSS produces HH:MM:SS in output.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • kevinelliott

Poem

🐰 I hopped through months with careful cheer,
Subtracted one so dates are clear,
One Date.UTC to bind the time,
Tests that pin the days in line,
February returns—hoppy and near.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the primary fix: subtracting 1 from ACARS month before setUTCMonth to correct the off-by-one bug.
Linked Issues check ✅ Passed The PR fully addresses issue #424 by subtracting 1 from ACARS month and refactoring to use Date.UTC for robust single-operation date construction.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the off-by-one month bug and date construction robustness; no unrelated alterations detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread lib/DateTimeUtils.test.ts Outdated
// (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)));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why we're setting system time. Can you explain?

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 incremental setUTC* 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.

@kevinelliott
Copy link
Copy Markdown
Contributor

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>
@SAY-5
Copy link
Copy Markdown
Author

SAY-5 commented May 28, 2026

@makrsmark good point - the implementation builds the date in a single new Date(Date.UTC(...)) call so system time has no effect on the output; the mock was a leftover. Dropped the fake timer in 374cc5e; the test now asserts the returned UTC string directly against new Date(Date.UTC(2026, 1, 28, 12, 30, 0)).toUTCString(). All 410 tests still pass.

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.

Bug: DateTimeUtils.UTCDateTimeToString month is off-by-one

4 participants