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

Different output between --parallel and -p #1192

Closed
5 tasks
timdaman opened this issue Mar 15, 2019 · 6 comments · Fixed by #1198
Closed
5 tasks

Different output between --parallel and -p #1192

timdaman opened this issue Mar 15, 2019 · 6 comments · Fixed by #1198

Comments

@timdaman
Copy link
Contributor

@timdaman timdaman commented Mar 15, 2019

I have run into a odd bug with the --parallel flag. When I use -p auto it works as expected but when I use the full form of the flag, --parallel auto I get test failures. The auto is passed as an argument to my pytest command. I would expect both of these flags to return the same results.

Below I have output showing:

  • contents of my tox.ini
  • tox --version output
  • pip list showing versions of all the packages installed
  • -p auto working
  • --parallel auto failing

My OS in Mac OS Mojave

$  cat tox.ini 
[tox]
envlist = py35,py36,py37
tox_pyenv_fallback = False
isolated_build = True
skip_missing_interpreters = True

[testenv]
deps = pipenv
commands =
    pipenv install --skip-lock
    pytest {posargs: -v --random-order-bucket module}
setenv =
    LC_ALL = {env:LC_ALL:en_US.UTF-8}
    LANG = {env:LC_ALL:en_US.UTF-8}

$ pipenv run pip list
Package             Version
------------------- -------
atomicwrites        1.3.0  
attrs               19.1.0 
coverage            4.5.3  
filelock            3.0.10 
more-itertools      6.0.0  
pip                 18.1   
pluggy              0.9.0  
py                  1.8.0  
pyfakefs            3.5.8  
pytest              4.3.1  
pytest-cov          2.6.1  
pytest-random-order 1.0.4  
setuptools          40.6.3 
six                 1.12.0 
toml                0.10.0 
tox                 3.7.0  
tox-pyenv           1.1.0  
virtualenv          16.4.3 
wheel               0.32.3 

$ pipenv run tox --version
3.7.0 imported from /Users/tim/.local/share/virtualenvs/check_docker-8LxgOXjy/lib/python3.7/site-packages/tox/__init__.py
registered plugins:
    tox-pyenv-1.1.0 at /Users/tim/.local/share/virtualenvs/check_docker-8LxgOXjy/lib/python3.7/site-packages/tox_pyenv.py

$ pipenv run tox -p auto
✔ OK py37 in 24.014 seconds
✔ OK py35 in 24.991 seconds
✔ OK py36 in 25.597 seconds
______________________________________________________________________________________________ summary ______________________________________________________________________________________________
  py35: commands succeeded
  py36: commands succeeded
  py37: commands succeeded
  congratulations :)

$ pipenv run tox --parallel auto
✖ FAIL py37 in 20.412 seconds
Failed py37 under process 10376, stdout:
py37 inst-nodeps: /Users/tim/PycharmProjects/check_docker/.tox/.tmp/package/4/check_docker-2.2.0.tar.gz
py37 installed: atomicwrites==1.3.0,attrs==19.1.0,certifi==2019.3.9,check-docker==2.2.0,coverage==4.5.3,filelock==3.0.10,more-itertools==6.0.0,pipenv==2018.11.26,pluggy==0.9.0,py==1.8.0,pyfakefs==3.5.8,pytest==4.3.1,pytest-cov==2.6.1,pytest-random-order==1.0.4,six==1.12.0,toml==0.10.0,tox==3.7.0,tox-pyenv==1.1.0,virtualenv==16.4.3,virtualenv-clone==0.5.1
py37 run-test-pre: PYTHONHASHSEED='924562642'
py37 runtests: commands[0] | pipenv install --skip-lock
Courtesy Notice: Pipenv found itself running within a virtual environment, so it will automatically use that environment, instead of creating its own for any project. You can set PIPENV_IGNORE_VIRTUALENVS=1 to force pipenv to ignore that environment and create its own instead. You can set PIPENV_VERBOSITY=-1 to suppress this warning.
Installing dependencies from Pipfile…
py37 runtests: commands[1] | pytest auto
============================= test session starts ==============================
platform darwin -- Python 3.7.1, pytest-4.3.1, py-1.8.0, pluggy-0.9.0
cachedir: .tox/py37/.pytest_cache
Test order randomisation NOT enabled. Enable with --random-order or --random-order-bucket=<bucket_type>
rootdir: /Users/tim/PycharmProjects/check_docker, inifile:
plugins: random-order-1.0.4, cov-2.6.1, pyfakefs-3.5.8

