GeezerVison is showing github.com — Disallow Mock spec arguments from being Mocks · Issue #87644 · python/cpython
Disallow Mock spec arguments from being Mocks #87644Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = None
created_at = <Date 2021-03-12.01:53:43.131>
labels = ['type-feature', 'tests', '3.10']
title = 'Disallow Mock spec arguments from being Mocks'
updated_at = <Date 2022-02-02.23:26:50.979>
user = 'https://bugs.python.org/msuozzo' bugs.python.org fields:
activity = <Date 2022-02-02.23:26:50.979>
actor = 'matthew.suozzo'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Tests']
creation = <Date 2021-03-12.01:53:43.131>
creator = 'msuozzo'
dependencies = []
files = []
hgrepos = []
issue_num = 43478
keywords = ['patch']
message_count = 10.0
messages = ['388532', '388753', '388755', '388766', '388767', '388769', '388878', '388880', '390690', '390743']
nosy_count = 10.0
nosy_names = ['gregory.p.smith', 'vstinner', 'cjw296', 'michael.foord', 'lisroach', 'mariocj89', 'pablogsal', 'xtreak', 'msuozzo', 'matthew.suozzo']
pr_nums = ['25326', '25335', '25227', '31090']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue43478'
versions = ['Python 3.10'] Activitymsuozzo commented on Mar 12, 2021 MannequinAuthor More actions An unfortunately common pattern over large codebases of Python tests is for spec'd Mock instances to be provided with Mock objects as their specs. This gives the false sense that a spec constraint is being applied when, in fact, nothing will be disallowed.
The two most frequently observed occurrences of this anti-pattern are as follows:
- Re-patching an existing autospec.
def setUp(self):
mock.patch.object(mod, 'Klass', autospec=True).start()
self.addCleanup(mock.patch.stopall)@mock.patch.object(mod, 'Klass', autospec=True) # :(
def testFoo(self, mock_klass):
# mod.Klass has no effective spec.
- Deriving an autospec Mock from an already-mocked object
def setUp(self):
mock.patch.object(mod, 'Klass').start()
...
mock_klass = mock.create_autospec(mod.Klass) # :(
# mock_klass has no effective specThis is fairly easy to detect using _is_instance_mock at patch time however it can break existing tests.
I have a patch ready and it seems like this error case is not frequent enough that it would be disruptive to address.
Another option would be add it as a warning for a version e.g. 3.10 and then potentially make it a breaking change in 3.11. However considering this is a test-only change with a fairly clear path to fix it, that might be overly cautious. gpshead commented on Mar 15, 2021 Lets go ahead and try making this a breaking change in 3.10. If users report it causes a bunch of problems during the beta -that they don't want to address so soon- (they are all likely bugs in test suites...) we can soften it to a warning for a cycle. My hope is that we won't need to do that. tirkarthi commented on Mar 15, 2021 For a simple experiment raising an exception I can see two tests failing in test suite that have the pattern of having an autospec which is a mock object.
diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 720f682efb..d33c7899a1 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -2612,6 +2612,9 @@ def create_autospec(spec, spec_set=False, instance=False, _parent=None,
# interpreted as a list of strings
spec = type(spec)
+ if _is_instance_mock(spec):
+ 1 / 0
+
is_type = isinstance(spec, type)
is_async_func = _is_async_func(spec)
_kwargs = {'spec': spec}./python -m unittest Lib.unittest.test.testmock
....................................................................................................................................................E..............................................................................................................................................................E...........................................................................................................................................................................................
======================================================================
ERROR: test_bool_not_called_when_passing_spec_arg (unittest.test.testmock.testmock.MockTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/root/cpython/Lib/unittest/test/testmock/testmock.py", line 2180, in test_bool_not_called_when_passing_spec_arg
with unittest.mock.patch.object(obj, 'obj_with_bool_func', autospec=True): pass
File "/root/cpython/Lib/unittest/mock.py", line 1503, in __enter__
new = create_autospec(autospec, spec_set=spec_set,
File "/root/cpython/Lib/unittest/mock.py", line 2616, in create_autospec
1 / 0
ZeroDivisionError: division by zero======================================================================
ERROR: test_create_autospec_awaitable_class (unittest.test.testmock.testasync.AsyncAutospecTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/root/cpython/Lib/unittest/test/testmock/testasync.py", line 204, in test_create_autospec_awaitable_class
self.assertIsInstance(create_autospec(awaitable_mock), AsyncMock)
File "/root/cpython/Lib/unittest/mock.py", line 2616, in create_autospec
1 / 0
ZeroDivisionError: division by zero
Ran 495 tests in 2.039s
FAILED (errors=2) cjw296 commented on Mar 15, 2021 I agree that this should raise an exception.
Can the two failing tests in mock's own suite be easily fixed? msuozzo commented on Mar 15, 2021 MannequinAuthor More actions I've fixed a bunch of these in our internal repo so I'd be happy to add that to a patch implementing raising exceptions for these cases. tirkarthi commented on Mar 15, 2021 The tests can be fixed. The change to raise exception can be merged to gather feedback during alpha/beta and see if there are any valid usecases. msuozzo commented on Mar 16, 2021 MannequinAuthor More actions A few more things:
Assertions on Mock-autospec'ed Mocks will silently pass since e.g. assert_called_once_with will now be mocked out. This may justify a more stringent stance on the pattern since it risks hiding real test failures.
One complicating factor with the implementation is that autospec'd objects may have children that are already Mocked out. Failing eagerly, however, would break cases where the child is never used (and consequently risk causing more friction than may be necessary) but failing only upon access of that child makes it trickier for the user to trace back to where the parent was mocked. msuozzo commented on Mar 16, 2021 MannequinAuthor More actions gpshead commented on Apr 10, 2021 pablogsal commented on Apr 10, 2021 cmaloney commented on Nov 7, 2025 This looks fully implemented; can the issue be closed?
MetadataMetadataStatusShow more project fields
DevelopmentNo branches or pull requestsIssue actions
|