Skip to content

bpo-43478: Restrict use of Mock objects as specs #31090

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gpshead merged 1 commit into from
Feb 3, 2022
Merged

bpo-43478: Restrict use of Mock objects as specs #31090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion Lib/unittest/mock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down Expand Up @@ -2789,6 +2792,7 @@ def __init__(self, spec, spec_set=False, parent=None,


file_spec = None
open_spec = None


def _to_stream(read_data):
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions Lib/unittest/test/testmock/testmock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Comment thread
msuozzo marked this conversation as resolved.
Outdated
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()
Expand Down
4 changes: 2 additions & 2 deletions Lib/unittest/test/testmock/testwith.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Tests/2022-02-03-00-21-32.bpo-43478.0nfcam.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.