========================= no tests ran in 0.00 seconds =========================
ERROR: file not found: auto

ERROR: InvocationError for command '/Users/tim/PycharmProjects/check_docker/.tox/py37/bin/pytest auto' (exited with code 4)
✖ FAIL py36 in 21.138 seconds
Failed py36 under process 10375, stdout:
py36 inst-nodeps: /Users/tim/PycharmProjects/check_docker/.tox/.tmp/package/5/check_docker-2.2.0.tar.gz
py36 installed: atomicwrites==1.3.0,attrs==19.1.0,certifi==2019.3.9,check-docker==2.2.0,coverage==4.5.3,filelock==3.0.10,more-itertools==6.0.0,pipenv==2018.11.26,pluggy==0.9.0,py==1.8.0,pyfakefs==3.5.8,pytest==4.3.1,pytest-cov==2.6.1,pytest-random-order==1.0.4,six==1.12.0,toml==0.10.0,tox==3.7.0,tox-pyenv==1.1.0,virtualenv==16.4.3,virtualenv-clone==0.5.1
py36 run-test-pre: PYTHONHASHSEED='702456295'
py36 runtests: commands[0] | pipenv install --skip-lock
Courtesy Notice: Pipenv found itself running within a virtual environment, so it will automatically use that environment, instead of creating its own for any project. You can set PIPENV_IGNORE_VIRTUALENVS=1 to force pipenv to ignore that environment and create its own instead. You can set PIPENV_VERBOSITY=-1 to suppress this warning.
Installing dependencies from Pipfile…
py36 runtests: commands[1] | pytest auto
============================= test session starts ==============================
platform darwin -- Python 3.6.7, pytest-4.3.1, py-1.8.0, pluggy-0.9.0
cachedir: .tox/py36/.pytest_cache
Test order randomisation NOT enabled. Enable with --random-order or --random-order-bucket=<bucket_type>
rootdir: /Users/tim/PycharmProjects/check_docker, inifile:
plugins: random-order-1.0.4, cov-2.6.1, pyfakefs-3.5.8

========================= no tests ran in 0.00 seconds =========================
ERROR: file not found: auto

ERROR: InvocationError for command '/Users/tim/PycharmProjects/check_docker/.tox/py36/bin/pytest auto' (exited with code 4)
✖ FAIL py35 in 21.308 seconds
Failed py35 under process 10374, stdout:
py35 inst-nodeps: /Users/tim/PycharmProjects/check_docker/.tox/.tmp/package/3/check_docker-2.2.0.tar.gz
py35 installed: atomicwrites==1.3.0,attrs==19.1.0,certifi==2019.3.9,check-docker==2.2.0,coverage==4.5.3,filelock==3.0.10,more-itertools==6.0.0,pathlib2==2.3.3,pipenv==2018.11.26,pluggy==0.9.0,py==1.8.0,pyfakefs==3.5.8,pytest==4.3.1,pytest-cov==2.6.1,pytest-random-order==1.0.4,six==1.12.0,toml==0.10.0,tox==3.7.0,tox-pyenv==1.1.0,virtualenv==16.4.3,virtualenv-clone==0.5.1
py35 run-test-pre: PYTHONHASHSEED='4099447597'
py35 runtests: commands[0] | pipenv install --skip-lock
Courtesy Notice: Pipenv found itself running within a virtual environment, so it will automatically use that environment, instead of creating its own for any project. You can set PIPENV_IGNORE_VIRTUALENVS=1 to force pipenv to ignore that environment and create its own instead. You can set PIPENV_VERBOSITY=-1 to suppress this warning.
Installing dependencies from Pipfile…
py35 runtests: commands[1] | pytest auto
============================= test session starts ==============================
platform darwin -- Python 3.5.6, pytest-4.3.1, py-1.8.0, pluggy-0.9.0
cachedir: .tox/py35/.pytest_cache
Test order randomisation NOT enabled. Enable with --random-order or --random-order-bucket=<bucket_type>
rootdir: /Users/tim/PycharmProjects/check_docker, inifile:
plugins: random-order-1.0.4, cov-2.6.1, pyfakefs-3.5.8

========================= no tests ran in 0.00 seconds =========================
ERROR: file not found: auto

