Skip to content

gh-104745: Limit starting a patcher more than once without stopping it #126649

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.

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
merged 11 commits into from
Nov 13, 2024
52 changes: 50 additions & 2 deletions Lib/test/test_unittest/testmock/testpatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,54 @@ def test_stop_idempotent(self):
self.assertIsNone(patcher.stop())


def test_exit_idempotent(self):
patcher = patch(foo_name, 'bar', 3)
with patcher:
patcher.stop()


def test_second_start_failure(self):
patcher = patch(foo_name, 'bar', 3)
patcher.start()
try:
self.assertRaises(RuntimeError, patcher.start)
finally:
patcher.stop()


def test_second_enter_failure(self):
patcher = patch(foo_name, 'bar', 3)
with patcher:
self.assertRaises(RuntimeError, patcher.start)


def test_second_start_after_stop(self):
patcher = patch(foo_name, 'bar', 3)
patcher.start()
patcher.stop()
patcher.start()
patcher.stop()


def test_property_setters(self):
mock_object = Mock()
mock_bar = mock_object.bar
patcher = patch.object(mock_object, 'bar', 'x')
with patcher:
self.assertEqual(patcher.is_local, False)
self.assertIs(patcher.target, mock_object)
self.assertEqual(patcher.temp_original, mock_bar)
patcher.is_local = True
patcher.target = mock_bar
patcher.temp_original = mock_object
self.assertEqual(patcher.is_local, True)
self.assertIs(patcher.target, mock_bar)
self.assertEqual(patcher.temp_original, mock_object)
# if changes are left intact, they may lead to disruption as shown below (it might be what someone needs though)
self.assertEqual(mock_bar.bar, mock_object)
self.assertEqual(mock_object.bar, 'x')


def test_patchobject_start_stop(self):
original = something
patcher = patch.object(PTModule, 'something', 'foo')
Expand Down Expand Up @@ -1098,7 +1146,7 @@ def test_new_callable_patch(self):

self.assertIsNot(m1, m2)
for mock in m1, m2:
self.assertNotCallable(m1)
self.assertNotCallable(mock)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice spot!



def test_new_callable_patch_object(self):
Expand All @@ -1111,7 +1159,7 @@ def test_new_callable_patch_object(self):

self.assertIsNot(m1, m2)
for mock in m1, m2:
self.assertNotCallable(m1)
self.assertNotCallable(mock)


def test_new_callable_keyword_arguments(self):
Expand Down
9 changes: 9 additions & 0 deletions Lib/unittest/mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -1360,6 +1360,7 @@ def __init__(
self.autospec = autospec
self.kwargs = kwargs
self.additional_patchers = []
self.is_started = False


def copy(self):
Expand Down Expand Up @@ -1472,6 +1473,9 @@ def get_original(self):

def __enter__(self):
"""Perform the patch."""
if self.is_started:
raise RuntimeError("Patch is already started")

new, spec, spec_set = self.new, self.spec, self.spec_set
autospec, kwargs = self.autospec, self.kwargs
new_callable = self.new_callable
Expand Down Expand Up @@ -1603,6 +1607,7 @@ def __enter__(self):
self.temp_original = original
self.is_local = local
self._exit_stack = contextlib.ExitStack()
self.is_started = True
try:
setattr(self.target, self.attribute, new_attr)
if self.attribute_name is not None:
Expand All @@ -1622,6 +1627,9 @@ def __enter__(self):

def __exit__(self, *exc_info):
"""Undo the patch."""
if not self.is_started:
return

if self.is_local and self.temp_original is not DEFAULT:
setattr(self.target, self.attribute, self.temp_original)
else:
Expand All @@ -1638,6 +1646,7 @@ def __exit__(self, *exc_info):
del self.target
exit_stack = self._exit_stack
del self._exit_stack
self.is_started = False
return exit_stack.__exit__(*exc_info)


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Limit starting a patcher (from :func:`unittest.mock.patch` or
:func:`unittest.mock.patch.object`) more than
once without stopping it
Loading