Guidance on "un-spying" a method

[melvinkcx]

Hi, I couldn't quite figure out how to un-spy a spied method yet. I'm not sure if it's even possible after browsing through the source code.

spy_foo = mocker.spy(module, "foo")

# do something

# how to un-spy `spy_foo`?

Any ideas or suggestions on how to achieve it? 🙏

[nicoddemus]

Hi @melvinkcx,

The only way right now is to call mocker.stopall(), but this will of course undo all others mocks for the test.

[melvinkcx]

Thanks for replying. Undoing all other mocks is the unintended side-effect here. Any thoughts on supporting this in the future? I'm happy to work on this, but any guides on achieving it will be greatly appreciated too =)

[nicoddemus]

No plans, but I'm not sure what a good API for it would be... suggestions welcome!

[melvinkcx]

Could this be an option? It is along the line with .reset_mock(), I believe

spy_foo = mocker.spy(module, "foo")
# do something
spy_foo.stop()  # introduce .stop() ?

[nicoddemus]

No because spy_foo is a MagicMock, so it is possible for it to be mocking an object that contains a stop() method. 😕

[melvinkcx]

Right, you're utterly right.

The only alternative I can think of so far is:

spy_foo = mocker.spy(module, "foo")

mocker.stop(spy_foo)

I think this is quite possible given that we keep track of all mocks in MockerFixture()._mocks?

[nicoddemus]

Ahh that's a great idea!

That would even work for any mock, not only spy. 😁

Would you like to work on a PR for that?

[melvinkcx]

I was looking into the implementation of MockerFixture, and what happens internally when we call spy, patch, etc.

The method _start_patch() appends the patcher object and the mocked object into 2 separate lists internally, but it doesn't seem like we can easily associate the objects in MockedFixtures()._mocks with their respective patchers.

I'm not too familiar with the code base, of course, please correct me if I'm wrong.

It seemed to me that, unless we introduce a new internal state to track the associations of patcher and mocked objects, we cannot support this:

spy_foo = mocker.spy(module, "foo")
mocker.stop(spy_foo)

Let me know what you think =)

[nicoddemus]

Hi @melvinkcx,

I see, you are right.

I think we can merge the two lists into one, a list of pairs (patcher, mock), so we can associate each mock with their patcher.

I played around with the idea and it works well:

diff --git a/src/pytest_mock/plugin.py b/src/pytest_mock/plugin.py
index 088e5ce..1b5d4c7 100644
--- a/src/pytest_mock/plugin.py
+++ b/src/pytest_mock/plugin.py
@@ -39,11 +39,10 @@ class MockerFixture:
     """

     def __init__(self, config: Any) -> None:
-        self._patches = []  # type: List[Any]
-        self._mocks = []  # type: List[Any]
+        self._patches_and_mocks = []
         self.mock_module = mock_module = get_mock_module(config)
         self.patch = self._Patcher(
-            self._patches, self._mocks, mock_module
+            self._patches_and_mocks, mock_module
         )  # type: MockerFixture._Patcher
         # aliases for convenience
         self.Mock = mock_module.Mock
@@ -76,8 +75,10 @@ class MockerFixture:
         else:
             supports_reset_mock_with_args = (self.Mock,)

-        for m in self._mocks:
+        for p, m in self._patches_and_mocks:
             # See issue #237.
+            if not hasattr(m, "reset_mock"):
+                continue
             if isinstance(m, supports_reset_mock_with_args):
                 m.reset_mock(return_value=return_value, side_effect=side_effect)
             else:
@@ -88,10 +89,18 @@ class MockerFixture:
         Stop all patchers started by this fixture. Can be safely called multiple
         times.
         """
-        for p in reversed(self._patches):
+        for p, m in reversed(self._patches_and_mocks):
             p.stop()
-        self._patches[:] = []
-        self._mocks[:] = []
+        self._patches_and_mocks.clear()
+
+
+    def stop(self, mock) -> None:
+        for index, (p, m) in enumerate(self._patches_and_mocks):
+            if mock is m:
+                p.stop()
+                del self._patches_and_mocks[index]
+                break
+

     def spy(self, obj: object, name: str) -> unittest.mock.MagicMock:
         """
@@ -167,9 +176,8 @@ class MockerFixture:

         DEFAULT = object()

-        def __init__(self, patches, mocks, mock_module):
-            self._patches = patches
-            self._mocks = mocks
+        def __init__(self, patches_and_mocks, mock_module):
+            self.__patches_and_mocks = patches_and_mocks
             self.mock_module = mock_module

         def _start_patch(
@@ -181,9 +189,8 @@ class MockerFixture:
             """
             p = mock_func(*args, **kwargs)
             mocked = p.start()  # type: unittest.mock.MagicMock
-            self._patches.append(p)
+            self.__patches_and_mocks.append((p, mocked))
             if hasattr(mocked, "reset_mock"):
-                self._mocks.append(mocked)
                 # check if `mocked` is actually a mock object, as depending on autospec or target
                 # parameters `mocked` can be anything
                 if hasattr(mocked, "__enter__") and warn_on_mock_enter:

Here are two tests which demonstrate that this works:

def test_stop_patch(mocker):

    class C:
        def foo(self):
            return 42

    m = mocker.patch.object(C, "foo", return_value=0)
    assert C().foo() == 0
    mocker.stop(m)
    assert C().foo() == 42


def test_stop_spy(mocker):

    class C:
        def foo(self):
            return 42

    spy = mocker.spy(C, "foo")
    assert C().foo() == 42
    assert spy.call_count == 1
    mocker.stop(spy)
    assert C().foo() == 42
    assert spy.call_count == 1

The code of course needs documentation, types, and perhaps a few more tests.

[melvinkcx]

Hi, thanks for testing the idea out. How do you think we can move forward further with this? Perhaps, I can spare some time to contribute more on this

[nicoddemus]

How do you think we can move forward further with this? Perhaps, I can spare some time to contribute more on this

Yeah it would be a great opportunity to contribute if you like.

[sgaist]

Hi,

Since it is still open, I would like to implement it for Hacktober fest. Can I be assigned ?