Skip to content

Enable/add some datetime tests unrelated to Y2K38#24442

Closed
lanurmi wants to merge 3 commits intowxWidgets:masterfrom
lanurmi:not-y2k38
Closed

Enable/add some datetime tests unrelated to Y2K38#24442
lanurmi wants to merge 3 commits intowxWidgets:masterfrom
lanurmi:not-y2k38

Conversation

@lanurmi
Copy link
Contributor

@lanurmi lanurmi commented Mar 28, 2024

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 the TZ environment variable, also affect this.

For example (on Linux):
./test DateTimeTestCase -> fail
TZ=CET ./test DateTimeTestCase -> fail
TZ=CEST ./test DateTimeTestCase -> no error

As 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.

lanurmi added 3 commits March 25, 2024 10:48
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.
@lanurmi
Copy link
Contributor Author

lanurmi commented Mar 28, 2024

And the tests did not even fail here with CI, although they do fail for me locally. 🤷‍♂️
(Windows 10, Fedora Rawhide; system timezone EET)

@vadz
Copy link
Contributor

vadz commented Apr 7, 2024

Sorry, I'm not sure if this one is supposed to be merged or if it's a draft/WIP?

@lanurmi
Copy link
Contributor Author

lanurmi commented Apr 7, 2024

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.

@vadz
Copy link
Contributor

vadz commented Apr 8, 2024

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.

@vadz vadz added the work needed Too useful to close, but can't be applied in current state label Apr 8, 2024
@lanurmi
Copy link
Contributor Author

lanurmi commented Apr 8, 2024

Makes sense. 5ad2ca1 alone should be safe to cherry-pick, though.

@vadz
Copy link
Contributor

vadz commented Apr 8, 2024

I had a brief look at this and the problem is that the date gets formatted as 1931-07-29 15:28:00 (note the century) and MakeFromTimezone() doesn't apply DST to it.

IOW I think things would work if we used a format with 4 digits year instead of %x.

@vadz
Copy link
Contributor

vadz commented May 10, 2025

@lanurmi Do you still plan to update this?

@lanurmi
Copy link
Contributor Author

lanurmi commented May 10, 2025

@lanurmi Do you still plan to update this?

Probably not, honestly. But as mentioned earlier, 5ad2ca1 on its own is should be harmless to cherry-pick.

vadz pushed a commit to vadz/wxWidgets that referenced this pull request May 10, 2025
TestTimeTicks() checks were skipped for test dates with gmticks == -1,
ensure that these tests really run by providing the expected ticks
values.

See wxWidgets#24442.
@vadz
Copy link
Contributor

vadz commented May 10, 2025

OK, thanks, I've added it to #25386 and will close this one then.

@vadz vadz closed this May 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

work needed Too useful to close, but can't be applied in current state

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants