Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
bpo-29403: Fix mock's broken autospec behavior on method-bound builtin functions#3
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
bpo-29403: Fix mock's broken autospec behavior on method-bound builtin functions#3
Uh oh!
There was an error while loading.Please reload this page.
Conversation
the-knights-who-say-ni commentedFeb 10, 2017
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 thePSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username onbugs.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 |
ShalbafZadeh commentedFeb 10, 2017
@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. |
ShalbafZadeh commentedFeb 10, 2017
@habnabit seems odd |
c8bc9c9 to9148028CompareRebased and pushed, including a test fix. The third-party mock used |
e12a088 to97bb733CompareRebased 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 -5 Continue to review full report at Codecov.
|
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.
I haven't had a time to dive into the whole life cycle of_set_signature() yet, but from a quick glance it looks good to me.
Please add an entry to |
97bb733 toc5b3653CompareOkay, 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? |
buhtz commentedJul 19, 2017
Can we merge this? |
Cython will, in the right circumstances, offer a MethodType instancewhere im_func is a builtin function. Any instance of MethodType isautomatically assumed to be a python-defined function (morespecifically, 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 theim_func had no inspectable signature. This causes problems deeper insidemock, as _set_signature is assumed to _always_ return a mock, andnothing checked its return value.In similar corner cases, autospec will simply not check the spec of thefunction, so _set_signature is amended to now return early with theoriginal, not-wrapped mock object.
75ad0f4 to989e99aCompare3: Add Py2x flag r=ltratt a=nanjekyejoannahThis 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 darwinType "help", "copyright", "credits" or "license" for more information.>>> import sys>>> sys.flagssys.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 <jnanjekye@python.org>
5: Add 2.x related warnings r=ltratt a=nanjekyejoannahI 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 beforepython#4 Co-authored-by: Joannah Nanjekye <jnanjekye@python.org>
http://bugs.python.org/issue29403