Skip to content

bpo-34482: Add tests for proper handling of non-UTF-8-encodable strin… #8878

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Oct 23, 2018
Merged

bpo-34482: Add tests for proper handling of non-UTF-8-encodable strin… #8878

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
97037bd
bpo-34482: Add tests for proper handling of non-UTF-8-encodable strin…
Aug 21, 2018
a7e435d
Remove FIXMEs and use assertEqual in strftime tests
Aug 23, 2018
4603f7e
Remove the NEWS entry
Aug 23, 2018
beca2bd
Add a separate test for datetime.strftime()
Aug 23, 2018
51bd826
Remove check for strftime() return value
Aug 24, 2018
f24c13c
Don't mix regular and surrogate-related tests in test_strptime()
Aug 30, 2018
faadbb9
Merge branch 'master' into bpo-34482
taleinat Oct 22, 2018
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
40 changes: 40 additions & 0 deletions Lib/test/datetimetester.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ def test_tzname(self):
self.assertEqual('UTC+09:30', timezone(9.5 * HOUR).tzname(None))
self.assertEqual('UTC-00:01', timezone(timedelta(minutes=-1)).tzname(None))
self.assertEqual('XYZ', timezone(-5 * HOUR, 'XYZ').tzname(None))
# bpo-34482: Check that surrogates are handled properly.
self.assertEqual('\ud800', timezone(ZERO, '\ud800').tzname(None))

# Sub-minute offsets:
self.assertEqual('UTC+01:06:40', timezone(timedelta(0, 4000)).tzname(None))
Expand Down Expand Up @@ -1308,6 +1310,12 @@ def test_strftime(self):
except ValueError:
pass

# bpo-34482: Check that surrogates don't cause a crash.
try:
t.strftime('%y\ud800%m')
except UnicodeEncodeError:
pass

#check that this standard extension works
t.strftime("%f")

Expand Down Expand Up @@ -1747,6 +1755,9 @@ def test_isoformat(self):
self.assertEqual(t.isoformat('T'), "0001-02-03T04:05:01.000123")
self.assertEqual(t.isoformat(' '), "0001-02-03 04:05:01.000123")
self.assertEqual(t.isoformat('\x00'), "0001-02-03\x0004:05:01.000123")
# bpo-34482: Check that surrogates are handled properly.
self.assertEqual(t.isoformat('\ud800'),
"0001-02-03\ud80004:05:01.000123")
self.assertEqual(t.isoformat(timespec='hours'), "0001-02-03T04")
self.assertEqual(t.isoformat(timespec='minutes'), "0001-02-03T04:05")
self.assertEqual(t.isoformat(timespec='seconds'), "0001-02-03T04:05:01")
Expand All @@ -1755,6 +1766,8 @@ def test_isoformat(self):
self.assertEqual(t.isoformat(timespec='auto'), "0001-02-03T04:05:01.000123")
self.assertEqual(t.isoformat(sep=' ', timespec='minutes'), "0001-02-03 04:05")
self.assertRaises(ValueError, t.isoformat, timespec='foo')
# bpo-34482: Check that surrogates are handled properly.
self.assertRaises(ValueError, t.isoformat, timespec='\ud800')
# str is ISO format with the separator forced to a blank.
self.assertEqual(str(t), "0001-02-03 04:05:01.000123")

Expand Down Expand Up @@ -2286,6 +2299,19 @@ def test_strptime(self):
self.assertIs(type(expected), self.theclass)
self.assertIs(type(got), self.theclass)

# bpo-34482: Check that surrogates are handled properly.
inputs = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's leave the first "happy path" test as it is.

I suggest separating the surrogate examples, keeping the loop just for them, and removing the type assertions for them, i.e. just checking that the result equals what is expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable, fixed.

('2004-12-01\ud80013:02:47.197', '%Y-%m-%d\ud800%H:%M:%S.%f'),
('2004\ud80012-01 13:02:47.197', '%Y\ud800%m-%d %H:%M:%S.%f'),
('2004-12-01 13:02\ud80047.197', '%Y-%m-%d %H:%M\ud800%S.%f'),
]
for string, format in inputs:
with self.subTest(string=string, format=format):
expected = _strptime._strptime_datetime(self.theclass, string,
format)
got = self.theclass.strptime(string, format)
self.assertEqual(expected, got)

strptime = self.theclass.strptime
self.assertEqual(strptime("+0002", "%z").utcoffset(), 2 * MINUTE)
self.assertEqual(strptime("-0002", "%z").utcoffset(), -2 * MINUTE)
Expand Down Expand Up @@ -2353,6 +2379,12 @@ def test_more_strftime(self):
t = t.replace(tzinfo=tz)
self.assertEqual(t.strftime("%z"), "-0200" + z)

# bpo-34482: Check that surrogates don't cause a crash.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this belongs in the test_more_strftime() test method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? TestDateTime.test_more_strftime is a datetime-specific extension to TestDate.strftime (which in inherited by TestDateTime), and the latter contains my test for surrogates. I suggest to either move all surrogate-related strftime tests to separate methods or keep things as is.

Copy link
Member

Choose a reason for hiding this comment

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

I personally think breaking the tests up a bit more is a good idea, though I don't know the overall test philosophy of CPython.

If the idea is to put all the strftime tests in one test, I'd probably use subTest to distinguish the various failure modes, at the very least.

try:
t.strftime('%y\ud800%m %H\ud800%M')
except UnicodeEncodeError:
pass

def test_extract(self):
dt = self.theclass(2002, 3, 4, 18, 45, 3, 1234)
self.assertEqual(dt.date(), date(2002, 3, 4))
Expand Down Expand Up @@ -2878,6 +2910,8 @@ def test_isoformat(self):
self.assertEqual(t.isoformat(timespec='microseconds'), "12:34:56.123456")
self.assertEqual(t.isoformat(timespec='auto'), "12:34:56.123456")
self.assertRaises(ValueError, t.isoformat, timespec='monkey')
# bpo-34482: Check that surrogates are handled properly.
self.assertRaises(ValueError, t.isoformat, timespec='\ud800')

t = self.theclass(hour=12, minute=34, second=56, microsecond=999500)
self.assertEqual(t.isoformat(timespec='milliseconds'), "12:34:56.999")
Expand Down Expand Up @@ -2928,6 +2962,12 @@ def test_strftime(self):
# A naive object replaces %z and %Z with empty strings.
self.assertEqual(t.strftime("'%z' '%Z'"), "'' ''")

# bpo-34482: Check that surrogates don't cause a crash.
try:
t.strftime('%H\ud800%M')
except UnicodeEncodeError:
pass

def test_format(self):
t = self.theclass(1, 2, 3, 4)
self.assertEqual(t.__format__(''), str(t))
Expand Down