Copilot commented on code in PR #3595:
URL: https://github.com/apache/thrift/pull/3595#discussion_r3469348936
##########
lib/py/test/thrift_TSerializer.py:
##########
@@ -70,6 +70,17 @@ def test_TCompactProtocolAccelerated(self):
factory = TCompactProtocolAcceleratedFactory()
self.verify(self.compact_serialized, factory)
+ def test_TCompactProtocolAccelerated_unicode_matches_python(self):
+ message = Message("é", 42)
Review Comment:
The new test uses a non-ASCII literal ("é") without an encoding cookie. This
can make the test file fail to parse under Python 2 (default ASCII source
encoding) and also makes it ambiguous whether `message.body` is intended to be
`unicode` vs `bytes` on Py2. Prefer using an explicit unicode escape literal so
the test is source-encoding-independent and always exercises the unicode path.
##########
lib/py/test/thrift_TBinaryProtocol.py:
##########
@@ -280,6 +296,28 @@ def test_TBinaryProtocol_write_read(self):
print("Assertion fail")
raise e
+ def test_accelerated_utf8_roundtrip_on_application_exception(self):
+ if not self._has_fastbinary:
+ self.skipTest("C extension not available")
+
+ original = TApplicationException(
+ type=TApplicationException.PROTOCOL_ERROR,
+ message=("snowman-\u2603-rocket-\U0001F680-" * 32),
Review Comment:
`message` is currently a plain string literal containing `\u`/`\U` escapes.
On Python 2 this may fail to parse (or not produce a `unicode` object), and it
also won’t reliably exercise the unicode encoding path. Make this an explicit
unicode literal so the test is version-independent and consistently tests UTF-8
encoding of unicode.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
