wanasit / chrono

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

Four failing tests due to DST #470

Closed Cito closed 1 year ago

Cito commented 2 years ago

When I run the tests on October 1st 2022 in Germany (CEST time zone), the following four tests are failing.

As far as I see, these tests somehow cannot cope with the fact taht they run in DST (CEST = Central European Summer Time).

Tested with the master branch as of October 1st 2022 (version 2.4.1).

Suggestion: It would be good if there were was a GitHub action running the tests in different time zones.

See also #439.

test/system.test.ts
  ● Test - Compare with native js

    Expected date to be: Mon Jan 01 1900 02:00:00 GMT+0100 (Mitteleuropäische Normalzeit) Received: Mon Jan 01 1900 03:00:00 GMT+0100 (Mitteleuropäische Normalzeit) ([ParsingResult {index: 0, text: '1900-01-01T00:00:00-01:00', ...}])

      138 |         testSingleCase(chrono, text, (result) => {
      139 |             expect(result.text).toBe(text);
    > 140 |             expect(result).toBeDate(expectedDate);
          |                            ^
      141 |         });
      142 |     };
      143 | 

      at checkResult (test/system.test.ts:140:28)
      at testByCompareWithNative (test/system.test.ts:138:9)
      at Object.<anonymous> (test/system.test.ts:158:5)

test/en/en.test.ts
  ● Test - Random text

    Got 0 results from '03-27-2022, 02:00 AM'

       98 |     });
       99 | 
    > 100 |     testSingleCase(chrono, "03-27-2022, 02:00 AM", new Date(2017, 7 - 1, 7), (result) => {
          |     ^
      101 |         console.log("RAND2", result);
      102 |         expect(result.text).toBe("03-27-2022, 02:00 AM");
      103 |         expect(result.start.get("day")).toBe(27);

      at Object.<anonymous> (test/en/en.test.ts:100:5)

 test/ru/ru_relative.test.ts
  ● Test - Future relative expressions

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

      58 |         expect(result.index).toBe(0);
      59 |         expect(result.text).toBe(text);
    > 60 |         expect(result.start).toBeDate(new Date(2016, 11 - 1, 1, 12));
         |                              ^
      61 |     });
      62 | 
      63 |     testSingleCase(chrono.ru, "в следующем квартале", new Date(2016, 10 - 1, 1, 12), (result, text) => {

      at checkResult (test/ru/ru_relative.test.ts:60:30)
      at Object.<anonymous> (test/ru/ru_relative.test.ts:57:5)

test/ru/ru_time_units_casual_relative.test.ts
  ● Test - Positive time units

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

      48 |         expect(result.index).toBe(0);
      49 |         expect(result.text).toBe(text);
    > 50 |         expect(result.start).toBeDate(new Date(2017, 1 - 1, 1, 12));
         |                              ^
      51 |     });
      52 | 
      53 |     testSingleCase(chrono.ru, "через неделю", new Date(2016, 10 - 1, 1, 12), (result, text) => {

      at checkResult (test/ru/ru_time_units_casual_relative.test.ts:50:30)
      at Object.<anonymous> (test/ru/ru_time_units_casual_relative.test.ts:47:5)
Prid13 commented 1 year ago

Can confirm. Was trying to parse "Sept. 14 at 7 AM PT" and got back "5 PM" local time when in fact it should've been "4 PM". Missed an event due to this, so I had to learn it the hard way 😅

Nantris commented 1 year ago

I seem to be having the same problem. Two of our tests are off by an hour. I can confirm that this was the problem come tomorrow.

Nantris commented 1 year ago

I see now that the ones I mocked time for failed earlier (2 of them) and now with one hour to go all of my tests that have a tolerance of less than 1 hour off are failing.

I expect to be able to confirm this when they all pass again tomorrow.

Nantris commented 1 year ago

Indeed all tests pass again.

@wanasit any thoughts about a possible fix?

wanasit commented 1 year ago

Hello @Slapbox @Prid13 @Cito. Thanks for reporting this. Sorry, I haven't been working on the project recently.

I have fixed those tests in 5ebfcc0e7f957260ca97f8a1d3056c7869b04844 (checking parsing result instead of JS date).

For now, I tested the change by running NodeJS with TZ flag. For example:

TZ=CET npm test

I will think of a way to make automated tests better.

Nantris commented 1 year ago

Thanks for your attention on this @wanasit. You do a great job maintaining this library and I really appreciate it!

It appears I misread the issue - it was my own tests failing in conjunction with chrono. If we wanted to implement the fix on our own tests - is the core of the fix these lines? Is there not actually an issue in the library itself, but only the tests?

Thanks again for the great work!

https://github.com/wanasit/chrono/blob/5ebfcc0e7f957260ca97f8a1d3056c7869b04844/test/result.test.ts#L152-L155