| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| Expand Up | @@ -591,27 +591,36 @@ def test_Struct_reinitialization(self): | |||||
| # Struct instance. This test can be used to detect the leak | ||||||
| # when running with regrtest -L. | ||||||
| s = struct.Struct('>h') | ||||||
| s.__init__('>hh') | ||||||
| msg = 'Re-initialization .* will not work' | ||||||
| with self.assertWarnsRegex(FutureWarning, msg): | ||||||
| s.__init__('>hh') | ||||||
| self.assertEqual(s.format, '>hh') | ||||||
| packed = b'\x00\x01\x00\x02' | ||||||
| self.assertEqual(s.pack(1, 2), packed) | ||||||
| self.assertEqual(s.unpack(packed), (1, 2)) | ||||||
|
|
||||||
| with self.assertRaises(UnicodeEncodeError): | ||||||
| s.__init__('\udc00') | ||||||
| s.__init__('>hh') # same format | ||||||
| self.assertEqual(s.format, '>hh') | ||||||
| self.assertEqual(s.pack(1, 2), packed) | ||||||
| self.assertEqual(s.unpack(packed), (1, 2)) | ||||||
|
|
||||||
| with self.assertRaises(struct.error): | ||||||
| s.__init__('$') | ||||||
| with self.assertWarnsRegex(FutureWarning, msg): | ||||||
| with self.assertRaises(UnicodeEncodeError): | ||||||
| s.__init__('\udc00') | ||||||
| self.assertEqual(s.format, '>hh') | ||||||
| self.assertEqual(s.pack(1, 2), packed) | ||||||
| self.assertEqual(s.unpack(packed), (1, 2)) | ||||||
|
|
||||||
| with self.assertWarnsRegex(FutureWarning, msg): | ||||||
| with self.assertRaises(struct.error): | ||||||
| s.__init__('$') | ||||||
| self.assertEqual(s.format, '>hh') | ||||||
| self.assertEqual(s.pack(1, 2), packed) | ||||||
| self.assertEqual(s.unpack(packed), (1, 2)) | ||||||
|
|
||||||
| def check_sizeof(self, format_str, number_of_codes): | ||||||
| # The size of 'PyStructObject' | ||||||
| totalsize = support.calcobjsize('2n3P') | ||||||
| totalsize = support.calcobjsize('2n3P?0P') | ||||||
| # The size taken up by the 'formatcode' dynamic array | ||||||
| totalsize += struct.calcsize('P3n0P') * (number_of_codes + 1) | ||||||
| support.check_sizeof(self, struct.Struct(format_str), totalsize) | ||||||
| Expand Down Expand Up | @@ -809,14 +818,152 @@ def test_error_propagation(fmt_str): | |||||
| test_error_propagation('N') | ||||||
| test_error_propagation('n') | ||||||
|
|
||||||
| def test_struct_subclass_instantiation(self): | ||||||
| def test_custom_struct_init(self): | ||||||
| # Regression test for https://github.com/python/cpython/issues/112358 | ||||||
| class MyStruct(struct.Struct): | ||||||
| def __init__(self): | ||||||
| def __init__(self, *args, **kwargs): | ||||||
| super().__init__('>h') | ||||||
|
|
||||||
| my_struct = MyStruct('>h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI think this will be more clear, per Victor's suggestion:
Suggested change
(and in all cases below too)
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityWhat if self->s_format and and self->s_codes are de-synchronized? The original test used Struct.pack().
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
That's true. But I think that with format field tests will be more clear.
I see, you test some such cases with bad characters.
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI added assertions for format, but we need also functional tests, because internals can be initialized twice, and if something goes wrong, we will and with inconsistent format and pack(). See new cases in test_Struct_reinitialization. We should fix this in other issue.
Sorry, something went wrong.
All reactions
|
||||||
| my_struct = MyStruct(format='>h') | ||||||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityAh, I entirely forgot that format is a positional or keyword argument in the master. I'll fix my patch. But again, this should emit a warning.
Sorry, something went wrong.
All reactions
|
||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
|
|
||||||
| warnmsg = r"Different format arguments for __new__\(\) and __init__\(\) methods of Struct" | ||||||
| with self.assertWarnsRegex(FutureWarning, warnmsg): | ||||||
| my_struct = MyStruct('<h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertWarnsRegex(FutureWarning, warnmsg): | ||||||
| my_struct = MyStruct(format='<h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
|
|
||||||
| warnmsg = r"Struct\(\) missing required argument 'format' \(pos 1\)" | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg): | ||||||
| my_struct = MyStruct() | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg): | ||||||
| my_struct = MyStruct(arg='>h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
|
|
||||||
| warnmsg = r"Struct\(\) takes at most 1 argument \(2 given\)" | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg): | ||||||
| my_struct = MyStruct('>h', 42) | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg): | ||||||
| my_struct = MyStruct('>h', arg=42) | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg): | ||||||
| my_struct = MyStruct('>h', format=42) | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg): | ||||||
| my_struct = MyStruct(format='>h', arg=42) | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
|
|
||||||
| warnmsg = r"Invalid 'format' argument for Struct\.__new__\(\): " | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg + '.*must be'): | ||||||
| my_struct = MyStruct(42) | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg + '.*must be'): | ||||||
| my_struct = MyStruct(format=42) | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg + 'bad char'): | ||||||
| my_struct = MyStruct('$') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg + 'bad char'): | ||||||
| my_struct = MyStruct(format='$') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg + ".*can't encode"): | ||||||
| my_struct = MyStruct('\udc00') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertWarnsRegex(DeprecationWarning, warnmsg + ".*can't encode"): | ||||||
| my_struct = MyStruct(format='\udc00') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
|
|
||||||
| def test_custom_struct_new(self): | ||||||
| # New way, no warnings: | ||||||
| class MyStruct(struct.Struct): | ||||||
| def __new__(cls, *args, **kwargs): | ||||||
| return super().__new__(cls, '>h') | ||||||
|
|
||||||
| for format in '>h', '<h', 42, '$', '\u20ac', '\udc00', b'\xa4': | ||||||
| with self.subTest(format=format): | ||||||
| my_struct = MyStruct(format) | ||||||
| self.assertEqual(my_struct.format, '>h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| my_struct = MyStruct(format='<h') | ||||||
| self.assertEqual(my_struct.format, '>h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| my_struct = MyStruct() | ||||||
| self.assertEqual(my_struct.format, '>h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| my_struct = MyStruct('<h', 42) | ||||||
| self.assertEqual(my_struct.format, '>h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
|
|
||||||
| def test_custom_struct_new_and_init(self): | ||||||
| # New way, no warnings: | ||||||
| class MyStruct(struct.Struct): | ||||||
| def __new__(cls, newargs, initargs): | ||||||
| return super().__new__(cls, *newargs) | ||||||
| def __init__(self, newargs, initargs): | ||||||
| if initargs is not None: | ||||||
| super().__init__(*initargs) | ||||||
|
|
||||||
| my_struct = MyStruct(('>h',), ('>h',)) | ||||||
|
Comment thread
Comment on lines
+904
to
+912
Copy link
Copy Markdown
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityAgain, why no warnings here? BTW, I doubt this usage pattern come from reality ;-)
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityBecause __new__() and __init__() are called with the same argument, as expected. You are supposed to do this if you write a code that works in all versions. As for other cases, having both custom __new__() and __init__() which call corresponding parent's methods with different arguments is unusual, but this need to be tested. I found bugs in my code when added these tests.
Sorry, something went wrong.
encukou reacted with heart emoji
All reactions
|
||||||
| self.assertEqual(my_struct.format, '>h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertRaises(TypeError): | ||||||
| MyStruct((), ()) | ||||||
| with self.assertRaises(TypeError): | ||||||
| MyStruct(('>h',), ()) | ||||||
| with self.assertRaises(TypeError): | ||||||
| MyStruct((), ('>h',)) | ||||||
| with self.assertRaises(TypeError): | ||||||
| MyStruct((42,), ('>h',)) | ||||||
| with self.assertWarns(FutureWarning): | ||||||
| with self.assertRaises(TypeError): | ||||||
| MyStruct(('>h',), (42,)) | ||||||
| with self.assertRaises(struct.error): | ||||||
| MyStruct(('$',), ('>h',)) | ||||||
| with self.assertWarns(FutureWarning): | ||||||
| with self.assertRaises(struct.error): | ||||||
| MyStruct(('>h',), ('$',)) | ||||||
| with self.assertRaises(UnicodeEncodeError): | ||||||
| MyStruct(('\udc00',), ('>h',)) | ||||||
| with self.assertWarns(FutureWarning): | ||||||
| with self.assertRaises(UnicodeEncodeError): | ||||||
| MyStruct(('>h',), ('\udc00',)) | ||||||
| with self.assertWarns(FutureWarning): | ||||||
| my_struct = MyStruct(('>h',), ('<h',)) | ||||||
| self.assertEqual(my_struct.format, '<h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x39\x30') | ||||||
|
|
||||||
| def test_no_custom_struct_new_or_init(self): | ||||||
| class MyStruct(struct.Struct): | ||||||
| pass | ||||||
|
|
||||||
| my_struct = MyStruct('>h') | ||||||
| self.assertEqual(my_struct.format, '>h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| my_struct = MyStruct(format='>h') | ||||||
| self.assertEqual(my_struct.format, '>h') | ||||||
| self.assertEqual(my_struct.pack(12345), b'\x30\x39') | ||||||
| with self.assertRaises(TypeError): | ||||||
| MyStruct() | ||||||
| with self.assertRaises(TypeError): | ||||||
| MyStruct(42) | ||||||
| with self.assertRaises(struct.error): | ||||||
| MyStruct('$') | ||||||
| with self.assertRaises(UnicodeEncodeError): | ||||||
| MyStruct('\udc00') | ||||||
| with self.assertRaises(TypeError): | ||||||
| MyStruct('>h', 42) | ||||||
| with self.assertRaises(TypeError): | ||||||
| MyStruct('>h', arg=42) | ||||||
| with self.assertRaises(TypeError): | ||||||
| MyStruct(arg=42) | ||||||
| with self.assertRaises(TypeError): | ||||||
| MyStruct('>h', format='>h') | ||||||
|
|
||||||
| def test_repr(self): | ||||||
| s = struct.Struct('=i2H') | ||||||
| Expand Down | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Calling the ``Struct.__new__()`` without required argument now is deprecated. | ||
| Calling :meth:`~object.__init__` method on initialized :class:`~struct.Struct` | ||
| objects is deprecated. |
| Back | FazBrowse Home | New Git URL |
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityWhy this not raises a warning? I think we should warn in all cases, where Struct.__init__() was explicitly called. // #143659 (comment)
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityBecause this works with old and with new code. Both __new__ and __init__ take the same argument.
The code
works now and will work in future.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.