From 9b33f55e3eb020d5b09a2ad06a63d6f1d17f087a Mon Sep 17 00:00:00 2001 From: Buck Golemon Date: Fri, 21 Nov 2014 16:20:54 -0800 Subject: [PATCH 1/3] asottile comments --- tests/functional/get_installed_test.py | 20 ++++-------- tests/functional/install_test.py | 12 ++++---- tests/functional/relocation_test.py | 42 ++++++++++++++++++++++++-- tests/functional/simple_test.py | 30 +++++++++++++++++- tests/testing.py | 20 ++++++++++++ venv_update.py | 31 +++++++++++++------ 6 files changed, 123 insertions(+), 32 deletions(-) diff --git a/tests/functional/get_installed_test.py b/tests/functional/get_installed_test.py index 7f4eb58..ae82945 100644 --- a/tests/functional/get_installed_test.py +++ b/tests/functional/get_installed_test.py @@ -1,26 +1,16 @@ from __future__ import print_function from __future__ import unicode_literals -from testing import run, Path -import venv_update - - -def set_up(tmpdir): - tmpdir.chdir() - - run('virtualenv', 'myvenv') - # surely there's a better way -.- - # NOTE: `pip install TOP` causes an infinite copyfiles loop, under tox - Path(venv_update.__file__).copy(tmpdir) +from testing import run, venv_update_script def get_installed(capfd): out, err = capfd.readouterr() # flush buffers - run('myvenv/bin/python', '-c', '''\ + venv_update_script('''\ import venv_update as v for p in sorted(v.pip_get_installed()): - print(p)''') + print(p)''', venv='myvenv') out, err = capfd.readouterr() assert err == '' @@ -28,7 +18,9 @@ def get_installed(capfd): def test_pip_get_installed(tmpdir, capfd): - set_up(tmpdir) + tmpdir.chdir() + + run('virtualenv', 'myvenv') assert get_installed(capfd) == [] run('myvenv/bin/pip', 'install', 'flake8') assert get_installed(capfd) == ['flake8', 'mccabe', 'pep8', 'pyflakes'] diff --git a/tests/functional/install_test.py b/tests/functional/install_test.py index ccb1a67..975e658 100644 --- a/tests/functional/install_test.py +++ b/tests/functional/install_test.py @@ -1,22 +1,22 @@ from __future__ import print_function from __future__ import unicode_literals -from testing import run - -from functional.get_installed_test import set_up +from testing import run, venv_update_script def test_pip_install_flake8(tmpdir, capfd): - set_up(tmpdir) + tmpdir.chdir() + + run('virtualenv', 'myvenv') run('myvenv/bin/pip', 'install', 'flake8') out, err = capfd.readouterr() # flush buffers - run('myvenv/bin/python', '-c', '''\ + venv_update_script('''\ import json from venv_update import pip_install print(json.dumps(sorted(pip_install(('flake8',))))) -''') +''', venv='myvenv') out, err = capfd.readouterr() assert err == '' diff --git a/tests/functional/relocation_test.py b/tests/functional/relocation_test.py index 596c2b6..1434018 100644 --- a/tests/functional/relocation_test.py +++ b/tests/functional/relocation_test.py @@ -1,4 +1,5 @@ from __future__ import absolute_import +from __future__ import print_function from __future__ import unicode_literals import io @@ -23,8 +24,6 @@ def test_is_relocatable(tmpdir): def test_is_relocatable_different_python_version(tmpdir): tmpdir.chdir() - get_scenario('trivial') - with io.open('requirements.txt', 'w') as reqs: reqs.write('doge==3.5.0') @@ -36,3 +35,42 @@ def test_is_relocatable_different_python_version(tmpdir): venv_update(python_arg) run('sh', '-c', '. virtualenv_run/bin/activate && doge --help') + + +def _get_virtualenv_data(capfd): + out, err = capfd.readouterr() # flush buffers + Path('assertions.py').write(''' +import json +import sys, virtualenv +print(json.dumps((virtualenv.__file__, sys.prefix, sys.real_prefix))) +''') + run('virtualenv_run/bin/python', 'assertions.py') + + out, err = capfd.readouterr() + assert err == '' + + from json import loads + lastline = out.splitlines()[-1] + return loads(lastline) + + +def path_is_within(path, within): + from os.path import relpath + return not relpath(path, within).startswith('..') + + +def test_is_relocatable_system_site_packages(tmpdir, capfd): + tmpdir.chdir() + requirements = Path('requirements.txt') + + # first show that virtualenv is installed to the system python + # then show that virtualenv is installed to the virtualenv python, when it's required + for reqs, invenv in ( + ('', False), + ('virtualenv\n', True), + ): + requirements.write(reqs) + venv_update('--system-site-packages') + virtualenv__file__, prefix, real_prefix = _get_virtualenv_data(capfd) + assert path_is_within(virtualenv__file__, prefix) == invenv + assert path_is_within(virtualenv__file__, real_prefix) == (not invenv) diff --git a/tests/functional/simple_test.py b/tests/functional/simple_test.py index 0964f24..2395435 100644 --- a/tests/functional/simple_test.py +++ b/tests/functional/simple_test.py @@ -3,7 +3,9 @@ from py._path.local import LocalPath as Path import pytest -from testing import get_scenario, run, strip_coverage_warnings, venv_update, TOP +from testing import ( + get_scenario, run, strip_coverage_warnings, venv_update, venv_update_script, TOP, +) from sys import version_info PY33 = (version_info >= (3, 3)) @@ -243,3 +245,29 @@ def test_uncolored_pipe(): out = pipe_output(read, write) assert not unprintable(out), out + + +@pytest.mark.parametrize("options,expected", [ + (('--system-site-packages',), True), + ((), False), +]) +def test_venv_uses_system_site_packages(capfd, tmpdir, options, expected): + tmpdir.chdir() + Path('requirements.txt').write('') + + for options, expected in ( + (options, expected), + # also show that going back and forth works + ((), False), + (('--system-site-packages',), True), + ): + venv_update(*options) + + out, err = capfd.readouterr() # flush buffers + venv_update_script('''\ +from venv_update import venv_uses_system_site_packages +print(venv_uses_system_site_packages())''') + + out, err = capfd.readouterr() # flush buffers + assert err == '' + assert out == '%s\n' % expected diff --git a/tests/testing.py b/tests/testing.py index d496c18..23bc59e 100644 --- a/tests/testing.py +++ b/tests/testing.py @@ -34,6 +34,26 @@ def venv_update(*args): ) +def venv_update_script(pyscript, venv='virtualenv_run'): + """Run a python script that imports venv_update""" + # I wish I didn't need this =/ + # surely there's a better way -.- + # NOTE: `pip install TOP` causes an infinite copyfiles loop, under tox + from venv_update import __file__ as venv_update_path, dotpy + + # symlink so that we get coverage, where possible + venv_update_path = Path(dotpy(venv_update_path)) + local_vu = Path(venv_update_path.basename) + if local_vu.exists(): + local_vu.remove() + local_vu.mksymlinkto(venv_update_path) + + # write it to a file so we get more-reasonable stack traces + testscript = Path('testscript.py') + testscript.write(pyscript) + run('%s/bin/python' % venv, testscript.strpath) + + # coverage.py adds some helpful warnings to stderr, with no way to quiet them. from re import compile as Regex, MULTILINE coverage_warnings_regex = Regex( diff --git a/venv_update.py b/venv_update.py index 2b65eb9..34573b2 100644 --- a/venv_update.py +++ b/venv_update.py @@ -202,8 +202,9 @@ def pip_install(args): from pip.commands.install import InstallCommand install = InstallCommand() - parsed = install.parse_args(list(args)) - successfully_installed = install.run(*parsed) + options, args = install.parse_args(list(args)) + + successfully_installed = install.run(options, args) if successfully_installed is None: return {} else: @@ -254,6 +255,12 @@ def venv(venv_path, venv_args): ) +def venv_uses_system_site_packages(): + from glob import glob + from sys import prefix + return not glob(prefix + '/lib/python*/no-global-site-packages.txt') + + def do_install(reqs): from os import environ @@ -281,15 +288,21 @@ def do_install(reqs): install_opts = ('--use-wheel',) + cache_opts wheel = ('wheel',) + cache_opts - # Bootstrap the install system; setuptools and pip are alreayd installed, just need wheel + # 1) Bootstrap the install system; setuptools and pip are already installed, just need wheel pip_install(install_opts + ('wheel',)) - # Caching: Make sure everything we want is downloaded, cached, and has a wheel. + # 2) Caching: Make sure everything we want is downloaded, cached, and has a wheel. pip(wheel + ('--wheel-dir=' + pip_download_cache, 'wheel') + requirements_as_options) - # Install: Use our well-populated cache (only) to do the installations. - # The --ignore-installed gives more-repeatable behavior in the face of --system-site-packages, - # and brings us closer to a --no-site-packages future + # 3) Install: Use our well-populated cache (only) to do the installations. + install_opts += ('--upgrade', '--no-index',) + if venv_uses_system_site_packages(): + # In the worst case, a --system-site-packages venv will be missing packages because it was built in an + # environment which satisfied requirements at the system level, then be relocated to a system that doesn't have + # the requirement, breaking it. This has bitten us before. --ignore-installed avoids the issue. + print('Detected --system-site-packages; using --ignore-installed. This makes things slower.') + install_opts += ('--ignore-installed',) + recently_installed = pip_install(install_opts + ('--upgrade', '--no-index',) + requirements_as_options) required_with_deps = trace_requirements( @@ -304,7 +317,7 @@ def do_install(reqs): set(['wheel']) # no need to install/uninstall wheel every run ) - # Finally, uninstall any extraneous packages + # 4) Uninstall any extraneous packages. if extraneous: pip(('uninstall', '--yes') + tuple(sorted(extraneous))) @@ -337,7 +350,7 @@ def mark_venv_invalid(venv_path, reqs): def dotpy(filename): - if filename.endswith('.pyc'): + if filename.endswith(('.pyc', '.pyo', '.pyd')): return filename[:-1] else: return filename From e55d234364a6f3a19cfd83d77c954939d98ac0cd Mon Sep 17 00:00:00 2001 From: Buck Golemon Date: Fri, 21 Nov 2014 16:49:26 -0800 Subject: [PATCH 2/3] revamp travis, so test actually works =/ --- .travis.yml | 45 +++++++++++++++++++---------- .travis/test.sh | 2 +- TODO | 2 +- tests/functional/relocation_test.py | 7 ++--- tests/functional/simple_test.py | 5 +--- tox.ini | 1 - venv_update.py | 2 +- 7 files changed, 37 insertions(+), 27 deletions(-) diff --git a/.travis.yml b/.travis.yml index 851c220..b35db32 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,23 +1,29 @@ language: python -env: # These should match the tox env list - # ordered by "most likely to fail in a unique way" first - - TOXENV=py34-lint -# - TOXENV=pypy3-test - - TOXENV=py34-test -# - TOXENV=pypy-test - - TOXENV=py26-test - - TOXENV=py27-test - - TOXENV=py27-lint +python: + - "2.6" + - "2.7" + - "3.4" +# - pypy +# - pypy3 +env: + - TOXENV=lint + - TOXENV=test matrix: # notify a failed build as soon as anything fails fast_finish: true -install: pip install -r requirements.d/travis.txt + exclude: + # pylint has dropped 2.6 support -.- + - python: "2.6" + env: TOXENV=lint +install: + - pip install -r requirements.d/travis.txt + # My tests need some package to be reliably installed to system-site-packages. + # This should maybe be a fixture of some kind, + # but I'd have to build python from source I think =/ + - SYS_REAL_PREFIX=$(python -c 'import sys; print(sys.real_prefix)') + - echo $SYS_REAL_PREFIX + - sudo $SYS_REAL_PREFIX/bin/pip install virtualenv script: tox -cache: - # This doesn't seem to do anything :( - directories: - - $HOME/.pip - - $HOME/.pre-commit after_success: - find -name '.coverage*' | xargs mv -t . - coverage combine @@ -25,3 +31,12 @@ after_success: after_failure: # attempt to show any pip.log on failure - find -name pip.log | xargs -r tail -n99999 + +sudo: true +cache: + # public caches only supported under sudo:false currently + # closest thing to documentation: + # http://blog.travis-ci.com/2014-10-07-free-builds-for-students-github-student-developer-pack + directories: + - $HOME/.pip + - $HOME/.pre-commit diff --git a/.travis/test.sh b/.travis/test.sh index feeae12..5e6a311 100755 --- a/.travis/test.sh +++ b/.travis/test.sh @@ -10,4 +10,4 @@ py.test -n $NCPU \ --cov --cov-config=$TOP/.coveragerc --cov-report='' \ "$@" $TOP/tests $SITEPACKAGES/${PROJECT}.py coverage combine -coverage report --rcfile=$TOP/.coveragerc --fail-under 97 # FIXME: should be 100 +coverage report --rcfile=$TOP/.coveragerc --fail-under 96 # FIXME: should be 100 diff --git a/TODO b/TODO index 790e9f0..294937e 100644 --- a/TODO +++ b/TODO @@ -5,7 +5,7 @@ Should probably do before it hits users Things that I want to do, but would put me past my deadline: -* coverage: 155, 208, 225, 229-231, 338, 368, 372 +* coverage: 155, 209, 226, 230-232, 303-304, 384, 388 * On ubuntu stock python2.7 I have to rm -rf $VIRTUAL_ENV/local to avoid the AssertionError('unexpected third case') diff --git a/tests/functional/relocation_test.py b/tests/functional/relocation_test.py index 1434018..0aad05b 100644 --- a/tests/functional/relocation_test.py +++ b/tests/functional/relocation_test.py @@ -9,6 +9,8 @@ from testing import get_scenario, run, venv_update +PY27 = sys.version_info[:2] == (2, 7) + def test_is_relocatable(tmpdir): tmpdir.chdir() @@ -27,10 +29,7 @@ def test_is_relocatable_different_python_version(tmpdir): with io.open('requirements.txt', 'w') as reqs: reqs.write('doge==3.5.0') - if sys.version_info[:2] == (2, 7): # pragma: no cover PY27 only - python_arg = '--python=python2.6' - else: # pragma: no cover PY26 only - python_arg = '--python=python2.7' + python_arg = '--python=python' + ('2.7' if not PY27 else '2.6') venv_update(python_arg) diff --git a/tests/functional/simple_test.py b/tests/functional/simple_test.py index 2395435..eaa0674 100644 --- a/tests/functional/simple_test.py +++ b/tests/functional/simple_test.py @@ -62,10 +62,7 @@ def test_arguments_version(tmpdir, capfd): assert excinfo.value.returncode == 1 out, err = capfd.readouterr() lasterr = strip_coverage_warnings(err).rsplit('\n', 2)[-2] - if PY33: - errname = 'FileNotFoundError' - else: - errname = 'OSError' + errname = 'FileNotFoundError' if PY33 else 'OSError' assert lasterr.startswith(errname + ': [Errno 2] No such file or directory'), err lines = out.split('\n') diff --git a/tox.ini b/tox.ini index c32262b..138e5c9 100644 --- a/tox.ini +++ b/tox.ini @@ -17,7 +17,6 @@ setenv = TMPDIR={envtmpdir} SITEPACKAGES={envsitepackagesdir} commands = - python --version pip freeze test: {toxinidir}/.travis/test.sh {posargs} lint: pre-commit run --all {posargs} diff --git a/venv_update.py b/venv_update.py index 34573b2..f0eb52b 100644 --- a/venv_update.py +++ b/venv_update.py @@ -303,7 +303,7 @@ def do_install(reqs): print('Detected --system-site-packages; using --ignore-installed. This makes things slower.') install_opts += ('--ignore-installed',) - recently_installed = pip_install(install_opts + ('--upgrade', '--no-index',) + requirements_as_options) + recently_installed = pip_install(install_opts + requirements_as_options) required_with_deps = trace_requirements( (req.req for req in required.values()) From e0e009ad53203428c077796b74b9605de22d900a Mon Sep 17 00:00:00 2001 From: Buck Golemon Date: Tue, 25 Nov 2014 10:18:19 -0800 Subject: [PATCH 3/3] back out the system-site-packages special case --- .travis.yml | 18 +++++-------- requirements.d/_lint.txt | 7 +----- tests/functional/relocation_test.py | 39 ----------------------------- tests/functional/simple_test.py | 28 +-------------------- venv_update.py | 13 ---------- 5 files changed, 8 insertions(+), 97 deletions(-) diff --git a/.travis.yml b/.travis.yml index b35db32..a6e4bd5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,14 +15,7 @@ matrix: # pylint has dropped 2.6 support -.- - python: "2.6" env: TOXENV=lint -install: - - pip install -r requirements.d/travis.txt - # My tests need some package to be reliably installed to system-site-packages. - # This should maybe be a fixture of some kind, - # but I'd have to build python from source I think =/ - - SYS_REAL_PREFIX=$(python -c 'import sys; print(sys.real_prefix)') - - echo $SYS_REAL_PREFIX - - sudo $SYS_REAL_PREFIX/bin/pip install virtualenv +install: pip install -r requirements.d/travis.txt script: tox after_success: - find -name '.coverage*' | xargs mv -t . @@ -32,11 +25,12 @@ after_failure: # attempt to show any pip.log on failure - find -name pip.log | xargs -r tail -n99999 -sudo: true + +# public caches only supported under sudo:false currently +# closest thing to documentation: +# http://blog.travis-ci.com/2014-10-07-free-builds-for-students-github-student-developer-pack +sudo: false cache: - # public caches only supported under sudo:false currently - # closest thing to documentation: - # http://blog.travis-ci.com/2014-10-07-free-builds-for-students-github-student-developer-pack directories: - $HOME/.pip - $HOME/.pre-commit diff --git a/requirements.d/_lint.txt b/requirements.d/_lint.txt index 8065510..bb2ccee 100644 --- a/requirements.d/_lint.txt +++ b/requirements.d/_lint.txt @@ -1,12 +1,7 @@ # packages needed for linting, but not for prod flake8 -pre-commit - # pylint # PR merged: https://bitbucket.org/logilab/pylint/pull-request/186/fixed_up_import_test-and-tests/diff # pending fresh release: https://bitbucket.org/logilab/pylint/issue/363/please-cut-a-release-131 # note: pylint is set up such that -e simply doesn't work. Their __init__ is in their top directory. hg+https://bitbucket.org/logilab/pylint@58c66aa083777059a2e6b46f6a0545a2f4977097#egg=pylint - -# astroid 1.3.0 is broken. -# see: https://bitbucket.org/logilab/astroid/issue/55/astroid-130-explodes-on-import-pytest -astroid!=1.3.0 +pre-commit diff --git a/tests/functional/relocation_test.py b/tests/functional/relocation_test.py index 0aad05b..ea266f1 100644 --- a/tests/functional/relocation_test.py +++ b/tests/functional/relocation_test.py @@ -34,42 +34,3 @@ def test_is_relocatable_different_python_version(tmpdir): venv_update(python_arg) run('sh', '-c', '. virtualenv_run/bin/activate && doge --help') - - -def _get_virtualenv_data(capfd): - out, err = capfd.readouterr() # flush buffers - Path('assertions.py').write(''' -import json -import sys, virtualenv -print(json.dumps((virtualenv.__file__, sys.prefix, sys.real_prefix))) -''') - run('virtualenv_run/bin/python', 'assertions.py') - - out, err = capfd.readouterr() - assert err == '' - - from json import loads - lastline = out.splitlines()[-1] - return loads(lastline) - - -def path_is_within(path, within): - from os.path import relpath - return not relpath(path, within).startswith('..') - - -def test_is_relocatable_system_site_packages(tmpdir, capfd): - tmpdir.chdir() - requirements = Path('requirements.txt') - - # first show that virtualenv is installed to the system python - # then show that virtualenv is installed to the virtualenv python, when it's required - for reqs, invenv in ( - ('', False), - ('virtualenv\n', True), - ): - requirements.write(reqs) - venv_update('--system-site-packages') - virtualenv__file__, prefix, real_prefix = _get_virtualenv_data(capfd) - assert path_is_within(virtualenv__file__, prefix) == invenv - assert path_is_within(virtualenv__file__, real_prefix) == (not invenv) diff --git a/tests/functional/simple_test.py b/tests/functional/simple_test.py index eaa0674..19c53ed 100644 --- a/tests/functional/simple_test.py +++ b/tests/functional/simple_test.py @@ -4,7 +4,7 @@ import pytest from testing import ( - get_scenario, run, strip_coverage_warnings, venv_update, venv_update_script, TOP, + get_scenario, run, strip_coverage_warnings, venv_update, TOP, ) from sys import version_info @@ -242,29 +242,3 @@ def test_uncolored_pipe(): out = pipe_output(read, write) assert not unprintable(out), out - - -@pytest.mark.parametrize("options,expected", [ - (('--system-site-packages',), True), - ((), False), -]) -def test_venv_uses_system_site_packages(capfd, tmpdir, options, expected): - tmpdir.chdir() - Path('requirements.txt').write('') - - for options, expected in ( - (options, expected), - # also show that going back and forth works - ((), False), - (('--system-site-packages',), True), - ): - venv_update(*options) - - out, err = capfd.readouterr() # flush buffers - venv_update_script('''\ -from venv_update import venv_uses_system_site_packages -print(venv_uses_system_site_packages())''') - - out, err = capfd.readouterr() # flush buffers - assert err == '' - assert out == '%s\n' % expected diff --git a/venv_update.py b/venv_update.py index f0eb52b..73043c4 100644 --- a/venv_update.py +++ b/venv_update.py @@ -255,12 +255,6 @@ def venv(venv_path, venv_args): ) -def venv_uses_system_site_packages(): - from glob import glob - from sys import prefix - return not glob(prefix + '/lib/python*/no-global-site-packages.txt') - - def do_install(reqs): from os import environ @@ -296,13 +290,6 @@ def do_install(reqs): # 3) Install: Use our well-populated cache (only) to do the installations. install_opts += ('--upgrade', '--no-index',) - if venv_uses_system_site_packages(): - # In the worst case, a --system-site-packages venv will be missing packages because it was built in an - # environment which satisfied requirements at the system level, then be relocated to a system that doesn't have - # the requirement, breaking it. This has bitten us before. --ignore-installed avoids the issue. - print('Detected --system-site-packages; using --ignore-installed. This makes things slower.') - install_opts += ('--ignore-installed',) - recently_installed = pip_install(install_opts + requirements_as_options) required_with_deps = trace_requirements(