-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Rewrite hook attempts to rewrite namespace package #2419
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
Comments
…1.2 and tests pass. Ref pytest-dev/pytest#2419.
@jaraco i believe the actual issue is that the rewriter also tries to write to the is there a cheap way to detect the setuptools sandbox so we can opt out of writing in such cases |
Yes, indeed. The rewriter is attempting to create the I want to be cautious (if possible) not to overfit the solution to this symptom. That is, it's not obvious to me that the rewrite hook should silently fail if it finds itself unable to write to the pycache folder... that is, unless it makes sense to always silently fail if creating the cache files is optional. More importantly, I'd like to answer the question - why is pytest marking these packages as needing rewrite and should it be more precise about what needs rewritten? For example, I can see how the inclusion of |
I disabled the functionality for marking plugin files for rewrite, after which the test suite quickly revealed that #1920 captures why plugins need their imports rewritten. I experimented with the idea of only marking for rewrite those modules specified by the plugins, so if the This behavior strikes me as problemmatic, as it's relying on the bundling of behavior into a distribution package to determine when to rewrite, which would fail in a scenario where a plugin imports fixtures from another package:
In this scenario, when myplugin (and thus other_pkg) are installed, the fixtures from both packages will be made available, but only those in myplugin will be assert rewritten (because those in other_pkg aren't part of the myplugin distribution, the current heuristic for determining candidacy for rewrite). I propose that pytest should instead change the spec such that:
Such a change would prevent pytest from over-eagerly reaching into some packages (as it does with namespace packages such as Thoughts? |
Also to note, such an approach would make the code for rewrite candidacy dramatically simpler, like:
|
The short answer is no. The sandboxing happens in a As I mentioned before, I feel uncomfortable with the idea of pytest specifically avoiding writing pycache files specifically when in a sandbox. Can the pytest assert rewrites work without writing pycache files? It's conceivable that pytest could catch and suppress a It seems there are a few things that pytest could do here:
Those are in my order of preference, although the first option is by far the most invasive. I'm open to other ideas also. I've worked around the urgency of the issue by pinning to the old backports module, but that solution is only temporary. I'll wait until others have time to absorb the considerations and recommend the preferred way forward for pytest. |
Given your very detailed description, I believe we should catch the sandbox error |
See the aforementioned repository pytest-dev/pytest-issue-2419. It has two branches, You can't use pytest master due to Python 2 incompatibility (and your patch excludes the possibility of a backport), so I'm not sure your patch is relevant. |
ok just confirmed that the changes in 5.0 do fix this, they can't be backported however because $ tox -e py37 -r
py37 recreate: /tmp/pytest-issue-2419/.tox/py37
py37 installdeps: pytest>=5, backports.unittest_mock==1.3
py37 develop-inst: /tmp/pytest-issue-2419
py37 installed: atomicwrites==1.3.0,attrs==19.1.0,backports.unittest-mock==1.3,importlib-metadata==0.18,more-itertools==7.1.0,packaging==19.0,pluggy==0.12.0,py==1.8.0,pyparsing==2.4.0,pytest==5.0.0,six==1.12.0,-e [email protected]:pytest-dev/pytest-issue-2419@1eab7a9c7a4bc579fcaa0f40f646e5136feec03a#egg=testproj,wcwidth==0.1.7,zipp==0.5.1
py37 run-test-pre: PYTHONHASHSEED='2338040538'
py37 run-test: commands[0] | py.test
============================= test session starts ==============================
platform linux -- Python 3.7.3, pytest-5.0.0, py-1.8.0, pluggy-0.12.0
cachedir: .tox/py37/.pytest_cache
rootdir: /tmp/pytest-issue-2419
plugins: backports.unittest-mock-1.3
collected 1 item
test_issue.py . [100%]
=========================== 1 passed in 0.01 seconds ===========================
___________________________________ summary ____________________________________
py37: commands succeeded
congratulations :) 2.7 produces a warning but at least does not outright fail: $ tox -r -e py27
py27 create: /tmp/pytest-issue-2419/.tox/py27
py27 installdeps: pytest, backports.unittest_mock==1.3
py27 develop-inst: /tmp/pytest-issue-2419
py27 installed: DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7.,atomicwrites==1.3.0,attrs==19.1.0,backports.unittest-mock==1.3,configparser==3.7.4,contextlib2==0.5.5,funcsigs==1.0.2,importlib-metadata==0.18,mock==3.0.5,more-itertools==5.0.0,packaging==19.0,pathlib2==2.3.4,pluggy==0.12.0,py==1.8.0,pyparsing==2.4.0,pytest==4.6.4,scandir==1.10.0,six==1.12.0,-e [email protected]:pytest-dev/pytest-issue-2419@1eab7a9c7a4bc579fcaa0f40f646e5136feec03a#egg=testproj,wcwidth==0.1.7,zipp==0.5.1
py27 run-test-pre: PYTHONHASHSEED='3636244050'
py27 run-test: commands[0] | py.test
============================= test session starts ==============================
platform linux2 -- Python 2.7.15+, pytest-4.6.4, py-1.8.0, pluggy-0.12.0
cachedir: .tox/py27/.pytest_cache
rootdir: /tmp/pytest-issue-2419
plugins: backports.unittest-mock-1.3
collected 1 item
test_issue.py . [100%]
=============================== warnings summary ===============================
.tox/py27/local/lib/python2.7/site-packages/_pytest/config/__init__.py:784
.tox/py27/local/lib/python2.7/site-packages/_pytest/config/__init__.py:784
/tmp/pytest-issue-2419/.tox/py27/local/lib/python2.7/site-packages/_pytest/config/__init__.py:784: PytestAssertRewriteWarning: Module already imported so cannot be rewritten: backports
self._mark_plugins_for_rewrite(hook)
-- Docs: https://docs.pytest.org/en/latest/warnings.html
===================== 1 passed, 2 warnings in 0.01 seconds =====================
___________________________________ summary ____________________________________
py27: commands succeeded
congratulations :) and that's probably the best we can do there? |
we should simply not war for namespace package roots that get added by pth files |
@RonnyPfannschmidt that's what's happening in pytest 5 -- as far as I can tell we can't detect that case to prevent it in python2 at all |
I'm seeing a similar warning in the pmxbot project:
This example is Python 3 only, and is a case where the package under test is also a plugin and exposes a namespace package ( |
As discovered in pypa/setuptools#1038, when the following is true:
backports.unittest_mock
) is a namespace package,py.test
will attempt to rewrite the namespace package module, which will cause errors in sandboxed environments where writing outside of the prescribed directory is disallowed.It's not obvious to me that this is a bug in pytest, and I admit, the factors are highly specialized.
Still, since namespace packages using the pkg_resources technique don't cause pytest to rewrite that package, I'd like to consider if this issue can be addressed by pytest detecting and honoring these namespace packages and excluding them from needing rewriting.
I have a minimal test project I will be pushing shortly demonstrating the issue.
The text was updated successfully, but these errors were encountered: