-
-
Notifications
You must be signed in to change notification settings - Fork 30.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
gh-53203: Fix test_strptime on Solaris #125785
Conversation
serhiy-storchaka
commented
Oct 21, 2024
•
edited by bedevere-app
bot
Loading
edited by bedevere-app
bot
- Issue: strptime(.., '%c') fails to parse output of strftime('%c', ..) in some locales #53203
@kulikjak, please check if these tests are passed on Solaris. |
Lib/test/test_strptime.py
Outdated
@@ -521,8 +521,17 @@ def test_date_time_locale(self): | |||
'my_MM', 'shn_MM') | |||
def test_date_time_locale2(self): | |||
# Test %c directive | |||
loc = locale.getlocale(locale.LC_TIME)[0] | |||
if sys.platform.startswith(('sunos', 'solaris')): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be safely just 'sunos'. AFAIK, neither Oracle nor OpenSolaris forks will report 'solaris' here.
Lib/test/test_strptime.py
Outdated
self.roundtrip('%c', slice(0, 6), (1800, 1, 1, 0, 0, 0, 0, 1, 0)) | ||
except ValueError: | ||
if 'LMT' in time.strftime('%c', (1800, 1, 1, 0, 0, 0, 0, 1, 0)): | ||
self.skipTest('different timezone in the past is not supported') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When implemented this way, all the tests look like skipped (because there is always a 'LMT' zone when running the test for 1800), even though the 1900 round trip already passed. But I don't really mind that - I just noticed it :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because you run tests in a place where LMT was in 1800.
Please check the tests again, now with a fixed timezone.
Lib/test/test_strptime.py
Outdated
@@ -553,6 +562,10 @@ def test_date_locale(self): | |||
'eu_ES', 'ar_AE', 'my_MM', 'shn_MM') | |||
def test_date_locale2(self): | |||
# Test %x directive | |||
loc = locale.getlocale(locale.LC_TIME)[0] | |||
if sys.platform.startswith(('sunos', 'solaris')): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
Thank you. I tested the patch and it works. (I was a little surprised that all the skips matched what I found.) |
I looked in the illumos locale data. |
Now it works. I'd suggest adding a comment or updating the skip message with the reason (the year has only two digits), but that is just a detail. Thank you for all the fixes!
Ah, makes sense. |
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
Use fixed timezone. Skip roundtrip tests on locales with 2-digit year. (cherry picked from commit 9dde463) Co-authored-by: Serhiy Storchaka <[email protected]>
Use fixed timezone. Skip roundtrip tests on locales with 2-digit year. (cherry picked from commit 9dde463) Co-authored-by: Serhiy Storchaka <[email protected]>
GH-125806 is a backport of this pull request to the 3.13 branch. |
GH-125807 is a backport of this pull request to the 3.12 branch. |
Use fixed timezone. Skip roundtrip tests on locales with 2-digit year. (cherry picked from commit 9dde463) Co-authored-by: Serhiy Storchaka <[email protected]>
Use fixed timezone. Skip roundtrip tests on locales with 2-digit year. (cherry picked from commit 9dde463) Co-authored-by: Serhiy Storchaka <[email protected]>