wanasit / chrono

A natural language date parser in Javascript
MIT License
4.61k stars 339 forks source link

Tests fail when DST switch interferes #439

Closed georgd closed 2 years ago

georgd commented 2 years ago

This was originally reported in https://github.com/wanasit/chrono/pull/434 and partially fixed with https://github.com/wanasit/chrono/commit/e3b76090e75a95e92bc821ec72ddf367db66b8ac.

Some tests fail when the system is located in a timezone where a daylight saving time switch occurs between a reference or source date and the target date. The target date will be an hour off of what is expected. This is mathematically correct but breaks the tests. In most cases, it’s just the hour that’s off (and the timeZoneOffset) but in some cases it’s even the day (when the source time is midnight, like in https://github.com/wanasit/chrono/blob/61ae8a34ba1beb736d86f11d86a074f7568ba239/test/en/en_time_units_ago.test.ts#L174).

Is it safe to manipulate the tests to make them pass?

Here’s the list of the failing tests:

 Summary of all failing tests
 FAIL  test/en/en_time_units_within.test.ts (20.31 s)
  ● Test - The normal within expression

    Expected date to be: Sat Nov 10 2012 12:14:00 GMT+0100 (Mitteleuropäische Normalzeit) Received: Sat Nov 10 2012 11:14:00 GMT+0100 (Mitteleuropäische Normalzeit) ([ParsingComponents {knownValues: {"month":11,"year":2012}, impliedValues: {"day":10,"hour":12,"minute":14,"second":0,"millisecond":0,"timezoneOffset":120}}, reference: {"instant":"2012-08-10T10:14:00.000Z"}])

      106 |         expect(result.text).toBe("within a few months");
      107 |
    > 108 |         expect(result.start).toBeDate(new Date(2012, 10, 10, 12, 14));
          |                              ^
      109 |     });
      110 |
      111 |     testSingleCase(chrono, "within one year", new Date(2012, 7, 10, 12, 14), (result) => {

      at checkResult (test/en/en_time_units_within.test.ts:108:30)
      at Object.<anonymous> (test/en/en_time_units_within.test.ts:104:5)

 FAIL  test/en/en_relative.test.ts
  ● Test - Relative date components' certainty

    expect(received).toBe(expected) // Object.is equality

    Expected: 60
    Received: 120

      184 |         expect(result.start.get("day")).toBe(7);
      185 |         expect(result.start.get("hour")).toBe(12);
    > 186 |         expect(result.start.get("timezoneOffset")).toBe(-expectedDate.getTimezoneOffset());
          |                                                    ^
      187 |
      188 |         expect(result.start.isCertain("year")).toBe(true);
      189 |         expect(result.start.isCertain("month")).toBe(true);

      at checkResult (test/en/en_relative.test.ts:186:52)
      at Object.<anonymous> (test/en/en_relative.test.ts:178:5)

 FAIL  test/en/en_time_units_ago.test.ts
  ● Test - Single Expression (Casual)

    Expected date to be: Sat Mar 10 2012 00:00:00 GMT+0100 (Mitteleuropäische Normalzeit) Received: Fri Mar 09 2012 23:00:00 GMT+0100 (Mitteleuropäische Normalzeit) ([ParsingComponents {knownValues: {"month":3,"year":2012}, impliedValues: {"day":10,"hour":0,"minute":0,"second":0,"millisecond":0,"timezoneOffset":120}}, reference: {"instant":"2012-08-09T22:00:00.000Z"}])

      181 |         expect(result.text).toBe("5 months ago");
      182 |
    > 183 |         expect(result.start).toBeDate(new Date(2012, 3 - 1, 10));
          |                              ^
      184 |     });
      185 |
      186 |     testSingleCase(chrono, "5 years ago, we did something", new Date(2012, 8 - 1, 10), (result) => {

      at checkResult (test/en/en_time_units_ago.test.ts:183:30)
      at Object.<anonymous> (test/en/en_time_units_ago.test.ts:174:5)

 FAIL  test/nl/nl_time_units_ago.test.ts
  ● Test - Single Expression (Casual)

    Expected date to be: Sat Mar 10 2012 00:00:00 GMT+0100 (Mitteleuropäische Normalzeit) Received: Fri Mar 09 2012 23:00:00 GMT+0100 (Mitteleuropäische Normalzeit) ([ParsingComponents {knownValues: {"month":3,"year":2012}, impliedValues: {"day":10,"hour":0,"minute":0,"second":0,"millisecond":0,"timezoneOffset":120}}, reference: {"instant":"2012-08-09T22:00:00.000Z"}])

      167 |         expect(result.text).toBe("5 maanden geleden");
      168 |
    > 169 |         expect(result.start).toBeDate(new Date(2012, 3 - 1, 10));
          |                              ^
      170 |     });
      171 |
      172 |     // 5 years ago, we did something

      at checkResult (test/nl/nl_time_units_ago.test.ts:169:30)
      at Object.<anonymous> (test/nl/nl_time_units_ago.test.ts:160:5)

 FAIL  test/nl/nl_relative.test.ts
  ● Test - Relative date components' certainty

    expect(received).toBe(expected) // Object.is equality

    Expected: 60
    Received: 120

      162 |         expect(result.start.get("day")).toBe(7);
      163 |         expect(result.start.get("hour")).toBe(12);
    > 164 |         expect(result.start.get("timezoneOffset")).toBe(-expectedDate.getTimezoneOffset());
          |                                                    ^
      165 |
      166 |         expect(result.start.isCertain("year")).toBe(true);
      167 |         expect(result.start.isCertain("month")).toBe(true);

      at checkResult (test/nl/nl_relative.test.ts:164:52)
      at Object.<anonymous> (test/nl/nl_relative.test.ts:156:5)

 FAIL  test/fr/fr_time_units_within.test.ts
  ● Test - Single Expression

    Expected date to be: Sat Nov 10 2012 22:14:00 GMT+0100 (Mitteleuropäische Normalzeit) Received: Sat Nov 10 2012 21:14:00 GMT+0100 (Mitteleuropäische Normalzeit) ([ParsingComponents {knownValues: {"month":11,"year":2012}, impliedValues: {"day":10,"hour":22,"minute":14,"second":0,"millisecond":0,"timezoneOffset":120}}, reference: {"instant":"2012-08-10T20:14:00.000Z"}])

      85 |         expect(result.text).toBe("dans quelques mois");
      86 |
    > 87 |         expect(result.start).toBeDate(new Date(2012, 10, 10, 22, 14));
         |                              ^
      88 |     });
      89 |
      90 |     testSingleCase(chrono.fr, "en une année", new Date(2012, 7, 10, 12, 14), (result) => {

      at checkResult (test/fr/fr_time_units_within.test.ts:87:30)
      at Object.<anonymous> (test/fr/fr_time_units_within.test.ts:83:5)

 FAIL  test/fr/fr_time_units_ago.test.ts
  ● Test - Single Expression (Casual)

    Expected date to be: Sat Mar 10 2012 00:00:00 GMT+0100 (Mitteleuropäische Normalzeit) Received: Fri Mar 09 2012 23:00:00 GMT+0100 (Mitteleuropäische Normalzeit) ([ParsingComponents {knownValues: {"month":3,"year":2012}, impliedValues: {"day":10,"hour":0,"minute":0,"second":0,"millisecond":0,"timezoneOffset":120}}, reference: {"instant":"2012-08-09T22:00:00.000Z"}])

      70 |         expect(result.text).toBe("il y a 5 mois");
      71 |
    > 72 |         expect(result.start).toBeDate(new Date(2012, 3 - 1, 10));
         |                              ^
      73 |     });
      74 |
      75 |     testSingleCase(chrono.fr, "il y a 5 ans, on a fait quelque chose", new Date(2012, 8 - 1, 10, 22, 22), (result) => {

      at checkResult (test/fr/fr_time_units_ago.test.ts:72:30)
      at Object.<anonymous> (test/fr/fr_time_units_ago.test.ts:63:5)

Test Suites: 7 failed, 57 passed, 64 total
Tests:       7 failed, 256 passed, 263 total
Snapshots:   0 total
Time:        40.023 s
Ran all test suites.
wanasit commented 2 years ago

Yes. I'm afraid I haven't found a way to resolve this correctly yet.

Is it safe to manipulate the tests to make them pass?

Yes. it's difficult for me to test it on my timezone. Please feel free to submit MR to make the test account for the error (moving the case to different month or comment out but document the behavior is also an option)

Nantris commented 1 year ago

@wanasit is there anything that can be done to fix this? I see it's closed but it doesn't seem to be resolved - is there some commit which was believed to resolve this? If so, it doesn't seem to fully fix it.

It worked entirely correctly in 1.4.4 - but the 2.x versions seem to have this problem.

Even if we set result.start.assign('hour', 12) - the .date() method returns 1pm instead of 12pm.

Specifically we can instigate this with strings like in 1 month (with the current date being Feb. 18 and us being in US Eastern time)