gh-91803: Mock - fix error when using autospec methods with seal#92213
gh-91803: Mock - fix error when using autospec methods with seal#92213cjw296 merged 12 commits intopython:mainfrom
Conversation
…lakov/cpython into 91803-Fix-seal-with-autospec-methods
AlexWaygood
added
the
stdlib
carljm
left a comment
carljm
left a comment
There was a problem hiding this comment.
The fix looks correct to me. "Being callable" is clearly part of the contract of a callable that should be replicated by autospec, and seal should not remove that.
| with self.assertRaises(AttributeError): | ||
| foo.bar = 1 | ||
| with self.assertRaises(AttributeError): | ||
| foo.bar2() |
There was a problem hiding this comment.
I think Nikita is largely on a break from open-source stuff right now FYI
There was a problem hiding this comment.
I feel this is an incorrect assert since bar2 method is present on Foo and shouldn't raise. Removing the assert statement the AttributeError here asserted is for the exception that return_value is not found due to seal which this PR fixes and not that bar2 is not found.
./python Lib/unittest/test/testmock/testsealable.py
..............EE....
======================================================================
ERROR: test_seal_with_autospec (__main__.TestSealable.test_seal_with_autospec) (spec_set=True)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/karthikeyan/stuff/python/cpython/Lib/unittest/test/testmock/testsealable.py", line 215, in test_seal_with_autospec
foo.bar2()
^^^^^^^^^^
File "/home/karthikeyan/stuff/python/cpython/Lib/unittest/mock.py", line 1108, in __call__
return self._mock_call(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/karthikeyan/stuff/python/cpython/Lib/unittest/mock.py", line 1112, in _mock_call
return self._execute_mock_call(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/karthikeyan/stuff/python/cpython/Lib/unittest/mock.py", line 1184, in _execute_mock_call
return self.return_value
^^^^^^^^^^^^^^^^^
File "/home/karthikeyan/stuff/python/cpython/Lib/unittest/mock.py", line 638, in __getattr__
raise AttributeError("Mock object has no attribute %r" % name)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: Mock object has no attribute 'return_value'
======================================================================
ERROR: test_seal_with_autospec (__main__.TestSealable.test_seal_with_autospec) (spec_set=False)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/karthikeyan/stuff/python/cpython/Lib/unittest/test/testmock/testsealable.py", line 215, in test_seal_with_autospec
foo.bar2()
^^^^^^^^^^
File "/home/karthikeyan/stuff/python/cpython/Lib/unittest/mock.py", line 1108, in __call__
return self._mock_call(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/karthikeyan/stuff/python/cpython/Lib/unittest/mock.py", line 1112, in _mock_call
return self._execute_mock_call(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/karthikeyan/stuff/python/cpython/Lib/unittest/mock.py", line 1184, in _execute_mock_call
return self.return_value
^^^^^^^^^^^^^^^^^
File "/home/karthikeyan/stuff/python/cpython/Lib/unittest/mock.py", line 638, in __getattr__
raise AttributeError("Mock object has no attribute %r" % name)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: Mock object has no attribute 'return_value'
----------------------------------------------------------------------
Ran 19 tests in 0.121s
FAILED (errors=2)
Lib/unittest/mock.py
Outdated
| _new_parent=parent, | ||
| **kwargs) | ||
| mock._mock_children[entry] = new | ||
| new.return_value = Mock() |
There was a problem hiding this comment.
This could use the child_klass to construct the mock object instead of Mock. This can break code that depended on the return value being an instance of child_klass. Example case as below :
from unittest.mock import create_autospec, seal
class Foo:
def bar(self):
pass
spec = create_autospec(Foo)
print(spec.bar())
print(len(spec.bar()))Python 3.10
python3.10 gh92213.py
<MagicMock name='mock.bar()' id='140499591708000'>
0
With patch
./python gh92213.py
<Mock name='mock.bar()' id='140578691284304'>
Traceback (most recent call last):
File "/home/karthikeyan/stuff/python/cpython/gh92213.py", line 10, in <module>
print(len(spec.bar()))
^^^^^^^^^^^^^^^
TypeError: object of type 'Mock' has no len()
There was a problem hiding this comment.
That makes sense to me, I used Mock originally because I wasn't sure if MagicMock might "evade" seal() if set explicitly, but I tested it and it does not:
class Foo:
x = 1
def foo(self) -> int:
return 0
foo=Foo()
foo = mock.create_autospec(foo)
foo.foo().x
==> AttributeError: mock.foo().x
I've pushed the update to use child_klass()
There was a problem hiding this comment.
@akulakov - rather than just manually testing this, please can you add a test to the test suite to ensure it doesn't change in the future?
There was a problem hiding this comment.
@cjw296 that makes sense, I've expanded the unit test - thanks for noting this.
tirkarthi
left a comment
tirkarthi
left a comment
There was a problem hiding this comment.
Please fix the conflicts in tests. Changes look fine to me.
|
@tirkarthi thanks for letting me know, I fixed the conflict. |
Co-authored-by: Karthikeyan Singaravelan <tir.karthi@gmail.com>
|
cc: @mariocj89 |
|
Updated branch to kick off test again. @tirkarthi - you approved this back in May. Any reason why it was not merged yet? |
|
Thanks for reviewing @tirkarthi |
8 participants
Uh oh!
There was an error while loading. Please reload this page.