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

bpo-37552 strptime/strftime return invalid results with UCRT version 17763.615 #14460

Merged
merged 8 commits into from
Jul 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion Lib/test/support/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import glob
import importlib
import importlib.util
import locale
import logging.handlers
import nntplib
import os
Expand Down Expand Up @@ -92,7 +93,7 @@
"bigmemtest", "bigaddrspacetest", "cpython_only", "get_attribute",
"requires_IEEE_754", "skip_unless_xattr", "requires_zlib",
"anticipate_failure", "load_package_tests", "detect_api_mismatch",
"check__all__", "skip_unless_bind_unix_socket",
"check__all__", "skip_unless_bind_unix_socket", "skip_if_buggy_ucrt_strfptime",
"ignore_warnings",
# sys
"is_jython", "is_android", "check_impl_detail", "unix_shell",
Expand Down Expand Up @@ -2501,6 +2502,27 @@ def skip_unless_symlink(test):
msg = "Requires functional symlink implementation"
return test if ok else unittest.skip(msg)(test)

_buggy_ucrt = None
def skip_if_buggy_ucrt_strfptime(test):
"""
Skip decorator for tests that use buggy strptime/strftime

If the UCRT bugs are present time.localtime().tm_zone will be
an empty string, otherwise we assume the UCRT bugs are fixed

See bpo-37552 [Windows] strptime/strftime return invalid
results with UCRT version 17763.615
"""
global _buggy_ucrt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a particularly expensive check, right? Why cache it instead of checking each time?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The caching is simple and easy to follow; I'd rather keep it than assume it's not worthwhile :)

if _buggy_ucrt is None:
if(sys.platform == 'win32' and
locale.getdefaultlocale()[1] == 'cp65001' and
time.localtime().tm_zone == ''):
_buggy_ucrt = True
else:
_buggy_ucrt = False
return unittest.skip("buggy MSVC UCRT strptime/strftime")(test) if _buggy_ucrt else test

class PythonSymlink:
"""Creates a symlink for the current Python executable"""
def __init__(self, link=None):
Expand Down
6 changes: 6 additions & 0 deletions Lib/test/test_strptime.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import os
import sys
from test import support
from test.support import skip_if_buggy_ucrt_strfptime
from datetime import date as datetime_date

import _strptime
Expand Down Expand Up @@ -135,6 +136,7 @@ def test_pattern_escaping(self):
"%s does not have re characters escaped properly" %
pattern_string)

@skip_if_buggy_ucrt_strfptime
def test_compile(self):
# Check that compiled regex is correct
found = self.time_re.compile(r"%A").match(self.locale_time.f_weekday[6])
Expand Down Expand Up @@ -365,6 +367,7 @@ def test_bad_offset(self):
_strptime._strptime("-01:3030", "%z")
self.assertEqual("Inconsistent use of : in -01:3030", str(err.exception))

@skip_if_buggy_ucrt_strfptime
def test_timezone(self):
# Test timezone directives.
# When gmtime() is used with %Z, entire result of strftime() is empty.
Expand Down Expand Up @@ -489,6 +492,7 @@ class CalculationTests(unittest.TestCase):
def setUp(self):
self.time_tuple = time.gmtime()

@skip_if_buggy_ucrt_strfptime
def test_julian_calculation(self):
# Make sure that when Julian is missing that it is calculated
format_string = "%Y %m %d %H %M %S %w %Z"
Expand All @@ -498,6 +502,7 @@ def test_julian_calculation(self):
"Calculation of tm_yday failed; %s != %s" %
(result.tm_yday, self.time_tuple.tm_yday))

@skip_if_buggy_ucrt_strfptime
def test_gregorian_calculation(self):
# Test that Gregorian date can be calculated from Julian day
format_string = "%Y %H %M %S %w %j %Z"
Expand All @@ -512,6 +517,7 @@ def test_gregorian_calculation(self):
self.time_tuple.tm_year, self.time_tuple.tm_mon,
self.time_tuple.tm_mday))

@skip_if_buggy_ucrt_strfptime
def test_day_of_week_calculation(self):
# Test that the day of the week is calculated as needed
format_string = "%Y %m %d %H %S %j %Z"
Expand Down
3 changes: 3 additions & 0 deletions Lib/test/test_time.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
except ImportError:
_testcapi = None

from test.support import skip_if_buggy_ucrt_strfptime

# Max year is only limited by the size of C int.
SIZEOF_INT = sysconfig.get_config_var('SIZEOF_INT') or 4
Expand Down Expand Up @@ -250,6 +251,7 @@ def test_default_values_for_zero(self):
result = time.strftime("%Y %m %d %H %M %S %w %j", (2000,)+(0,)*8)
self.assertEqual(expected, result)

@skip_if_buggy_ucrt_strfptime
def test_strptime(self):
# Should be able to go round-trip from strftime to strptime without
# raising an exception.
Expand Down Expand Up @@ -672,6 +674,7 @@ class TestStrftime4dyear(_TestStrftimeYear, _Test4dYear, unittest.TestCase):


class TestPytime(unittest.TestCase):
@skip_if_buggy_ucrt_strfptime
@unittest.skipUnless(time._STRUCT_TM_ITEMS == 11, "needs tm_zone support")
def test_localtime_timezone(self):

Expand Down