bpo-32153: Add unit test for create_autospec with partial function returned in getattr by tirkarthi · Pull Request #10398 · python/cpython

tirkarthi · 2018-11-07T19:32:30Z

This adds an unit test for https://bugs.python.org/issue32153 which was out dated and fixed by https://bugs.python.org/issue28919 . During create_autospec the function attributes are copied and when getattr returns a partial function then it doesn't contain __name__ causing AttributeError . This was fixed in 3.7 suppressing AttributeError but was not backported thus this test can be applied only on 3.7 and master.

Fails on 3.6 branch

   cpython git:(25bd107399)   ./python.exe
Python 3.6.7+ (remotes/upstream/3.6:25bd107399, Nov  8 2018, 00:50:43)
[GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> ^D
   cpython git:(25bd107399)   ./python.exe -m unittest -v unittest.test.testmock.testhelpers.SpecSignatureTest.test_autospec_getattr_partial_function
test_autospec_getattr_partial_function (unittest.test.testmock.testhelpers.SpecSignatureTest) ... ERROR

======================================================================
ERROR: test_autospec_getattr_partial_function (unittest.test.testmock.testhelpers.SpecSignatureTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/test/testmock/testhelpers.py", line 884, in test_autospec_getattr_partial_function
    autospec = create_autospec(proxy)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 2182, in create_autospec
    _check_signature(spec, mock, is_type, instance)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 102, in _check_signature
    _copy_func_details(func, checksig)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/unittest/mock.py", line 107, in _copy_func_details
    funcopy.__name__ = func.__name__
AttributeError: 'functools.partial' object has no attribute '__name__'

----------------------------------------------------------------------
Ran 1 test in 0.007s

FAILED (errors=1)

Passes on master

   cpython git:(bpo32153) ./python.exe
Python 3.8.0a0 (heads/bpo32153:3e9cd8d982, Nov  8 2018, 00:53:46)
[Clang 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> ^D
   cpython git:(bpo32153) ./python.exe -m unittest -v unittest.test.testmock.testhelpers.SpecSignatureTest.test_autospec_getattr_partial_function
test_autospec_getattr_partial_function (unittest.test.testmock.testhelpers.SpecSignatureTest) ... ok

----------------------------------------------------------------------
Ran 1 test in 0.005s

OK

https://bugs.python.org/issue32513

serhiy-storchaka · 2018-11-09T07:56:38Z

+        class Foo(object):
+
+            def __getattr__(self, name):
+                return partial(lambda name: name, name)


What is name here? The parameter of __getattr__ or the parameter of the lambda? It is better to use different names for avoiding confusion.

Unconditional return in __getattr__() can cause issues, because internal code can get some special instance attributes and be confused by getting a lambda. Return only for specific name, and raise an AttributeError in other cases.

if name == '...': return ... raise AttributeError

What is name here? The parameter of getattr or the parameter of the lambda? It is better to use different names for avoiding confusion.

I will change them. Thanks.

Unconditional return in getattr() can cause issues, because internal code can get some special instance attributes and be confused by getting a lambda. Return only for specific name, and raise an AttributeError in other cases.

From the original report this is about __getattr__ returning a partial function unconditionally for all attributes and partial functions have no __name__ attribute during copy which occurs at https://github.com/python/cpython/blob/master/Lib/unittest/mock.py#L108. I am slightly confused over why we should have an if clause for this and raise AttributeError that might change the original report that needs to be tested.

serhiy-storchaka · 2018-11-09T07:56:55Z

+    def test_autospec_getattr_partial_function(self):
+        # bpo-32153 : getattr returning partial functions without
+        # __name__ should not create AttributeError in create_autospec
+        class Foo(object):


(object) is not needed.

Removed. Thanks.

bedevere-bot · 2018-12-03T07:58:17Z

@cjw296: Please replace # with GH- in the commit message next time. Thanks!

tirkarthi · 2018-12-03T08:06:45Z

@cjw296 Thanks for merging the PR. This test can be back ported to 3.7 since it's fixed there. I think @miss-islington will make the back port PR once you add the label "needs backport to 3.7".

miss-islington · 2018-12-03T08:08:28Z

Thanks @tirkarthi for the PR, and @cjw296 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

bedevere-bot · 2018-12-03T08:09:03Z

GH-10855 is a backport of this pull request to the 3.7 branch.

…turned in getattr (pythonGH-10398) * Add create_autospec with partial function returned in getattr * Use self.assertFalse instead of assert * Use different names and remove object (cherry picked from commit c667b09) Co-authored-by: Xtreak <tirkarthi@users.noreply.github.com>

cjw296 · 2018-12-03T08:11:00Z

@tirkarthi - done, do you know if the fix made it to 3.6 too?

…turned in getattr (GH-10398) (#10855) * Add create_autospec with partial function returned in getattr * Use self.assertFalse instead of assert * Use different names and remove object (cherry picked from commit c667b09) Co-authored-by: Xtreak <tirkarthi@users.noreply.github.com>

tirkarthi · 2018-12-03T08:39:14Z

@cjw296 No, the issue was not fixed in 3.6 as noted in https://bugs.python.org/issue32153#msg327531 . I also ran the test for confirmation in latest 3.6 branch while opening the PR #10398 (comment) and it fails. So this test should not be back ported to 3.6.

Add create_autospec with partial function returned in getattr

757fbf4

bedevere-bot added the awaiting review label Nov 7, 2018

the-knights-who-say-ni added the CLA signed label Nov 7, 2018

tirkarthi changed the title ~~bpo-32513: Add unit test for create_autospec with partial function returned in getattr~~ bpo-32153: Add unit test for create_autospec with partial function returned in getattr Nov 7, 2018

lisroach reviewed Nov 9, 2018

View reviewed changes

Comment thread Lib/unittest/test/testmock/testhelpers.py Outdated

lisroach added the skip news label Nov 9, 2018

Use self.assertFalse instead of assert

d184402

lisroach approved these changes Nov 9, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Nov 9, 2018

serhiy-storchaka reviewed Nov 9, 2018

View reviewed changes

Use different names and remove object

2e71976

tirkarthi mentioned this pull request Dec 1, 2018

mock.create_autospec fails if an attribute is a partial function testing-cabal/mock#418

Closed

cjw296 merged commit c667b09 into python:master Dec 3, 2018

bedevere-bot removed the awaiting merge label Dec 3, 2018

cjw296 added the needs backport to 3.7 label Dec 3, 2018

bedevere-bot removed the needs backport to 3.7 label Dec 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

bpo-32153: Add unit test for create_autospec with partial function returned in getattr#10398

bpo-32153: Add unit test for create_autospec with partial function returned in getattr#10398
cjw296 merged 3 commits into
python:masterfrom
tirkarthi:bpo32153

tirkarthi commented Nov 7, 2018 •

edited by bedevere-bot

Loading

Uh oh!

Uh oh!

serhiy-storchaka Nov 9, 2018

Uh oh!

tirkarthi Nov 9, 2018

Uh oh!

serhiy-storchaka Nov 9, 2018

Uh oh!

tirkarthi Nov 9, 2018

Uh oh!

bedevere-bot commented Dec 3, 2018

Uh oh!

tirkarthi commented Dec 3, 2018

Uh oh!

miss-islington commented Dec 3, 2018

Uh oh!

bedevere-bot commented Dec 3, 2018

Uh oh!

cjw296 commented Dec 3, 2018

Uh oh!

tirkarthi commented Dec 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

tirkarthi commented Nov 7, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

serhiy-storchaka Nov 9, 2018

Choose a reason for hiding this comment

Uh oh!

tirkarthi Nov 9, 2018

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Nov 9, 2018

Choose a reason for hiding this comment

Uh oh!

tirkarthi Nov 9, 2018

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Dec 3, 2018

Uh oh!

tirkarthi commented Dec 3, 2018

Uh oh!

miss-islington commented Dec 3, 2018

Uh oh!

bedevere-bot commented Dec 3, 2018

Uh oh!

cjw296 commented Dec 3, 2018

Uh oh!

tirkarthi commented Dec 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tirkarthi commented Nov 7, 2018 •

edited by bedevere-bot

Loading