Skip to content
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

Running with pytest-xdist changes sys.path #376

Closed
nedbat opened this issue Nov 13, 2018 · 8 comments
Closed

Running with pytest-xdist changes sys.path #376

nedbat opened this issue Nov 13, 2018 · 8 comments

Comments

@nedbat
Copy link

nedbat commented Nov 13, 2018

A simple test to show sys.path (and then fail, to always see the output):

$ cat test_sys_path.py
import pprint, sys

def test_sys_path():
    pprint.pprint(sys.path)
    1/0
$ pip freeze
apipkg==1.5
atomicwrites==1.2.1
attrs==18.2.0
execnet==1.5.0
more-itertools==4.3.0
pluggy==0.8.0
py==1.7.0
pytest==3.10.1
pytest-forked==0.2
pytest-xdist==1.24.1
six==1.11.0
$ pytest -n 0 -q
F                                                                                                                                                        [100%]
=========================================================================== FAILURES ===========================================================================
________________________________________________________________________ test_sys_path _________________________________________________________________________

    def test_sys_path():
        pprint.pprint(sys.path)
>       1/0
E       ZeroDivisionError: division by zero

test_sys_path.py:5: ZeroDivisionError
--------------------------------------------------------------------- Captured stdout call ---------------------------------------------------------------------
['/private/var/folders/j2/gr3cj3jn63s5q8g3bjvw57hm0000gp/T/pytest_sys_path',
 '/usr/local/virtualenvs/tmp-9cf2e53a94c913c/bin',
 '/usr/local/virtualenvs/tmp-9cf2e53a94c913c/lib/python36.zip',
 '/usr/local/virtualenvs/tmp-9cf2e53a94c913c/lib/python3.6',
 '/usr/local/virtualenvs/tmp-9cf2e53a94c913c/lib/python3.6/lib-dynload',
 '/usr/local/py/cpython-v3.6.7/lib/python3.6',
 '/usr/local/virtualenvs/tmp-9cf2e53a94c913c/lib/python3.6/site-packages']
1 failed in 0.05 seconds
$ pytest -n 1 -q
gw0 [1]
scheduling tests via LoadScheduling
F                                                                                                                                                        [100%]
=========================================================================== FAILURES ===========================================================================
________________________________________________________________________ test_sys_path _________________________________________________________________________
[gw0] darwin -- Python 3.6.7 /usr/local/virtualenvs/tmp-9cf2e53a94c913c/bin/python3.6

    def test_sys_path():
        pprint.pprint(sys.path)
>       1/0
E       ZeroDivisionError: division by zero

test_sys_path.py:5: ZeroDivisionError
--------------------------------------------------------------------- Captured stdout call ---------------------------------------------------------------------
['/private/var/folders/j2/gr3cj3jn63s5q8g3bjvw57hm0000gp/T/pytest_sys_path',
 '',
 '/usr/local/virtualenvs/tmp-9cf2e53a94c913c/lib/python36.zip',
 '/usr/local/virtualenvs/tmp-9cf2e53a94c913c/lib/python3.6',
 '/usr/local/virtualenvs/tmp-9cf2e53a94c913c/lib/python3.6/lib-dynload',
 '/usr/local/py/cpython-v3.6.7/lib/python3.6',
 '/usr/local/virtualenvs/tmp-9cf2e53a94c913c/lib/python3.6/site-packages']
1 failed in 0.54 seconds

The second entry in sys.path is different: without xdist, it's the bin directory of the virtualenv. With xdist, it's an empty string (current directory).

xdist shouldn't change this, it means my tests behave differently under the two scenarios.

@nedbat
Copy link
Author

nedbat commented Nov 25, 2018

Also, xdist is setting PYTHONPATH to the launch directory and empty string. This was what really hurt my tests.

@alexmazurik
Copy link

Does anyone know why pytest-xdist prepend sys.path instead of appending in this line? I suppose changing
import sys ; sys.path.insert(0, %r)
to
import sys ; sys.path.append(%r)
could fix many import-related problems

@nicoddemus
Copy link
Member

Thanks @alexmazurik, but that's only when using rsync, I think @nedbat's description of the problem is done right at the start of the remote here:

