bpo-6986: Add a comment to clarify a test of _json.make_encoder() #3789
Conversation
Lib/test/test_json/test_speedups.py
Outdated
| @@ -31,6 +31,8 @@ def test(value): | |||
|
|
|||
| class TestEncode(CTest): | |||
| def test_make_encoder(self): | |||
| # Issue #6986: The interpreter shouldn't crash in case c_make_encoder() | |||
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Lib/test/test_json/test_speedups.py
Outdated
| @@ -31,6 +31,8 @@ def test(value): | |||
|
|
|||
| class TestEncode(CTest): | |||
| def test_make_encoder(self): | |||
| # Issue #6986: The interpreter shouldn't crash in case c_make_encoder() | |||
| # receives invalid arguments. | |||
There was a problem hiding this comment.
What exactly invalid argument is tested?
There was a problem hiding this comment.
Is it just insufficient number of arguments? Then why not just call it without arguments? Or call it with 8 valid arguments (it requires 9 arguments)?
There was a problem hiding this comment.
I guess that before Victor's patch was committed, these specially crafted arguments would cause a crash, and with the patch, nothing bad would happen. (Victor, please correct me if I am wrong.)
Maybe changing the test's name would make it clearer? Or do you have something else in mind?
|
I didn't expect the Spanish Inquisition! |
|
Nobody expects the Spanish Inquisition! @Haypo: please review the changes made to this pull request. |
https://bugs.python.org/issue6986