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

Support tox-current-env by indirectly calling pytest #86

Closed
jaraco opened this issue Jun 25, 2023 · 3 comments
Closed

Support tox-current-env by indirectly calling pytest #86

jaraco opened this issue Jun 25, 2023 · 3 comments

Comments

@jaraco
Copy link
Owner

jaraco commented Jun 25, 2023

Using tox with tox-current-env and a mixed Python 3.7 and 3.9 environment, the wrong pytest is used on Python 3.7 tests when the pytest executable points to a Python 3.9 installation.

$ ls -l /usr/bin/pytest{,-3*} /usr/bin/python{,3.[79]} /usr/bin/tox{,-3*}
lrwxrwxrwx 1 root root    10 Sep 15  2022 /usr/bin/pytest -> pytest-3.9
-r-xr-xr-x 1 root bin    223 Nov 12  2022 /usr/bin/pytest-3.7
-r-xr-xr-x 1 root bin    223 Nov 12  2022 /usr/bin/pytest-3.9
lrwxrwxrwx 1 root root     9 Sep 14  2022 /usr/bin/python -> python3.9
-r-xr-xr-x 2 root bin  16416 Feb 20 07:50 /usr/bin/python3.7
-r-xr-xr-x 1 root bin  16368 Feb 19 23:55 /usr/bin/python3.9
lrwxrwxrwx 1 root root     7 Oct 14  2022 /usr/bin/tox -> tox-3.9
-r-xr-xr-x 1 root bin    206 Jan 15 22:39 /usr/bin/tox-3.7
-r-xr-xr-x 1 root bin    206 Jan 15 22:39 /usr/bin/tox-3.9
$ cat tox.ini 
[testenv]
commands =
    -pytest
    -python -m pytest
$ /usr/bin/tox-3.7 --current-env --no-provision --recreate -e py37
ROOT: tox-gh-actions won't override envlist because tox is not running in GitHub Actions
py37: remove tox env folder /tmp/test/.tox/py37
py37: commands[0]> pytest
================================================================================================================ test session starts ================================================================================================================
platform sunos5 -- Python 3.9.16, pytest-7.4.0, pluggy-1.2.0
cachedir: .tox/py37/.pytest_cache
Using --randomly-seed=3895753261
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /tmp/test
plugins: mypy-0.10.3, time-machine-2.9.0, env-0.8.2, flake8-1.1.1, datadir-1.4.1, mock-3.11.1, console-scripts-1.4.1, socket-0.6.0, xdist-3.2.1, perf-0.12.0, forked-1.6.0, asyncio-0.21.0, cov-4.1.0, rerunfailures-11.1.2, randomly-3.12.0, lazy-fixture-0.6.3, backports.unittest-mock-1.5, typeguard-4.0.0, freezegun-0.4.2, timeout-2.0.2, hypothesis-6.79.1, subtests-0.10.0, mypy-plugins-1.11.1, pytest_freezer-0.4.8, black-0.3.12, regressions-2.4.2, black-multipy-1.0.1, kgb-7.1.1, teamcity-messages-1.32, checkdocs-2.9.0, flaky-3.7.0, reporter-0.5.2, expect-1.1.0, benchmark-4.0.0, travis-fold-1.3.0, enabler-2.1.0, jaraco.test-5.3.0, pyfakefs-5.2.2
asyncio: mode=strict
collected 0 items                                                                                                                                                                                                                                   

=============================================================================================================== no tests ran in 0.15s ===============================================================================================================
py37: exit 5 (2.98 seconds) /tmp/test> pytest pid=4281
py37: command failed but is marked ignore outcome so handling it as success
py37: commands[1]> python -m pytest
================================================================================================================ test session starts ================================================================================================================
platform sunos5 -- Python 3.7.16, pytest-7.4.0, pluggy-1.2.0
cachedir: .tox/py37/.pytest_cache
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
Using --randomly-seed=3050309400
rootdir: /tmp/test
plugins: xdist-3.2.1, kgb-7.1.1, subtests-0.10.0, jaraco.test-5.3.0, reporter-0.5.2, mypy-plugins-1.11.1, travis-fold-1.3.0, hypothesis-6.79.1, time-machine-2.9.0, regressions-2.4.2, benchmark-4.0.0, freezegun-0.4.2, checkdocs-2.9.0, mypy-0.10.3, datadir-1.4.1, teamcity-messages-1.32, pyfakefs-5.2.2, typeguard-4.0.0, flake8-1.1.1, black-multipy-1.0.1, flaky-3.7.0, backports.unittest-mock-1.5, randomly-3.12.0, expect-1.1.0, asyncio-0.21.0, pytest_freezer-0.4.8, rerunfailures-11.1.2, black-0.3.12, socket-0.6.0, perf-0.12.0, mock-3.11.1, forked-1.6.0, lazy-fixture-0.6.3, env-0.8.2, enabler-2.1.0, timeout-2.0.2, cov-4.1.0, console-scripts-1.4.0
asyncio: mode=strict
collected 0 items                                                                                                                                                                                                                                   

=============================================================================================================== no tests ran in 0.12s ===============================================================================================================
py37: exit 5 (2.87 seconds) /tmp/test> python -m pytest pid=4335
py37: command failed but is marked ignore outcome so handling it as success
  py37: OK (5.92=setup[0.06]+cmd[2.98,2.87] seconds)
  congratulations :) (6.51 seconds)
$

Please note the wrong Python version for plain pytest command above (commands[0]).

Originally posted by @mtelka in jaraco/backports.entry_points_selectable#8 (comment)