ERROR: InvocationError for command '/Users/tim/PycharmProjects/check_docker/.tox/py35/bin/pytest auto' (exited with code 4)
______________________________________________________________________________________________ summary ______________________________________________________________________________________________
ERROR:   py35: parallel child exit code 1
ERROR:   py36: parallel child exit code 1
ERROR:   py37: parallel child exit code 1

Thanks for submitting an issue!

If submitting a BUG please provide:

  • Minimal reproducible example or detailed description, assign "bug"
  • OS and pip list output

if submitting an ENHANCEMENT issue:

  • a clear problem statement with an example
  • suggested change with example
  • if you have want help to do a PR yourself
@rpkilby
Copy link
Member

@rpkilby rpkilby commented Mar 15, 2019

Quick thoughts...

  • Have you tried this outside of pipenv (e.g., just a regular virtualenv)?
  • Have you tried --parallel=auto instead of --parallel auto?

@timdaman
Copy link
Contributor Author

@timdaman timdaman commented Mar 16, 2019

Interesting. I first reused the venv shown by pipenv --venv and then I created a new venv by hand using a the pip freeze command to get a package list to install in my new venv.

The behavior was very consistent between all of three venvs I have

  • Using pipenv run
  • Manually activation venv shown by pipenv --venv
  • Manually creating venv and populating with package list from pipenv run pip freeze.

When I used the --parallel auto I got failures but --parallel=auto worked in all of the above.

For now I am unblocked but I think it likely to surprise others so maybe fix or update documentation? I am rather unwell ATM but if I feel up to I can try digging in too.

@timdaman
Copy link
Contributor Author

@timdaman timdaman commented Mar 17, 2019

This bug is kind of neat. Here is what is a happening

When passing -p=auto tox works as expected
When passing -p auto tox works as expected
When passing —-parallel=auto tox works as expected
When passing -—parallel auto tox breaks, it passes “auto” as a value in “posargs”

This is why

When tox is run in parallel mode it spawns “sub-tox”s to do the work and then gathers the results. When it calls a sub-tox it uses the same arguments the parent tox received with some modifications. One of these is it removes the first ‘—parallel’ flag it finds. I assume this is to to prevent infinite recursion. As part of running a “sub-tox” an environment variable is set PARALLEL_ENV_VAR_KEY. That var is used by tox to identify it is a sub-tox. Thus sub-toxs don’t try to create sub-toxs.

The bug is that when removing ‘—parallel’ tox doesn’t not remove the value it is being set too. The args argparse parameter scoops this value up as a free positional argument and passes as part of posargs. The bug goes away when using the —parallel=value form a because there is no abandoned value.

When using the -p flag the problem does not occur because -p is never removed so it cannot leave an abandoned value.

There is another bug. When multiple ‘—parallel’ flags are given the duplicates are not removed before calling the sub-toxs, even when using the —parallel=value form.

The environment variable is actually the only thing preventing infinite recursion.

I have submitted two PRs.

  • #1198 Removed the arg manipulation in favor of relying solely on the environment variable for protection. This has a disadvantage that the arguments passed to the sub-toxs can be a bit confusing since they will include an ignored —parallel flag. That is already how it is with the -p flag so perhaps not a big deal. It also makes the code simpler.
  • #1197 Making the arg manipulation much more robust so it is doing what was intended. This removes all forms of the parallel flag as well as catch duplicates.

If you like please only merge one and close the other.

I have looked the unit tests (for the second PR) and tried to write one but frankly I finding it hard to figure out how to test a condition in a test command that is run in a sub-tox. If you have an example of such a test I would be happy to add tests.

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 17, 2019

Parallel tests are here https://github.com/tox-dev/tox/blob/master/tests/unit/session/test_parallel.py#L6

GitHub
Command line driven CI frontend and development task automation tool - tox-dev/tox

@timdaman
Copy link
Contributor Author

@timdaman timdaman commented Mar 17, 2019

Before I go too far down this path, which PR is preferred, the one that removes the broken code (#1198) or the one that fixes it (#1197)? I would rather not work on both.

@gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Mar 17, 2019

I would say let's go with the no removal and that probably also has the benefit of not requiring tests 😁

@helpr helpr bot added pr-rejected and removed pr-available labels Mar 17, 2019
gaborbernat added a commit that referenced this issue Mar 18, 2019
This resolves issue #1192 where the argument to --parallel was sometimes being passed as a `posarg`

Fixes: #1192
@helpr helpr bot added pr-merged and removed pr-rejected labels Mar 18, 2019
@tox-dev tox-dev locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants