-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Enable/add some datetime tests unrelated to Y2K38 #24442
base: master
Are you sure you want to change the base?
Conversation
TestTimeTicks() checks were skipped for testDates with gmticks == -1, i.e. nearly all of them. (What is the point of the test if it is mostly skipped?)
Dates are, however, well below Y2K38.
And the tests did not even fail here with CI, although they do fail for me locally. 🤷♂️ |
Sorry, I'm not sure if this one is supposed to be merged or if it's a draft/WIP? |
The tests that were added are simple and completely valid, and tests passed in CI, so from that perspective it should be merged. But as mentioned, tests with the new date in 2031 fail for me locally (and I don't know why), meaning they could fail for others, too. Then again, if there is an issue, it is there whether or not this is merged, but without merging the issue is untested, undocumented, and hidden. |
FWIW I can confirm that this fails for me too locally, so I'm not going to merge it yet, as I want the tests to pass when I run them. Might be related to #15370. |
Makes sense. 5ad2ca1 alone should be safe to cherry-pick, though. |
I had a brief look at this and the problem is that the date gets formatted as IOW I think things would work if we used a format with 4 digits year instead of |
TestTimeTicks()
was actually run for very few values; enabled some more of those in the array.In
TestTimeFormat()
I added some new dates, and found out that any date within a period of DST will fail the test by 1 hour, but only between the years [2031, 2037], and only on MSW and Linux, but not macOS. The timezone of the running environment, and theTZ
environment variable, also affect this.For example (on Linux):
./test DateTimeTestCase
-> failTZ=CET ./test DateTimeTestCase
-> failTZ=CEST ./test DateTimeTestCase
-> no errorAs I don't yet know what the reason is, or why 2030/2031 are different, this PR serves both as a PR and an issue with a minimal reproducible example of a problem.