bpo-29403: Fix mock's broken autospec behavior on method-bound builtin functions #3
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:
Thanks again to your contribution and we look forward to looking at it! |
|
I signed the CLA when I opened the bug on bpo last week, but my account doesn't say received still: http://bugs.python.org/user25445 |
|
@habnabit |
|
@ShalbafZadeh that was an old account; I made a new one last week from a newer openid provider. http://bugs.python.org/user25445 is the new one, which I'm logged into now. |
|
@habnabit seems odd |
habnabit
force-pushed
the
fix-mock-builtin-methods-for-cython
branch
from
c8bc9c9
to
9148028
Compare
|
Rebased and pushed, including a test fix. The third-party mock used |
habnabit
force-pushed
the
fix-mock-builtin-methods-for-cython
branch
2 times, most recently
from
e12a088
to
97bb733
Compare
|
Rebased again! |
Codecov Report@@ Coverage Diff @@
## master #3 +/- ##
==========================================
+ Coverage 82.37% 82.37% +<.01%
==========================================
Files 1427 1427
Lines 350948 350959 +11
==========================================
+ Hits 289088 289104 +16
+ Misses 61860 61855 -5Continue to review full report at Codecov.
|
|
Please add an entry to |
habnabit
force-pushed
the
fix-mock-builtin-methods-for-cython
branch
from
97bb733
to
c5b3653
Compare
|
Okay, rebased and added a NEWS entry. |
|
This change looks good to me: returning None does not make sense in the one place where _set_signature is called, and I don't think there's a better solution here. If the code wants to try harder and figure out what signature to use, that would be a change to _get_signature, not _set_signature, and this change would still make sense. @voidspace, do you want to take a look, or can we merge? |
|
Can we merge this? |
Cython will, in the right circumstances, offer a MethodType instance where im_func is a builtin function. Any instance of MethodType is automatically assumed to be a python-defined function (more specifically, a function that has an inspectable signature), but _set_signature was still conservative in its assumptions. As a result _set_signature would return early with None instead of a mock since the im_func had no inspectable signature. This causes problems deeper inside mock, as _set_signature is assumed to _always_ return a mock, and nothing checked its return value. In similar corner cases, autospec will simply not check the spec of the function, so _set_signature is amended to now return early with the original, not-wrapped mock object.
berkerpeksag
force-pushed
the
fix-mock-builtin-methods-for-cython
branch
from
75ad0f4
to
989e99a
Compare
3: Add warnings for numbers r=vext01 a=nanjekyejoannah This has several changes, after git playing games with me, I managed to split the commits instead of committing one long one, as I had done before. *First commit*: Adds warnings for when the suffix "L" is used with numbers but also adds a custom Exception for our work suggested in the review of python#2 , called `Py3xWarning`. I should have committed the exception separately though. *Second commit*: Add warnings for octal literals. I fixed a test too Co-authored-by: Joannah Nanjekye <[email protected]>
Fix an uninitialized bool in exception print context.
`struct exception_print_context.need_close` was uninitialized.
Found by oss-fuzz in a test case running under the undefined behavior sanitizer.
https://oss-fuzz.com/testcase-detail/6217746058182656
```
Python/pythonrun.c:1241:28: runtime error: load of value 253, which is not a valid value for type 'bool'
#0 0xbf2203 in print_chained cpython3/Python/pythonrun.c:1241:28
#1 0xbea4bb in print_exception_cause_and_context cpython3/Python/pythonrun.c:1320:19
#2 0xbea4bb in print_exception_recursive cpython3/Python/pythonrun.c:1470:13
#3 0xbe9e39 in _PyErr_Display cpython3/Python/pythonrun.c:1517:9
```
Pretty obvious what the ommission was upon code inspection.
3: Add Py2x flag r=ltratt a=nanjekyejoannah This PR adds the Py2x flag: ``` Python 3.12.0a0 (heads/migration-dirty:5842fee697, May 23 2022, 00:25:04) [Clang 12.0.0 (clang-1200.0.32.29)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> import sys >>> sys.flags sys.flags(debug=0, inspect=0, interactive=0, optimize=0, dont_write_bytecode=0, no_user_site=0, no_site=0, ignore_environment=0, verbose=0, bytes_warning=0, py2x_warning=1, quiet=0, hash_randomization=1, isolated=0, dev_mode=False, utf8_mode=0, warn_default_encoding=0) >>> ``` Co-authored-by: Joannah Nanjekye <[email protected]>
5: Add 2.x related warnings r=ltratt a=nanjekyejoannah I have broken away the warning bit from the [flag](python#3 ) and the [port ](python#4 )PR. Well, the way function calls are done between C and Python is confusing, nothing scary anyway, review maybe a bit annoying. Review this PR before python#4 Co-authored-by: Joannah Nanjekye <[email protected]>
http://bugs.python.org/issue29403