From: Matthew Suozzo <msuozzo@google.com>
Date: Thu, 3 Feb 2022 08:41:19 +0000 (-0500)
Subject: Restrict use of Mock objects as specs (GH-31090)
X-Git-Tag: v3.11.0a5~9
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6394e981adaca2c0daa36c8701611e250d74024c;p=thirdparty%2FPython%2Fcpython.git

Restrict use of Mock objects as specs (GH-31090)

Follow-on to https://github.com/python/cpython/pull/25326

This covers cases where mock objects are passed directly to spec.
---

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 913750193000..2719f74d6fca 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -489,6 +489,9 @@ class NonCallableMock(Base):
 
     def _mock_add_spec(self, spec, spec_set, _spec_as_instance=False,
                        _eat_self=False):
+        if _is_instance_mock(spec):
+            raise InvalidSpecError(f'Cannot spec a Mock object. [object={spec!r}]')
+
         _spec_class = None
         _spec_signature = None
         _spec_asyncs = []
@@ -2789,6 +2792,7 @@ FunctionTypes = (
 
 
 file_spec = None
+open_spec = None
 
 
 def _to_stream(read_data):
@@ -2845,8 +2849,12 @@ def mock_open(mock=None, read_data=''):
         import _io
         file_spec = list(set(dir(_io.TextIOWrapper)).union(set(dir(_io.BytesIO))))
 
+    global open_spec
+    if open_spec is None:
+        import _io
+        open_spec = list(set(dir(_io.open)))
     if mock is None:
-        mock = MagicMock(name='open', spec=open)
+        mock = MagicMock(name='open', spec=open_spec)
 
     handle = MagicMock(spec=file_spec)
     handle.__enter__.return_value = handle
diff --git a/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py
index fdba543b5351..c99098dc4ea8 100644
--- a/Lib/unittest/test/testmock/testmock.py
+++ b/Lib/unittest/test/testmock/testmock.py
@@ -226,6 +226,14 @@ class MockTest(unittest.TestCase):
             with self.assertRaisesRegex(InvalidSpecError,
                                         "Cannot spec attr 'B' as the spec_set "):
                 mock.patch.object(A, 'B', spec_set=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()
+            with self.assertRaisesRegex(InvalidSpecError, "Cannot spec a Mock object."):
+                mock.Mock(A.B)
+            with mock.patch('builtins.open', mock.mock_open()):
+                mock.mock_open()  # should still be valid with open() mocked
+
 
     def test_reset_mock(self):
         parent = Mock()
diff --git a/Lib/unittest/test/testmock/testwith.py b/Lib/unittest/test/testmock/testwith.py
index 42ebf3898c89..c74d49a63c89 100644
--- a/Lib/unittest/test/testmock/testwith.py
+++ b/Lib/unittest/test/testmock/testwith.py
@@ -130,8 +130,8 @@ class WithTest(unittest.TestCase):
 
         c = C()
 
-        with patch.object(c, 'f', autospec=True) as patch1:
-            with patch.object(c, 'f', autospec=True) as patch2:
+        with patch.object(c, 'f') as patch1:
+            with patch.object(c, 'f') as patch2:
                 c.f()
             self.assertEqual(patch2.call_count, 1)
             self.assertEqual(patch1.call_count, 0)
diff --git a/Misc/NEWS.d/next/Tests/2022-02-03-00-21-32.bpo-43478.0nfcam.rst b/Misc/NEWS.d/next/Tests/2022-02-03-00-21-32.bpo-43478.0nfcam.rst
new file mode 100644
index 000000000000..7c8fc47cfc75
--- /dev/null
+++ b/Misc/NEWS.d/next/Tests/2022-02-03-00-21-32.bpo-43478.0nfcam.rst
@@ -0,0 +1 @@
+Mocks can no longer be provided as the specs for other Mocks. As a result, an already-mocked object cannot be passed to `mock.Mock()`. 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.