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).

1 files changed, 38 insertions, 4 deletions

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 720f682..c606715 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -36,6 +36,10 @@ from unittest.util import safe_repr
 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 @@ class NonCallableMock(Base):
             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 @@ class _patch(object):
                 )
         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 @@ class _patch(object):
             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: