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

Integration with `setup.py test` should not require `virtualenv` to be installed #330

Closed
pytoxbot opened this Issue Sep 17, 2016 · 18 comments

Comments

Projects
None yet
7 participants
@pytoxbot

pytoxbot commented Sep 17, 2016

In my Python projects I want to separate the requirements only needed for testing cleanly from the actual project.

The beautiful thing of tox is I can specify all those additional requirements in the tox configuration (tox.ini), and with the integration in setup.py it's possible to not even have tox installed to execute tests.

Actually, this is the theory, because unfortunately, running tests with python setup.py test still requires one dependency to be installed beforehand: virtualenv

Current Behavior

$ python setup.py test
running test
Searching for tox
Reading https://pypi.python.org/simple/tox/
Best match: tox 2.3.1
Downloading https://pypi.python.org/packages/46/39/e15a857fda1852da1485bc88ac4268dbcef037ab526e1ac21accf2a5c24c/tox-2.3.1.tar.gz#md5=9371b3d3e25c03751a0372e19602dfb9
Processing tox-2.3.1.tar.gz
Writing /tmp/easy_install-2zqkvsk9/tox-2.3.1/setup.cfg
Running tox-2.3.1/setup.py -q bdist_egg --dist-dir /tmp/easy_install-2zqkvsk9/tox-2.3.1/egg-dist-tmp-f202aq90
...
Installed /home/user/repos/my-project/.eggs/tox-2.3.1-py3.4.egg
Searching for pluggy<0.4.0,>=0.3.0
Reading https://pypi.python.org/simple/pluggy/
Best match: pluggy 0.3.1
Downloading https://pypi.python.org/packages/1b/a9/6f5f80b75a8d84d21a8a13486fe26a2da9f043f93b464b2e3928be256dc4/pluggy-0.3.1.tar.gz#md5=ecdd791e309f60668b66fec97c2ee7db
Processing pluggy-0.3.1.tar.gz
Writing /tmp/easy_install-p0d1rfyw/pluggy-0.3.1/setup.cfg
Running pluggy-0.3.1/setup.py -q bdist_egg --dist-dir /tmp/easy_install-p0d1rfyw/pluggy-0.3.1/egg-dist-tmp-gsv46avp
...
Installed /home/user/repos/my-project/.eggs/pluggy-0.3.1-py3.4.egg
Searching for py>=1.4.17
Reading https://pypi.python.org/simple/py/
Best match: py 1.4.31
Downloading https://pypi.python.org/packages/f4/9a/8dfda23f36600dd701c6722316ba8a3ab4b990261f83e7d3ffc6dfedf7ef/py-1.4.31.tar.gz#md5=5d2c63c56dc3f2115ec35c066ecd582b
Processing py-1.4.31.tar.gz
Writing /tmp/easy_install-ax6qj5s2/py-1.4.31/setup.cfg
...
Installed /home/user/repos/my-project/.eggs/py-1.4.31-py3.4.egg
Searching for virtualenv>=1.11.2
Reading https://pypi.python.org/simple/virtualenv/
Best match: virtualenv 15.0.1
Downloading https://pypi.python.org/packages/c8/82/7c1eb879dea5725fae239070b48187de74a8eb06b63d9087cd0a60436353/virtualenv-15.0.1.tar.gz#md5=28d76a0d9cbd5dc42046dd14e76a6ecc
Processing virtualenv-15.0.1.tar.gz
Writing /tmp/easy_install-qkl88xj8/virtualenv-15.0.1/setup.cfg
...
Installed /home/user/repos/my-project/.eggs/virtualenv-15.0.1-py3.4.egg
running egg_info
creating my_project.egg-info
writing my_project.egg-info/PKG-INFO
...
writing manifest file 'my_project.egg-info/SOURCES.txt'
running build_ext
GLOB sdist-make: /home/user/repos/my-project/setup.py
flake8 create: /home/user/repos/my-project/.tox/flake8
ERROR: invocation failed (exit code 1), logfile: /home/user/repos/my-project/.tox/flake8/log/flake8-0.log
ERROR: actionid: flake8
msg: getenv
cmdargs: ['/home/user/.virtualenvs/my-project/bin/python', '-m', 'virtualenv', '--python', '/home/user/.virtualenvs/my-project/bin/python3.4', 'flake8']
env: {...}

