Skip to content

Fix RuntimeError if user code calls patcher.stopall #138

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 1 commit into from
Mar 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
1.10.2
------

* Fix bug at the end of the test session when a call to ``patch.stopall`` is done explicitly by
user code. Thanks `@craiga`_ for the report (`#137`_).

.. _#137: https://github.com/pytest-dev/pytest-mock/issues/137
.. _@craiga: https://github.com/craiga

1.10.1
------

Expand Down
12 changes: 11 additions & 1 deletion pytest_mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,17 @@ def wrap_assert_methods(config):

def unwrap_assert_methods():
for patcher in _mock_module_patches:
patcher.stop()
try:
patcher.stop()
except RuntimeError as e:
# a patcher might have been stopped by user code (#137)
# so we need to catch this error here and ignore it;
# unfortunately there's no public API to check if a patch
# has been started, so catching the error it is
if text_type(e) == "stop called on unstarted patcher":
pass
else:
raise
Copy link

Choose a reason for hiding this comment

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

Thanks heaps for this, but I'm concerned that this might obfuscate other errors.

What would happen if some test called .stopall(), then some other test which relied on pytest-mock's internal mocks was called. Would it be clear what was going on?

Copy link

Choose a reason for hiding this comment

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

This might be a separate issue. It's possible to get into this situation as-is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the review!

I don't think so, as we are not catching any RuntimeError, we only ignore the very specific message... as I commented above, I think this is safe, but might break in future mock releases if they ever change the message. We should also switch to a public API that allows us to check if a patcher has already been stopped, if one is ever introduced.

_mock_module_patches[:] = []
_mock_module_originals.clear()

Expand Down
21 changes: 21 additions & 0 deletions test_pytest_mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -610,3 +610,24 @@ def test_assert_called_with_unicode_arguments(mocker):

with pytest.raises(AssertionError):
stub.assert_called_with(u"lak")


def test_plain_stopall(testdir):
"""Calling patch.stopall() in a test would cause an error during unconfigure (#137)"""
testdir.makepyfile(
"""
import random

def get_random_number():
return random.randint(0, 100)

def test_get_random_number(mocker):
patcher = mocker.mock_module.patch("random.randint", lambda x, y: 5)
patcher.start()
assert get_random_number() == 5
mocker.mock_module.patch.stopall()
"""
)
result = testdir.runpytest_subprocess()
result.stdout.fnmatch_lines("* 1 passed in *")
assert "RuntimeError" not in result.stderr.str()
3 changes: 3 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,6 @@ usedevelop = True
extras = dev
basepython = python3.6
commands = pre-commit run --all-files --show-diff-on-failure

[pytest]
addopts = -ra