search

moment / moment-timezone

Timezone support for moment.js
momentjs.com/timezone
MIT License
3.82k stars 835 forks source link

Some tz.guess() tests are failing in 2024 #1086

Closed gilmoreorless closed 7 months ago

gilmoreorless commented 8 months ago

Running npm test on the latest 0.5.44 commit in this repo worked fine in December 2023. Now in January 2024 several tests are failing on the .tz.guess() section.

Re-running grunt data-tests to regenerate the test files shows several changes, e.g.

-        "guess:by:offset" : helpers.makeTestGuess("Pacific/Wake", { offset: true, expect: "Asia/Kamchatka" }),
+        "guess:by:offset" : helpers.makeTestGuess("Pacific/Wake", { offset: true, expect: "Pacific/Fiji" }),

The expected value of the guess function shouldn't be changing that much just because the year changed. I need to investigate how to make this function (or these tests) more stable.

gilmoreorless commented 7 months ago

The tests changed because of the following sequence:

  1. When Intl is not available, tz.guess() falls back to checking a range of dates from <current year> - 2 to <current year> + 2. It looks for all zones that match the user's UTC offsets for that date, then chooses the zone with the largest listed population (to make the sorting stable).
  2. Fiji stopped having DST after 2021. This included it in the <current year> - 2 calculation until the end of 2023.
  3. Once the year ticked over to 2024, Fiji suddenly acted the same as a bunch of other Pacific zones without DST for the guess() function.
  4. Since Fiji has a larger population than the other zones, it became the new default guess value, which failed the existing tests.

Given the low frequency of major DST changes like this, and given it's only for the non-Intl fallback, I don't think it's worth trying to change this long term. I've updated the tests in 49fa3a919de8844bfa0a7a6b49e766cf4b646d56, and we'll just have to keep an eye on the guess tests at the start of each year.