/home/user/.virtualenvs/my-project/bin/python: No module named virtualenv

ERROR: InvocationError: /home/user/.virtualenvs/my-project/bin/python -m virtualenv --python /home/user/.virtualenvs/my-project/bin/python3.4 flake8 (see /home/user/repos/my-project/.tox/flake8/log/flake8-0.log)
_____________________________________________________________________________________ summary ______________________________________________________________________________________
ERROR:   flake8: InvocationError: /home/user/.virtualenvs/my-project/bin/python -m virtualenv --python /home/user/.virtualenvs/my-project/bin/python3.4 flake8 (see /home/user/repos/my-project/.tox/flake8/log/flake8-0.log)

When I run pip install virtualenv the same testrun runs like a charm.

Is this a problem of tox, or an issue of setuptools?

Expected Behavior

After having set up tox with setuptools integration
it should be possible to run python setup.py test to execute tests driven by tox
without having had to install any requirements (such as virtualenv or tox) manually beforehand.

References

This problem is also documented in issue #245.

@pytoxbot

This comment has been minimized.

pytoxbot commented Sep 17, 2016

Original comment by @bittner

Looks like for some reason the virtualenv module is not found in the PYTHONPATH directly after being installed by setuptools.

@pytoxbot

This comment has been minimized.

pytoxbot commented Sep 17, 2016

Original comment by @bittner

@hpk42 Is this problem known to the tox project?

I've been contacted privately by people that also face this issue. (Not sure why they don't post here.) - Is there anything I can contribute to have this fixed?

@bittner

This comment has been minimized.

Contributor

bittner commented Sep 17, 2016

Can a maintainer comment on this issue? Is this known as a bug?

@RonnyPfannschmidt

This comment has been minimized.

Contributor

RonnyPfannschmidt commented Sep 17, 2016

@bittner currently tox does not expect to be part of a easy-install temporary installation (multi version installed eggs enabled only for the current python session)

the setup.py integration only works if tox is actually installed

@jaraco

This comment has been minimized.

Contributor

jaraco commented Sep 17, 2016

the setup.py integration only works if tox is actually installed

That obviates the whole point of invoking the tests from setup.py. If that's true, the docs shouldn't suggest using that setuptools command at all and should just recommend invoking python -m tox or simply tox. I think tox does want to support setuptools-bootstrapped invocation.

Although I have a fix for this issue in pypa/setuptools#794, the fix creates a new issue: If the package under test depends on any of the packages under the tests_require (or their dependencies), pip will not install those in the target virtualenv (because the requirement is already satisfied by the egg in PYTHONPATH), but when the tests run, the dependency is no longer reachable. I encountered this when running the CherryPy tests - six doesn't get installed into .tox/py27.

What's a little surprising is that I would expect PYTHONPATH should still be set and that would satisfy the test run, but it doesn't. I'll see if I can discover why that is.

@bittner

This comment has been minimized.

Contributor

bittner commented Sep 17, 2016

Exactly. 👍

Just to give some motivation: For me, one point of having this working is to obsolete the requirements-dev.txt file you see in some projects. I can't stand it anymore. There should be only one canonical set of requirements in the project. What is needed for running tests and verification of any sort should be provided by the the test tool and its integration. Ideally.

Why? Because developers love to make their job easy. They will "just make it work", install the full set of requirements. And then we'll have it again. "No. Not possible. That works on my machine!"
I've had enough of those type of conversations.

@jaraco

This comment has been minimized.

Contributor

jaraco commented Sep 17, 2016

What's a little surprising is that I would expect PYTHONPATH should still be set and that would satisfy the test run, but it doesn't. I'll see if I can discover why that is.

So here's where tox cleans the env when running the tests.

But when dependencies are installed, the PYTHONPATH is present, so pip won't install any overlapping dependencies and they'll be missing when running tests.

@bittner

This comment has been minimized.

Contributor

bittner commented Sep 17, 2016

@jaraco Re "the docs shouldn't suggest using that setuptools command":

My guess is that part of the documentation was written by @hpk42, and I feel he's a fan of making things straightforward. Pragmatic. @RonnyPfannschmidt and @The-Compiler, you seem to partly agree with his ideas, but not to a degree a common user of tox and py.test would expect, correct? No offence, seriously. I'm drawing my conclusions from a few observations in a few conversations.

I'm interested to see where this topic leads us to. I'm really a big fan of "there must be a better way".

@jaraco

This comment has been minimized.

Contributor

jaraco commented Sep 17, 2016

The "better way", if I may be so bold, is to use rwt, which may become pip run:

python -m rwt -r tests/requirements.txt -- -m tox

However, I think it will fall prey to the same issue I'm working on in my last couple of comments.

jaraco added a commit to jaraco/tox that referenced this issue Sep 18, 2016

@jaraco

This comment has been minimized.

Contributor

jaraco commented Sep 18, 2016

With the patch above and the (yet unreleased setuptools 27.3.0), tox will now run the CherryPy tests via setup.py test, even on a system that has no virtualenv or tox or six.

@hpk42, @RonnyPfannschmidt: Can you think of any reason tox shouldn't exclude PYTHONPATH when invoking the installer?

@bittner

This comment has been minimized.

Contributor

bittner commented Sep 18, 2016

@jaraco Re rwt, that translates to having to install rwt for testing in advance. Is that not something we should liberate us from? 😕

Arrgh, and then there is this tests/requirements.txt. I know this is common. Still, I feel those things should go into tox.ini as deps, the context where it belongs to. Would make it harder to do the wrong things easily, don't you think?

@jaraco

This comment has been minimized.

Contributor

jaraco commented Sep 18, 2016

@bittner: Yes, good point. I had originally typed rwt tox -- -m tox (or pip run tox -- -m tox), which will perhaps be a better recommendation, which will bootstrap tox and then let tox run its thing. We'll definitely have to get that story right and then document it properly.

@RonnyPfannschmidt

This comment has been minimized.

Contributor

RonnyPfannschmidt commented Sep 18, 2016

@jaraco i agree that the setup.py integration is not good
my personal op-pinion by now is however, that setup.py test should run on exactly the given python version

i'm not a fan of rwt/pip run either

@bittner

This comment has been minimized.

Contributor

bittner commented Oct 1, 2016

Let's agree on that whatever we do on this repository is to enhance the feature set and fix the documentation of Tox. In other words,

  • Personal opinions about Tox "yes" -- if you can't help!
  • Personal opinions about other tools (setuptools, rwt, ...), what they do and shouldn't do, "no"
  • If things are broken with documentation or integration of any kind, let's fix that!

Current State

As it stands, the Tox documentation promotes an integration with setuptools (in a very specific way). The intended result of this integration is that python setup.py test 1.) runs tox 2.) without requiring it to be installed beforehand. (Note: a nice approach to separate additional test from production requirements.)

Unfortunately, this integration doesn't manage to install the virtualenv dependency in a working way. That's a bug: it makes it impossible to realize the intended behavior.

How Fix This?

  • Either we make the integration work as intended,
  • Or we stop promoting the integration with setuptools, i.e. remove it from the documentation (and ideally add a note about what is suggested instead -- even if this is just "Stop doing it, because ..."),

Are there any other alternatives? Short-term, at least?

@jaraco

This comment has been minimized.

Contributor

jaraco commented Oct 1, 2016

I'd very much like to see the integration work as intended. It's not just this mode (setup.py invocation) but also any tool that makes packages available on PYTHONPATH (which rwt does and which Setuptools and pytest runner now also do) when invoking tox will encounter this issue.

jsonn pushed a commit to jsonn/pkgsrc that referenced this issue Oct 3, 2016

wiz
Updated py-setuptools to 28.1.0.
v28.1.0
-------

* #803: Bump certifi to 2016.9.26.

v28.0.0
-------

* #733: Do not search excluded directories for packages.
  This introduced a backwards incompatible change in ``find_packages()``
  so that ``find_packages(exclude=['foo']) == []``, excluding subpackages of ``foo``.
  Previously, ``find_packages(exclude=['foo']) == ['foo.bar']``,
  even though the parent ``foo`` package was excluded.