sys.path.insert(0, importpath) # XXX only for remote situations
os.environ["PYTHONPATH"] = (
importpath + os.pathsep + os.environ.get("PYTHONPATH", "")
)

This was added by d81ffaa, which states:

some changes to make remote testing work better - slightly hackish and will remain so until 'tox' is used to create environments and properly set things up.

Not sure what was the problem at the time, but perhaps this can be removed then?

@nicoddemus
Copy link
Member

I applied this patch as a quick test:

diff --git a/xdist/remote.py b/xdist/remote.py
index b0092e0..a845d2b 100644
--- a/xdist/remote.py
+++ b/xdist/remote.py
@@ -253,11 +253,11 @@ def remote_initconfig(option_dict, args):
 if __name__ == "__channelexec__":
     channel = channel  # noqa
     workerinput, args, option_dict = channel.receive()
-    importpath = os.getcwd()
-    sys.path.insert(0, importpath)  # XXX only for remote situations
-    os.environ["PYTHONPATH"] = (
-        importpath + os.pathsep + os.environ.get("PYTHONPATH", "")
-    )
+    #importpath = os.getcwd()
+    #sys.path.insert(0, importpath)  # XXX only for remote situations
+    #os.environ["PYTHONPATH"] = (
+    #    importpath + os.pathsep + os.environ.get("PYTHONPATH", "")
+    #)
     os.environ["PYTEST_XDIST_WORKER"] = workerinput["workerid"]
     os.environ["PYTEST_XDIST_WORKER_COUNT"] = str(workerinput["workercount"])
     # os.environ['PYTHONPATH'] = importpath

and all of xdist's tests still pass.

@RonnyPfannschmidt
Copy link
Member

I very vaugely recall this was used to ensure correctness in multi interpreter usage id like to do some archeology before dropping it

@nicoddemus
Copy link
Member

Oh that makes sense. I think today such multi interpreter usages are rare, given that we now have tox. But it is definitely interesting to know what you can dig. 😁

nicoddemus added a commit to nicoddemus/pytest-xdist that referenced this issue Jan 8, 2019
nicoddemus added a commit to nicoddemus/pytest-xdist that referenced this issue Jan 8, 2019
nicoddemus added a commit to nicoddemus/pytest-xdist that referenced this issue Jan 9, 2019
@nicoddemus
Copy link
Member

Released in 1.26.0. 👍

@webknjaz
Copy link
Member

webknjaz commented Jan 1, 2021

FTR I've found out that this is still happening: pypa/setuptools#2459 (comment).

jaraco added a commit to webknjaz/setuptools that referenced this issue Jan 18, 2021
…ke the repair at the session level and only when xdist is present.
clrpackages pushed a commit to clearlinux-pkgs/setuptools that referenced this issue May 11, 2021
…on 52.0.0

Jason R. Coombs (17):
      Remove support for easy_install-based downloads for fetch_build_eggs (setup_requires).
      Add changelog.
      Rely on pytest-enabler to enable pytest-xdist when present and enabled.
      Avoid indirection in src_dir
      Rely on rootdir to determine the source. Avoids coupling with position in the test suite.
      Extract workaround for pytest-dev/pytest-xdist#376 as a fixture. Invoke the repair at the session level and only when xdist is present.
      Simplify get_build_backend to simply allow override of cwd.
      Restore test_build_sdist_relative_path_import to its former simple implementation.
      Simplify and enhance tests in test_build_meta. Ref #2459.
      Add changelog.
      Prefer 'rootdir' for resolving the project root.
      Remove eggsecutable
      Update changelog.
      Remove easy_install script and module.
      Remove 'main' function from 'easy_install'.
      Update changelog
      Bump version: 51.3.3 → 52.0.0

Sviatoslav Sydorenko (8):
      Parallelize the test runs via pytest-xdist
      Sanitize CWD out of sys.path in xdist mode
      Clarify `test_build_sdist_relative_path_import`
      Make `get_build_backend` cwd path customizable
      Replace `tmpdir_cwd` fixture with `tmp_path`
      Make `test_pip_upgrade_from_source` xdist-friendly
      Isolate src for `test_distutils_adoption`
      Use tmp src copy in `test_clean_env_install`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants