[Python-checkins] bpo-43478: Restrict use of Mock objects as specs (GH-25326)
gpshead
webhook-mailer at python.org
Fri Apr 9 23:45:58 EDT 2021
https://github.com/python/cpython/commit/dccdc500f9b5dab0a20407ae0178d393796a8828
commit: dccdc500f9b5dab0a20407ae0178d393796a8828
branch: master
author: Matthew Suozzo <msuozzo at google.com>
committer: gpshead <greg at krypto.org>
date: 2021-04-09T20:45:50-07:00
summary:
bpo-43478: Restrict use of Mock objects as specs (GH-25326)
* Restrict using Mock objects as specs as this is always a test bug where the resulting mock is misleadingly useless.
* Skip a broken test that exposes a bug elsewhere in mock (noted in the original issue).
files:
A Misc/NEWS.d/next/Library/2021-04-10-03-30-36.bpo-43478.iZcBTq.rst
M Lib/unittest/mock.py
M Lib/unittest/test/testmock/testasync.py
M Lib/unittest/test/testmock/testmock.py
diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 720f682efbb54..c6067151de14f 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -36,6 +36,10 @@
from functools import wraps, partial
+class InvalidSpecError(Exception):
+ """Indicates that an invalid value was used as a mock spec."""
+
+
_builtins = {name for name in dir(builtins) if not name.startswith('_')}
FILTER_DIR = True
@@ -653,10 +657,17 @@ def __getattr__(self, name):
self._mock_children[name] = result
elif isinstance(result, _SpecState):
- result = create_autospec(
- result.spec, result.spec_set, result.instance,
- result.parent, result.name
- )
+ try:
+ result = create_autospec(
+ result.spec, result.spec_set, result.instance,
+ result.parent, result.name
+ )
+ except InvalidSpecError:
+ target_name = self.__dict__['_mock_name'] or self
+ raise InvalidSpecError(
+ f'Cannot autospec attr {name!r} from target '
+ f'{target_name!r} as it has already been mocked out. '
+ f'[target={self!r}, attr={result.spec!r}]')
self._mock_children[name] = result
return result
@@ -1273,6 +1284,14 @@ def __init__(
)
if not unsafe:
_check_spec_arg_typos(kwargs)
+ if _is_instance_mock(spec):
+ raise InvalidSpecError(
+ f'Cannot spec attr {attribute!r} as the spec '
+ f'has already been mocked out. [spec={spec!r}]')
+ if _is_instance_mock(spec_set):
+ raise InvalidSpecError(
+ f'Cannot spec attr {attribute!r} as the spec_set '
+ f'target has already been mocked out. [spec_set={spec_set!r}]')
self.getter = getter
self.attribute = attribute
@@ -1500,6 +1519,18 @@ def __enter__(self):
if autospec is True:
autospec = original
+ if _is_instance_mock(self.target):
+ raise InvalidSpecError(
+ f'Cannot autospec attr {self.attribute!r} as the patch '
+ f'target has already been mocked out. '
+ f'[target={self.target!r}, attr={autospec!r}]')
+ if _is_instance_mock(autospec):
+ target_name = getattr(self.target, '__name__', self.target)
+ raise InvalidSpecError(
+ f'Cannot autospec attr {self.attribute!r} from target '
+ f'{target_name!r} as it has already been mocked out. '
+ f'[target={self.target!r}, attr={autospec!r}]')
+
new = create_autospec(autospec, spec_set=spec_set,
_name=self.attribute, **kwargs)
elif kwargs:
@@ -2613,6 +2644,9 @@ def create_autospec(spec, spec_set=False, instance=False, _parent=None,
spec = type(spec)
is_type = isinstance(spec, type)
+ if _is_instance_mock(spec):
+ raise InvalidSpecError(f'Cannot autospec a Mock object. '
+ f'[object={spec!r}]')
is_async_func = _is_async_func(spec)
_kwargs = {'spec': spec}
if spec_set:
diff --git a/Lib/unittest/test/testmock/testasync.py b/Lib/unittest/test/testmock/testasync.py
index 690ca4f55f9e3..e1866a3492cb5 100644
--- a/Lib/unittest/test/testmock/testasync.py
+++ b/Lib/unittest/test/testmock/testasync.py
@@ -199,9 +199,9 @@ def test_create_autospec_instance(self):
with self.assertRaises(RuntimeError):
create_autospec(async_func, instance=True)
+ @unittest.skip('Broken test from https://bugs.python.org/issue37251')
def test_create_autospec_awaitable_class(self):
- awaitable_mock = create_autospec(spec=AwaitableClass())
- self.assertIsInstance(create_autospec(awaitable_mock), AsyncMock)
+ self.assertIsInstance(create_autospec(AwaitableClass), AsyncMock)
def test_create_autospec(self):
spec = create_autospec(async_func_args)
diff --git a/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py
index e38f41e1d2152..fdba543b53511 100644
--- a/Lib/unittest/test/testmock/testmock.py
+++ b/Lib/unittest/test/testmock/testmock.py
@@ -11,7 +11,7 @@
call, DEFAULT, patch, sentinel,
MagicMock, Mock, NonCallableMock,
NonCallableMagicMock, AsyncMock, _Call, _CallList,
- create_autospec
+ create_autospec, InvalidSpecError
)
@@ -205,6 +205,28 @@ def f(): pass
self.assertRaisesRegex(ValueError, 'Bazinga!', mock)
+ def test_autospec_mock(self):
+ class A(object):
+ class B(object):
+ C = None
+
+ with mock.patch.object(A, 'B'):
+ with self.assertRaisesRegex(InvalidSpecError,
+ "Cannot autospec attr 'B' from target <MagicMock spec='A'"):
+ create_autospec(A).B
+ with self.assertRaisesRegex(InvalidSpecError,
+ "Cannot autospec attr 'B' from target 'A'"):
+ mock.patch.object(A, 'B', autospec=True).start()
+ with self.assertRaisesRegex(InvalidSpecError,
+ "Cannot autospec attr 'C' as the patch target "):
+ mock.patch.object(A.B, 'C', autospec=True).start()
+ with self.assertRaisesRegex(InvalidSpecError,
+ "Cannot spec attr 'B' as the spec "):
+ mock.patch.object(A, 'B', spec=A.B).start()
+ with self.assertRaisesRegex(InvalidSpecError,
+ "Cannot spec attr 'B' as the spec_set "):
+ mock.patch.object(A, 'B', spec_set=A.B).start()
+
def test_reset_mock(self):
parent = Mock()
spec = ["something"]
@@ -2177,7 +2199,7 @@ def __init__(self):
self.obj_with_bool_func = unittest.mock.MagicMock()
obj = Something()
- with unittest.mock.patch.object(obj, 'obj_with_bool_func', autospec=True): pass
+ with unittest.mock.patch.object(obj, 'obj_with_bool_func', spec=object): pass
self.assertEqual(obj.obj_with_bool_func.__bool__.call_count, 0)
diff --git a/Misc/NEWS.d/next/Library/2021-04-10-03-30-36.bpo-43478.iZcBTq.rst b/Misc/NEWS.d/next/Library/2021-04-10-03-30-36.bpo-43478.iZcBTq.rst
new file mode 100644
index 0000000000000..aaa1992f07fb4
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2021-04-10-03-30-36.bpo-43478.iZcBTq.rst
@@ -0,0 +1 @@
+Mocks can no longer be used as the specs for other Mocks. As a result, an already-mocked object cannot have an attribute mocked using `autospec=True` or be the subject of a `create_autospec(...)` call. This can uncover bugs in tests since these Mock-derived Mocks will always pass certain tests (e.g. isinstance) and builtin assert functions (e.g. assert_called_once_with) will unconditionally pass.
\ No newline at end of file
More information about the Python-checkins
mailing list