* #795: Bump certifi.

* #719: Suppress decoding errors and instead log a warning
  when metadata cannot be decoded.

v27.3.1
-------

* #790: In MSVC monkeypatching, explicitly patch each
  function by name in the target module instead of inferring
  the module from the function's ``__module__``. Improves
  compatibility with other packages that might have previously
  patched distutils functions (i.e. NumPy).

v27.3.0
-------

* #794: In test command, add installed eggs to PYTHONPATH
  when invoking tests so that subprocesses will also have the
  dependencies available. Fixes `tox 330
  <https://github.com/tox-dev/tox/issues/330>`_.

* #795: Update vendored pyparsing 2.1.9.
@FRidh

This comment has been minimized.

FRidh commented Oct 5, 2016

Or we stop promoting the integration with setuptools, i.e. remove it from the documentation (and ideally add a note about what is suggested instead -- even if this is just "Stop doing it, because ..."),

Yes, please! As a distro packager (Nix/NixOS) this is very annoying. We package packages for multiple interpreter versions, and during the build process we automatically run tests as well (by default using python setup.py test). If this would call tox, it might want to test against other interpreters. This won't work, because we build the packages in isolated environments.

Granted, our distro is a bit different than others. But as @RonnyPfannschmidt pointed out, an invocation of python setup.py test should only test against exactly that interpreter. If you want a tool that tests multiple envs, go ahead, use tox, but let tox run python setup.py test for each env.

I think this should also be communicated clearly: python setup.py test is for testing against the current env, and tox is for testing against multiple envs. It would be great if more projects would stick to this, because its getting crazy with all the different commands for running the test runner (python setup.py, nosetests, py.test, python run_tests.py, python runtests.py, tox, ...).

@warsaw

This comment has been minimized.

warsaw commented Oct 5, 2016

On Oct 05, 2016, at 04:01 AM, Frederik Rietdijk wrote:

I think this should also be communicated clearly: python setup.py test is
for testing against the current env, and tox is for testing against
multiple envs.

+1

jsonn pushed a commit to jsonn/pkgsrc that referenced this issue Oct 6, 2016

wiz
Updated py-setuptools to 28.1.0.
v28.1.0
-------

* #803: Bump certifi to 2016.9.26.

v28.0.0
-------

* #733: Do not search excluded directories for packages.
  This introduced a backwards incompatible change in ``find_packages()``
  so that ``find_packages(exclude=['foo']) == []``, excluding subpackages of ``foo``.
  Previously, ``find_packages(exclude=['foo']) == ['foo.bar']``,
  even though the parent ``foo`` package was excluded.

* #795: Bump certifi.

* #719: Suppress decoding errors and instead log a warning
  when metadata cannot be decoded.

v27.3.1
-------

* #790: In MSVC monkeypatching, explicitly patch each
  function by name in the target module instead of inferring
  the module from the function's ``__module__``. Improves
  compatibility with other packages that might have previously
  patched distutils functions (i.e. NumPy).

v27.3.0
-------

* #794: In test command, add installed eggs to PYTHONPATH
  when invoking tests so that subprocesses will also have the
  dependencies available. Fixes `tox 330
  <https://github.com/tox-dev/tox/issues/330>`_.

* #795: Update vendored pyparsing 2.1.9.

@hpk42 hpk42 closed this in #376 Oct 11, 2016

@hpk42

This comment has been minimized.

Contributor

hpk42 commented Oct 11, 2016

just to be clear, in the tox docs we now don't recommend anymore to integrate tox with setup.py test. See https://github.com/tox-dev/tox/blob/master/doc/example/basic.txt#L237

jdknight added a commit to tonybaloney/sphinxcontrib-confluencebuilder that referenced this issue Jan 25, 2018

tox: remove explicit custom python path
Removes the custom Python path explicitly set in the tox configuration
file. This should resolve situations [1] where pip may not install
required packages (and dependencies) into target virtualenv since they
are already provided in PYTHONPATH (see also [2]).

[1]: #45
[2]: tox-dev/tox#330

Signed-off-by: James Knight <james.d.knight@live.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment