[Python-checkins] Restrict use of Mock objects as specs (GH-31090)

[gpshead]

https://github.com/python/cpython/commit/6394e981adaca2c0daa36c8701611e250d74024c
commit: 6394e981adaca2c0daa36c8701611e250d74024c
branch: main
author: Matthew Suozzo <msuozzo at google.com>
committer: gpshead <greg at krypto.org>
date: 2022-02-03T00:41:19-08:00
summary:

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.

files:
A Misc/NEWS.d/next/Tests/2022-02-03-00-21-32.bpo-43478.0nfcam.rst
M Lib/unittest/mock.py
M Lib/unittest/test/testmock/testmock.py
M Lib/unittest/test/testmock/testwith.py

diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 9137501930000..2719f74d6fca5 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -489,6 +489,9 @@ def mock_add_spec(self, spec, spec_set=False):
 
     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 @@ def __init__(self, spec, spec_set=False, parent=None,
 
 
 file_spec = None
+open_spec = None
 
 
 def _to_stream(read_data):
@@ -2845,8 +2849,12 @@ def _next_side_effect():
         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 fdba543b53511..c99098dc4ea86 100644
--- a/Lib/unittest/test/testmock/testmock.py
+++ b/Lib/unittest/test/testmock/testmock.py
@@ -226,6 +226,14 @@ class B(object):
             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 42ebf3898c89e..c74d49a63c892 100644
--- a/Lib/unittest/test/testmock/testwith.py
+++ b/Lib/unittest/test/testmock/testwith.py
@@ -130,8 +130,8 @@ def f(self): pass
 
         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 0000000000000..7c8fc47cfc751
--- /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.