Create a safeguard module to import stdlib objects from

[nicoddemus]

The rationale here being that users sometimes monkeypatch functions and objects from the standard library which can break pytest itself.

@RonnyPfannschmidt's idea from #3288 (comment).

Refs: #3288, #2180, #3352

[nicoddemus]

To elaborate, the idea is that in normal pytest modules we never import from stdlib or external modules directly, but from a safeimp.py instead which imports all required symbols when pytest starts.

The issue is that sometimes people will monkey patch some stdlib module or builtin function, and this breaks pytest internals.

For example some pytest module (say python.py) might have this import:

import os

    # down the module...
    os.environ['PYTEST_CURRENT_TEST'] = node.id

Now if a user during a test does something like this:

import os
def test(monkeypatch):
    monkeypatch.setattr(os, 'environ', 1)

It will break pytest internally because os.environ will be 1 instead of a dict, which will break the code in python.py.

That is the problem, the solution @RonnyPfannschmidt suggests is to import all symbols we require early in a safeimp.py module:

# safeimp.py
from os import environ

So the code in python.py is changed to:

from .safeimp import environ

    # down the module...
    environ['PYTEST_CURRENT_TEST'] = node.id

So we get the original os.environ even if the user messes with it during the tests.

You might start with looking at all imports from the stdlib (like os.environ above) and move them to this new safeimp.py file. And don't worry if we miss some, it is then a matter of updating it with new symbols in the future, the important thing is to have this mechanism in-place.

[brianmaissy]

Wouldn't this damage the readability of the code, because we would always have to refer to stdlib objects without their module?

[nicoddemus]

@brianmaissy Definitely it would, and it would be a hassle to enforce that without a linter. 😬

[brianmaissy]

Then I wonder if it's worth the bother. How common is the use case of someone monkeypatching the standard library so badly that it doesn't behave like the standard library anymore? I sort feel like you deserve what you get if you make os.environ something that doesn't behave reasonably like a mapping.

[feuillemorte]

@nicoddemus

Seems, it doesn't help or I doing smth wrong :(

test_1.py:

import functools

from unittest.mock import Mock


def test_partial(monkeypatch):
    monkeypatch.setattr(functools, "partial", Mock())
    assert functools.partial.call_count == 1

safeguarded_function.py:

import functools

compat.py:

from _pytest import safeguarded_function as sf
......
if isinstance(obj, sf.functools.partial):
....

output:

platform linux -- Python 3.6.4, pytest-3.5.1.dev26+gad0b4330.d20180404, py-1.5.3, pluggy-0.6.0
rootdir: /home/feuillemorte/github/pytest-dev/pytest/pytest_3290, inifile: tox.ini
collected 1 item                                                                                                                                                                       

tests/test_1.py 
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/main.py", line 107, in wrap_session
INTERNALERROR>     session.exitstatus = doit(config, session) or 0
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/main.py", line 145, in _main
INTERNALERROR>     config.hook.pytest_runtestloop(session=session)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/__init__.py", line 617, in __call__
INTERNALERROR>     return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/__init__.py", line 222, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/__init__.py", line 216, in <lambda>
INTERNALERROR>     firstresult=hook.spec_opts.get('firstresult'),
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/callers.py", line 201, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/callers.py", line 76, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/callers.py", line 180, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/main.py", line 168, in pytest_runtestloop
INTERNALERROR>     item.config.hook.pytest_runtest_protocol(item=item, nextitem=nextitem)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/__init__.py", line 617, in __call__
INTERNALERROR>     return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/__init__.py", line 222, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/__init__.py", line 216, in <lambda>
INTERNALERROR>     firstresult=hook.spec_opts.get('firstresult'),
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/callers.py", line 201, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/callers.py", line 76, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/callers.py", line 180, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/runner.py", line 62, in pytest_runtest_protocol
INTERNALERROR>     runtestprotocol(item, nextitem=nextitem)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/runner.py", line 79, in runtestprotocol
INTERNALERROR>     reports.append(call_and_report(item, "call", log))
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/runner.py", line 160, in call_and_report
INTERNALERROR>     report = hook.pytest_runtest_makereport(item=item, call=call)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/__init__.py", line 617, in __call__
INTERNALERROR>     return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/__init__.py", line 222, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/__init__.py", line 216, in <lambda>
INTERNALERROR>     firstresult=hook.spec_opts.get('firstresult'),
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/callers.py", line 196, in _multicall
INTERNALERROR>     gen.send(outcome)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/skipping.py", line 117, in pytest_runtest_makereport
INTERNALERROR>     rep = outcome.get_result()
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/callers.py", line 76, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pluggy-0.6.0-py3.6.egg/pluggy/callers.py", line 180, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/runner.py", line 312, in pytest_runtest_makereport
INTERNALERROR>     longrepr = item.repr_failure(excinfo)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/python.py", line 598, in repr_failure
INTERNALERROR>     return self._repr_failure_py(excinfo, style=style)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/python.py", line 591, in _repr_failure_py
INTERNALERROR>     style=style)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/nodes.py", line 229, in _repr_failure_py
INTERNALERROR>     self._prunetraceback(excinfo)
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/python.py", line 567, in _prunetraceback
INTERNALERROR>     code = _pytest._code.Code(get_real_func(self.obj))
INTERNALERROR>   File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3290/env/lib/python3.6/site-packages/pytest-3.5.1.dev26+gad0b4330.d20180404-py3.6.egg/_pytest/compat.py", line 214, in get_real_func
INTERNALERROR>     if isinstance(obj, sf.functools.partial):
INTERNALERROR> TypeError: isinstance() arg 2 must be a type or tuple of types

[brianmaissy]

@feuillemorte You need to import partial itself in safeguarded_function, not justfunctools. You are monkeypatching the same module which you have already imported.

[feuillemorte]

@brianmaissy I see, thank you!

[twmr]

Note that this safeimpl approach fails when tests mock methods/attributes of classes from the stdlibrary!

[nicoddemus]

@Thisch that's true, thanks for pointing it out. If anybody has any other suggestions they are welcome though, the outline above is what I got from @RonnyPfannschmidt's suggestion.