@jaraco jaraco changed the title My scenario is a bit different: Support tox-current-env by indirectly calling pytest Jun 25, 2023
@jaraco
Copy link
Owner Author

jaraco commented Jun 25, 2023

Oh. Now I understand better how you're using tox-current-env and what it does. I guess it's no big surprise that after disabling isolation, you're finding problems with overlapping concerns. It's apparent from the output of ls /usr/bin that the binary for pytest is installed for Python 3.9 and not Python 3.7.

It's interesting that python -m pytest would invoke the relevant Python. I'm guessing that tox performs some magic to replace a "python" execution with the relevant Python executable for the environment (and not simply relying on search path manipulation), because /usr/bin/python also refers to Python 3.9. I'd be interested to see if tox does something special to "python" as an executable to resolve it to "sys.executable".

That explains why I was unable to replicate the issue - because in my tox environment, I hadn't provisioned pytest or other testing dependencies.

Note that even the tox official docs suggest to use the bare pytest to invoke tests and the same is true for pytest, so this change would be deviating away from the standard, recommended approach.

At the very least, a change like this probably deserves a comment explaining why a non-standard approach is taken.

But I'm also concerned - what other effects might this proposed change have? For example - it's known that runpy invocation (python -m pytest) has different effects on the sys.path than direct invocation of a script. That's likely to have unexpected consequences for some projects.

Moreover, the proposed change will only help in situations where pytest is the execution runner, but the issue would imply that any other executable (flake8, ruff, black, mypy, ...) also needs to be invoked through Python in order to get the correct version. In the case of skeleton-based projects, that's not a big deal because those checks are invoked through pytest, but should a downstream project have a bespoke executable, it also would need a similar workaround.

I don't feel like this is the right place to resolve this issue either - as it won't solve the problem for other projects and it alters the behavior and reduces simplicity for many unaffected environments. The skeleton project (and its derivatives) attempts to adopt best practices for the ecosystem, so I'd really like to see the recommendation of using python -m pytest to be the recommended invocation (at least in tox docs) prior to adopting.

But honestly, I doubt the project would accept that either, since pytest is simpler and more intuitive, and because there's only a narrow environment affected by this concern (environments specifically bypassing tox's isolation design).

I'm wondering if maybe there's a better way to handle this concern downstream, in those affected test environments.

Since the issue is at the selection of Python when invoking pytest, what if you replaced that binary with something that would honor whatever Python tox was invoked under? Or even better - what if you coded tox-current-env to replace invocations of pytest with python -m pytest. Actually, that last option seems like exactly the right fix at the right place. Since tox-current-env is implicated in the missed expectation, it should adjust the expectation.

@jaraco
Copy link
Owner Author

jaraco commented Jun 25, 2023

I've filed another issue in that project. Let's see where that goes. I'm closing this one for now.

@jaraco jaraco closed this as completed Jun 25, 2023
@mtelka
Copy link

mtelka commented Jun 25, 2023

It's interesting that python -m pytest would invoke the relevant Python. I'm guessing that tox performs some magic to replace a "python" execution with the relevant Python executable for the environment (and not simply relying on search path manipulation), because /usr/bin/python also refers to Python 3.9. I'd be interested to see if tox does something special to "python" as an executable to resolve it to "sys.executable".

The magic you are referring to is done by tox-current-env, not core tox. It is one of main purposes of tox-current-env.

Note that even the tox official docs suggest to use the bare pytest to invoke tests and the same is true for pytest, so this change would be deviating away from the standard, recommended approach.

A bit newer pytest doc is okay with the python -m pytest way.

But I'm also concerned - what other effects might this proposed change have? For example - it's known that runpy invocation (python -m pytest) has different effects on the sys.path than direct invocation of a script. That's likely to have unexpected consequences for some projects.

I package many Python packages for OpenIndiana and where it applies I automatically convert pytest to python -m pytest in tox.ini to run tests. I didn't see any issue so far caused by this.

Moreover, the proposed change will only help in situations where pytest is the execution runner, but the issue would imply that any other executable (flake8, ruff, black, mypy, ...) also needs to be invoked through Python in order to get the correct version. In the case of skeleton-based projects, that's not a big deal because those checks are invoked through pytest, but should a downstream project have a bespoke executable, it also would need a similar workaround.

In this case we are talking about direct/indirect pytest only (because of skeleton). For my particular use case I convert automatically bunch of tools to get called indirectly.

I don't feel like this is the right place to resolve this issue either - as it won't solve the problem for other projects and it alters the behavior and reduces simplicity for many unaffected environments. The skeleton project (and its derivatives) attempts to adopt best practices for the ecosystem, so I'd really like to see the recommendation of using python -m pytest to be the recommended invocation (at least in tox docs) prior to adopting.

Please see above.

But honestly, I doubt the project would accept that either, since pytest is simpler and more intuitive, and because there's only a narrow environment affected by this concern (environments specifically bypassing tox's isolation design).

I'm wondering if maybe there's a better way to handle this concern downstream, in those affected test environments.

Hm. I'm not sure what is the project and downstream here :-(.

Since the issue is at the selection of Python when invoking pytest, what if you replaced that binary with something that would honor whatever Python tox was invoked under? Or even better - what if you coded tox-current-env to replace invocations of pytest with python -m pytest. Actually, that last option seems like exactly the right fix at the right place. Since tox-current-env is implicated in the missed expectation, it should adjust the expectation.

Interesting idea to have tox-current-env to do the replacement automatically. Unfortunately, such functionality is not implemented (yet), so I'd like to have different approach.

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

2 participants