Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SetAndReadDateTimeLocal_DateRange test fails due to DST changes in America/Sao_Paulo timezone #82

Open
m0nzderr opened this issue Sep 24, 2019 · 1 comment

Comments

@m0nzderr
Copy link

Below is what happens on my PT-BR system. Compiled against v8-v142 (7.5.288.23), target net45.

Since the test case runs through the all dates at 0:00, it receives back 1:00 AM when it hits
DST shift time in Brazil (and other local time adjustments like the one in 1914). This is the expected behavior for V8, since such combinations of date/time does not exist in America/Sao_Paulo timezone.

Message: 
    Assert.Fail failed. Expected 01/01/1914 00:00:00, but got 01/01/1914 00:06:28
    Expected 03/10/1932 00:00:00, but got 03/10/1932 01:00:00
    Expected 01/12/1949 00:00:00, but got 01/12/1949 01:00:00
    Expected 01/12/1950 00:00:00, but got 01/12/1950 01:00:00
    Expected 01/12/1951 00:00:00, but got 01/12/1951 01:00:00
    ....
    Expected 16/10/2016 00:00:00, but got 16/10/2016 01:00:00
    Expected 15/10/2017 00:00:00, but got 15/10/2017 01:00:00
    Expected 04/11/2018 00:00:00, but got 04/11/2018 01:00:00
    Expected 03/11/2019 00:00:00, but got 03/11/2019 01:00:00
    ....
    Expected 03/11/2097 00:00:00, but got 03/11/2097 01:00:00
    Expected 02/11/2098 00:00:00, but got 02/11/2098 01:00:00
    Expected 01/11/2099 00:00:00, but got 01/11/2099 01:00:00

I added val.toString() and val.getTime() to the test output, so I could verify that V8 Date objects are correct. Here is what I got back:

....
    Expected 16/10/2016 00:00:00, but got 16/10/2016 01:00:00 
    str: Sun Oct 16 2016 01:00:00 GMT-0200 (Horário de Verão de Brasília), 
    timestamp: 1476586800000

    Expected 15/10/2017 00:00:00, but got 15/10/2017 01:00:00 
    str: Sun Oct 15 2017 01:00:00 GMT-0200 (Horário de Verão de Brasília), 
    timestamp: 1508036400000
....

It is exactly the same behavior as I get in chrome console for that dates:

>new Date(2016,9,16)
<Sun Oct 16 2016 01:00:00 GMT-0200 (Horário de Verão de Brasília)
>1476586800000

So the problem is with the test itself, as long as you are only dealing with local (timezone-unaware) DateTime objects. However, it will be a different story for timezone-aware DateTime objects (those with timezone information).

In order to have stable behavior across different locales, I recommend to avoid component-based Date conversion. Instead, it would be better to always have any .NET DateTime object first converted to UTC timestmp (locaized or not), an then use this timestamp to construct V8 Date constructor (it could be int64 value or ISOString).

@spahnke
Copy link
Collaborator

spahnke commented Sep 25, 2019

The reason why we do the conversion in a component wise fashion is because V8 does use the IANA timezone database and C# does not. Leading to inherent discrepancies explained in #75 and #76 .

In order to have stable behavior across different locales, I recommend to avoid component-based Date conversion. Instead, it would be better to always have any .NET DateTime object first converted to UTC timestmp (locaized or not), an then use this timestamp to construct V8 Date constructor (it could be int64 value or ISOString).

I think that's basically what we tried with #55 IIRC which had the nasty bugs explained in the other two PRs.

The brute-force test was more or less an assurance for me while writing the conversion, that I don't miss any more edge cases where C# and V8 disagree. So it's not the best test anyway, especially since you can't fix a timezone while running a test. (See #76 for more timezones where the brute-force test was known to fail)

I do not pretend to understand this topic completely in all its details but this was the closest we could get. If you find a better solution where the existing tests still pass you're welcome to create a PR 🙇

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

No branches or pull requests

2 participants