Create a safeguard module to import stdlib objects from #3290
Comments
nicoddemus
added
type: bug
|
To elaborate, the idea is that in normal pytest modules we never import from stdlib or external modules directly, but from a 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 import os
# down the module...
os.environ['PYTEST_CURRENT_TEST'] = node.idNow 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 That is the problem, the solution @RonnyPfannschmidt suggests is to import all symbols we require early in a # safeimp.py
from os import environSo the code in from .safeimp import environ
# down the module...
environ['PYTEST_CURRENT_TEST'] = node.idSo we get the original You might start with looking at all imports from the stdlib (like |
|
Wouldn't this damage the readability of the code, because we would always have to refer to stdlib objects without their module? |
|
@brianmaissy Definitely it would, and it would be a hassle to enforce that without a linter. 😬 |
|
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. |
|
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 == 1safeguarded_function.py: import functoolscompat.py: from _pytest import safeguarded_function as sf
......
if isinstance(obj, sf.functools.partial):
....output: |
|
@feuillemorte You need to import |
|
@brianmaissy I see, thank you! |
|
Note that this safeimpl approach fails when tests mock methods/attributes of classes from the stdlibrary! |
|
@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. |
|
we can definitively pick a service level for patching the stdlib tho
|
You got a point and I kinda agree with you. The problem is that people sometimes do it (for legitimate testing reasons), and they waste a lot of time trying to figure out what happened. If we can save those users time by implementing some safe-guard, I think it is worth doing it. My 2 cents. |
|
What if I import it like from functools import partial as functools_partial
from inspect import isclass as inspect_isclass, CO_VARARGS as inspect_CO_VARARGS, \
CO_VARKEYWORDS as inspect_CO_VARKEYWORDS
from sys import exc_info as sys_exc_infothen this will easier to understand what is it: from _pytest.safeguarded_function import functools_partial
.....
if isinstance(obj, functools_partial):
obj = obj.func
return objBecause it's really hard to understand what I import: from _pytest.safeguarded_function import isclass, CO_VARARGS, CO_VARKEYWORDS, exc_info, ref, ib, Factory, sAlso, file from functools import \
partial as functools_partial
from inspect import \
isclass as inspect_isclass,\
CO_VARARGS as inspect_CO_VARARGS,\
CO_VARKEYWORDS as inspect_CO_VARKEYWORDS
from sys import \
exc_info as sys_exc_info
from attr import \
ib as attr_ib, \
s as attr_s, \
Factory as attr_Factory
from weakref import \
ref as weakref_ref
from re import \
search as re_search |
|
Looking at the implementation so far, I'm starting to have second thoughts about this myself because it seems we will be adding a lot of overhead to our development. Let's backtrack a little, to recap: #3288: Except for #2180, the others could be fixed by using If the above is true, we could instead improve Something like: with monkeypatch.context() as m:
m.setattr(functools, "partial", Mock())( |
|
i agree - lets document that monkeypatching the stdlib without care can and will break pytest |
|
Sounds good, and what do you think about the suggestions I made above? |
|
the "i agree" was targetted at those, its a good starting point and i believe with a few iterations we will arrive at something fabulous |
class MonkeyPatch(object):
def __enter__(self):
print('HEY OPEN!')
def __exit__(self, exc_type, exc_val, exc_tb):
print('HEY CLOSE!')def test_partial(monkeypatch):
with monkeypatch as m:
m.setattr(functools, "partial", Mock())
assert functools.partial.call_count == 1output: But how to create another namespace for with statement? I don't understand how i can save |
|
the new namespace is the return value - the monkeypatcher nesting should be done as a separate feature tool and should be used for the capture plugin as well |
|
Merged. So, I'm closing the task |
This PR updates [pytest](https://pypi.org/project/pytest) from **3.5.1** to **3.6.3**. <details> <summary>Changelog</summary> ### 3.6.2 ``` ========================= Bug Fixes --------- - Fix regression in ``Node.add_marker`` by extracting the mark object of a ``MarkDecorator``. (`3555 <https://github.com/pytest-dev/pytest/issues/3555>`_) - Warnings without ``location`` were reported as ``None``. This is corrected to now report ``<undetermined location>``. (`3563 <https://github.com/pytest-dev/pytest/issues/3563>`_) - Continue to call finalizers in the stack when a finalizer in a former scope raises an exception. (`3569 <https://github.com/pytest-dev/pytest/issues/3569>`_) - Fix encoding error with `print` statements in doctests (`3583 <https://github.com/pytest-dev/pytest/issues/3583>`_) Improved Documentation ---------------------- - Add documentation for the ``--strict`` flag. (`3549 <https://github.com/pytest-dev/pytest/issues/3549>`_) Trivial/Internal Changes ------------------------ - Update old quotation style to parens in fixture.rst documentation. (`3525 <https://github.com/pytest-dev/pytest/issues/3525>`_) - Improve display of hint about ``--fulltrace`` with ``KeyboardInterrupt``. (`3545 <https://github.com/pytest-dev/pytest/issues/3545>`_) - pytest's testsuite is no longer runnable through ``python setup.py test`` -- instead invoke ``pytest`` or ``tox`` directly. (`3552 <https://github.com/pytest-dev/pytest/issues/3552>`_) - Fix typo in documentation (`3567 <https://github.com/pytest-dev/pytest/issues/3567>`_) ``` ### 3.6.1 ``` ========================= Bug Fixes --------- - Fixed a bug where stdout and stderr were logged twice by junitxml when a test was marked xfail. (`3491 <https://github.com/pytest-dev/pytest/issues/3491>`_) - Fix ``usefixtures`` mark applyed to unittest tests by correctly instantiating ``FixtureInfo``. (`3498 <https://github.com/pytest-dev/pytest/issues/3498>`_) - Fix assertion rewriter compatibility with libraries that monkey patch ``file`` objects. (`3503 <https://github.com/pytest-dev/pytest/issues/3503>`_) Improved Documentation ---------------------- - Added a section on how to use fixtures as factories to the fixture documentation. (`3461 <https://github.com/pytest-dev/pytest/issues/3461>`_) Trivial/Internal Changes ------------------------ - Enable caching for pip/pre-commit in order to reduce build time on travis/appveyor. (`3502 <https://github.com/pytest-dev/pytest/issues/3502>`_) - Switch pytest to the src/ layout as we already suggested it for good practice - now we implement it as well. (`3513 <https://github.com/pytest-dev/pytest/issues/3513>`_) - Fix if in tests to support 3.7.0b5, where a docstring handling in AST got reverted. (`3530 <https://github.com/pytest-dev/pytest/issues/3530>`_) - Remove some python2.5 compatibility code. (`3529 <https://github.com/pytest-dev/pytest/issues/3529>`_) ``` ### 3.6.0 ``` ========================= Features -------- - Revamp the internals of the ``pytest.mark`` implementation with correct per node handling which fixes a number of long standing bugs caused by the old design. This introduces new ``Node.iter_markers(name)`` and ``Node.get_closest_mark(name)`` APIs. Users are **strongly encouraged** to read the `reasons for the revamp in the docs <https://docs.pytest.org/en/latest/mark.htmlmarker-revamp-and-iteration>`_, or jump over to details about `updating existing code to use the new APIs <https://docs.pytest.org/en/latest/mark.htmlupdating-code>`_. (`3317 <https://github.com/pytest-dev/pytest/issues/3317>`_) - Now when ``pytest.fixture`` is applied more than once to the same function a ``ValueError`` is raised. This buggy behavior would cause surprising problems and if was working for a test suite it was mostly by accident. (`2334 <https://github.com/pytest-dev/pytest/issues/2334>`_) - Support for Python 3.7's builtin ``breakpoint()`` method, see `Using the builtin breakpoint function <https://docs.pytest.org/en/latest/usage.htmlbreakpoint-builtin>`_ for details. (`3180 <https://github.com/pytest-dev/pytest/issues/3180>`_) - ``monkeypatch`` now supports a ``context()`` function which acts as a context manager which undoes all patching done within the ``with`` block. (`3290 <https://github.com/pytest-dev/pytest/issues/3290>`_) - The ``--pdb`` option now causes KeyboardInterrupt to enter the debugger, instead of stopping the test session. On python 2.7, hitting CTRL+C again exits the debugger. On python 3.2 and higher, use CTRL+D. (`3299 <https://github.com/pytest-dev/pytest/issues/3299>`_) - pytest not longer changes the log level of the root logger when the ``log-level`` parameter has greater numeric value than that of the level of the root logger, which makes it play better with custom logging configuration in user code. (`3307 <https://github.com/pytest-dev/pytest/issues/3307>`_) Bug Fixes --------- - A rare race-condition which might result in corrupted ``.pyc`` files on Windows has been hopefully solved. (`3008 <https://github.com/pytest-dev/pytest/issues/3008>`_) - Also use iter_marker for discovering the marks applying for marker expressions from the cli to avoid the bad data from the legacy mark storage. (`3441 <https://github.com/pytest-dev/pytest/issues/3441>`_) - When showing diffs of failed assertions where the contents contain only whitespace, escape them using ``repr()`` first to make it easy to spot the differences. (`3443 <https://github.com/pytest-dev/pytest/issues/3443>`_) Improved Documentation ---------------------- - Change documentation copyright year to a range which auto-updates itself each time it is published. (`3303 <https://github.com/pytest-dev/pytest/issues/3303>`_) Trivial/Internal Changes ------------------------ - ``pytest`` now depends on the `python-atomicwrites <https://github.com/untitaker/python-atomicwrites>`_ library. (`3008 <https://github.com/pytest-dev/pytest/issues/3008>`_) - Update all pypi.python.org URLs to pypi.org. (`3431 <https://github.com/pytest-dev/pytest/issues/3431>`_) - Detect `pytest_` prefixed hooks using the internal plugin manager since ``pluggy`` is deprecating the ``implprefix`` argument to ``PluginManager``. (`3487 <https://github.com/pytest-dev/pytest/issues/3487>`_) - Import ``Mapping`` and ``Sequence`` from ``_pytest.compat`` instead of directly from ``collections`` in ``python_api.py::approx``. Add ``Mapping`` to ``_pytest.compat``, import it from ``collections`` on python 2, but from ``collections.abc`` on Python 3 to avoid a ``DeprecationWarning`` on Python 3.7 or newer. (`3497 <https://github.com/pytest-dev/pytest/issues/3497>`_) ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/pytest - Changelog: https://pyup.io/changelogs/pytest/ - Repo: https://github.com/pytest-dev/pytest/issues - Homepage: http://pytest.org </details>
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
The text was updated successfully, but these errors were encountered: