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

Minimize interpreter bootstrapping in tests. #571

Merged
merged 3 commits into from
Oct 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 19 additions & 22 deletions pex/bin/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -557,19 +557,7 @@ def installer_provider(sdist):
return interpreter.with_extra(egg.name, egg.raw_version, egg.path)


def get_interpreter(python_interpreter, interpreter_cache_dir, repos, use_wheel):
interpreter = None

if python_interpreter:
if os.path.exists(python_interpreter):
interpreter = PythonInterpreter.from_binary(python_interpreter)
else:
interpreter = PythonInterpreter.from_env(python_interpreter)
if interpreter is None:
die('Failed to find interpreter: %s' % python_interpreter)
else:
interpreter = PythonInterpreter.get()

def setup_interpreter(interpreter, interpreter_cache_dir, repos, use_wheel):
with TRACER.timed('Setting up interpreter %s' % interpreter.binary, V=2):
resolve = functools.partial(resolve_interpreter, interpreter_cache_dir, repos)

Expand All @@ -585,13 +573,16 @@ def get_interpreter(python_interpreter, interpreter_cache_dir, repos, use_wheel)

def build_pex(args, options, resolver_option_builder):
with TRACER.timed('Resolving interpreters', V=2):
interpreters = [
get_interpreter(interpreter,
options.interpreter_cache_dir,
options.repos,
options.use_wheel)
for interpreter in options.python or [None]
]
def to_python_interpreter(full_path_or_basename):
if os.path.exists(full_path_or_basename):
return PythonInterpreter.from_binary(full_path_or_basename)
else:
interpreter = PythonInterpreter.from_env(full_path_or_basename)
if interpreter is None:
die('Failed to find interpreter: %s' % full_path_or_basename)
return interpreter

interpreters = [to_python_interpreter(interp) for interp in options.python or [sys.executable]]

if options.interpreter_constraint:
# NB: options.python and interpreter constraints cannot be used together, so this will not
Expand All @@ -602,7 +593,13 @@ def build_pex(args, options, resolver_option_builder):
pex_python_path = rc_variables.get('PEX_PYTHON_PATH', '')
interpreters = find_compatible_interpreters(pex_python_path, constraints)

if not interpreters:
setup_interpreters = [setup_interpreter(interp,
options.interpreter_cache_dir,
options.repos,
options.use_wheel)
for interp in interpreters]

if not setup_interpreters:
die('Could not find compatible interpreter', CANNOT_SETUP_INTERPRETER)

try:
Expand All @@ -612,7 +609,7 @@ def build_pex(args, options, resolver_option_builder):
# options.preamble_file is None
preamble = None

interpreter = min(interpreters)
interpreter = min(setup_interpreters)

pex_builder = PEXBuilder(path=safe_mkdtemp(), interpreter=interpreter, preamble=preamble)

Expand Down
14 changes: 14 additions & 0 deletions pex/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,21 @@ def bootstrap_python_installer(dest):
raise RuntimeError("Helper method could not clone pyenv from git after 3 tries")


# NB: We keep the pool of bootstrapped interpreters as small as possible to avoid timeouts in CI
# otherwise encountered when fetching and building too many on a cache miss. In the past we had
# issues with the combination of 7 total unique interpreter versions and a Travis-CI timeout of 50
# minutes for a shard.
PY27 = '2.7.15'
PY35 = '3.5.6'
PY36 = '3.6.6'

_VERSIONS = (PY27, PY35, PY36)


def ensure_python_distribution(version):
if version not in _VERSIONS:
raise ValueError('Please constrain version to one of {}'.format(_VERSIONS))

pyenv_root = os.path.join(os.getcwd(), '.pyenv_test')
interpreter_location = os.path.join(pyenv_root, 'versions', version)
pyenv = os.path.join(pyenv_root, 'bin', 'pyenv')
Expand Down
9 changes: 5 additions & 4 deletions tests/test_installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@

import pytest

from pex.bin.pex import get_interpreter
from pex.bin.pex import setup_interpreter
from pex.installer import WheelInstaller
from pex.testing import ensure_python_interpreter, make_installer, temporary_dir
from pex.interpreter import PythonInterpreter
from pex.testing import PY36, ensure_python_interpreter, make_installer, temporary_dir
from pex.version import SETUPTOOLS_REQUIREMENT, WHEEL_REQUIREMENT


Expand All @@ -23,8 +24,8 @@ def mixins(self):
@contextlib.contextmanager
def bare_interpreter():
with temporary_dir() as interpreter_cache:
yield get_interpreter(
python_interpreter=ensure_python_interpreter('3.6.3'),
yield setup_interpreter(
interpreter=PythonInterpreter.from_binary(ensure_python_interpreter(PY36)),
interpreter_cache_dir=interpreter_cache,
repos=None,
use_wheel=True
Expand Down
36 changes: 19 additions & 17 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
NOT_CPYTHON27_OR_OSX,
NOT_CPYTHON36,
NOT_CPYTHON36_OR_LINUX,
PY27,
PY36,
ensure_python_interpreter,
get_dep_dist_names_from_pex,
run_pex_command,
Expand Down Expand Up @@ -354,7 +356,7 @@ def test_interpreter_constraints_to_pex_info_py2():

@pytest.mark.skipif(IS_PYPY)
def test_interpreter_constraints_to_pex_info_py3():
py3_interpreter = ensure_python_interpreter('3.6.3')
py3_interpreter = ensure_python_interpreter(PY36)
with environment_as(PATH=os.path.dirname(py3_interpreter)):
with temporary_dir() as output_dir:
# target python 3
Expand Down Expand Up @@ -387,8 +389,8 @@ def test_interpreter_resolution_with_pex_python_path():
with open(pexrc_path, 'w') as pexrc:
# set pex python path
pex_python_path = ':'.join([
ensure_python_interpreter('2.7.10'),
ensure_python_interpreter('3.6.3')
ensure_python_interpreter(PY27),
ensure_python_interpreter(PY36)
])
pexrc.write("PEX_PYTHON_PATH=%s" % pex_python_path)

Expand Down Expand Up @@ -422,8 +424,8 @@ def test_interpreter_resolution_pex_python_path_precedence_over_pex_python():
with open(pexrc_path, 'w') as pexrc:
# set both PPP and PP
pex_python_path = ':'.join([
ensure_python_interpreter('2.7.10'),
ensure_python_interpreter('3.6.3')
ensure_python_interpreter(PY27),
ensure_python_interpreter(PY36)
])
pexrc.write("PEX_PYTHON_PATH=%s\n" % pex_python_path)
pex_python = '/path/to/some/python'
Expand Down Expand Up @@ -464,8 +466,8 @@ def test_pex_exec_with_pex_python_path_only():
with open(pexrc_path, 'w') as pexrc:
# set pex python path
pex_python_path = ':'.join([
ensure_python_interpreter('2.7.10'),
ensure_python_interpreter('3.6.3')
ensure_python_interpreter(PY27),
ensure_python_interpreter(PY36)
])
pexrc.write("PEX_PYTHON_PATH=%s" % pex_python_path)

Expand All @@ -490,8 +492,8 @@ def test_pex_exec_with_pex_python_path_and_pex_python_but_no_constraints():
with open(pexrc_path, 'w') as pexrc:
# set both PPP and PP
pex_python_path = ':'.join([
ensure_python_interpreter('2.7.10'),
ensure_python_interpreter('3.6.3')
ensure_python_interpreter(PY27),
ensure_python_interpreter(PY36)
])
pexrc.write("PEX_PYTHON_PATH=%s\n" % pex_python_path)
pex_python = '/path/to/some/python'
Expand All @@ -513,14 +515,14 @@ def test_pex_exec_with_pex_python_path_and_pex_python_but_no_constraints():

@pytest.mark.skipif(IS_PYPY)
def test_pex_python():
py2_path_interpreter = ensure_python_interpreter('2.7.10')
py3_path_interpreter = ensure_python_interpreter('3.6.3')
py2_path_interpreter = ensure_python_interpreter(PY27)
py3_path_interpreter = ensure_python_interpreter(PY36)
path = ':'.join([os.path.dirname(py2_path_interpreter), os.path.dirname(py3_path_interpreter)])
with environment_as(PATH=path):
with temporary_dir() as td:
pexrc_path = os.path.join(td, '.pexrc')
with open(pexrc_path, 'w') as pexrc:
pex_python = ensure_python_interpreter('3.6.3')
pex_python = ensure_python_interpreter(PY36)
pexrc.write("PEX_PYTHON=%s" % pex_python)

# test PEX_PYTHON with valid constraints
Expand All @@ -541,7 +543,7 @@ def test_pex_python():
# test PEX_PYTHON with incompatible constraints
pexrc_path = os.path.join(td, '.pexrc')
with open(pexrc_path, 'w') as pexrc:
pex_python = ensure_python_interpreter('2.7.10')
pex_python = ensure_python_interpreter(PY27)
pexrc.write("PEX_PYTHON=%s" % pex_python)

pex_out_path = os.path.join(td, 'pex2.pex')
Expand Down Expand Up @@ -578,7 +580,7 @@ def test_entry_point_targeting():
with temporary_dir() as td:
pexrc_path = os.path.join(td, '.pexrc')
with open(pexrc_path, 'w') as pexrc:
pex_python = ensure_python_interpreter('3.6.3')
pex_python = ensure_python_interpreter(PY36)
pexrc.write("PEX_PYTHON=%s" % pex_python)

# test pex with entry point
Expand Down Expand Up @@ -609,9 +611,9 @@ def test_interpreter_selection_using_os_environ_for_bootstrap_reexec():
# execute with. The child interpreter is the interpreter we expect the
# child pex to execute with.
if (sys.version_info[0], sys.version_info[1]) == (3, 6):
child_pex_interpreter_version = '3.6.3'
child_pex_interpreter_version = PY36
else:
child_pex_interpreter_version = '2.7.10'
child_pex_interpreter_version = PY27

# Write parent pex's pexrc.
with open(pexrc_path, 'w') as pexrc:
Expand Down Expand Up @@ -993,7 +995,7 @@ def test_invalid_entry_point_verification_3rdparty():
def test_multiplatform_entrypoint():
with temporary_dir() as td:
pex_out_path = os.path.join(td, 'p537.pex')
interpreter = ensure_python_interpreter('3.6.3')
interpreter = ensure_python_interpreter(PY36)
res = run_pex_command(['p537==1.0.3',
'--no-build',
'--python={}'.format(interpreter),
Expand Down
19 changes: 12 additions & 7 deletions tests/test_interpreter.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
import pytest

from pex import interpreter
from pex.compatibility import PY3
from pex.testing import (
IS_PYPY,
PY27,
PY35,
ensure_python_distribution,
ensure_python_interpreter,
temporary_dir
Expand All @@ -20,24 +23,26 @@
from unittest.mock import patch


def version_from_tuple(version_tuple):
return '.'.join(str(x) for x in version_tuple)
def tuple_from_version(version_string):
return tuple(int(component) for component in version_string.split('.'))


class TestPythonInterpreter(object):

@pytest.mark.skipif('sys.version_info >= (3,0)')
@pytest.mark.skipif(PY3,
reason='This test relies on the `reload` builtin which is not available in '
'python 3 as a builtin.')
def test_all_does_not_raise_with_empty_path_envvar(self):
""" additionally, tests that the module does not raise at import """
with patch.dict(os.environ, clear=True):
reload(interpreter)
interpreter.PythonInterpreter.all()

TEST_INTERPRETER1_VERSION_TUPLE = (2, 7, 10)
TEST_INTERPRETER1_VERSION = version_from_tuple(TEST_INTERPRETER1_VERSION_TUPLE)
TEST_INTERPRETER1_VERSION = PY27
TEST_INTERPRETER1_VERSION_TUPLE = tuple_from_version(TEST_INTERPRETER1_VERSION)

TEST_INTERPRETER2_VERSION_TUPLE = (2, 7, 9)
TEST_INTERPRETER2_VERSION = version_from_tuple(TEST_INTERPRETER2_VERSION_TUPLE)
TEST_INTERPRETER2_VERSION = PY35
TEST_INTERPRETER2_VERSION_TUPLE = tuple_from_version(TEST_INTERPRETER2_VERSION)

@pytest.fixture
def test_interpreter1(self):
Expand Down
10 changes: 6 additions & 4 deletions tests/test_pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import pytest
from twitter.common.contextutil import temporary_file

from pex.bin.pex import get_interpreter
from pex.bin.pex import setup_interpreter
from pex.compatibility import PY2, WINDOWS, nested, to_bytes
from pex.installer import EggInstaller, WheelInstaller
from pex.interpreter import PythonInterpreter
Expand All @@ -21,6 +21,8 @@
from pex.resolver import resolve
from pex.testing import (
IS_PYPY,
PY27,
PY36,
ensure_python_interpreter,
make_bdist,
make_installer,
Expand Down Expand Up @@ -322,9 +324,9 @@ def test_pex_verify_entry_point_module_should_fail():
@pytest.mark.skipif(IS_PYPY)
def test_activate_interpreter_different_from_current():
with temporary_dir() as pex_root:
interp_version = '3.6.3' if PY2 else '2.7.10'
custom_interpreter = get_interpreter(
python_interpreter=ensure_python_interpreter(interp_version),
interp_version = PY36 if PY2 else PY27
custom_interpreter = setup_interpreter(
interpreter=PythonInterpreter.from_binary(ensure_python_interpreter(interp_version)),
interpreter_cache_dir=os.path.join(pex_root, 'interpreters'),
repos=None, # Default to PyPI.
use_wheel=True
Expand Down
50 changes: 18 additions & 32 deletions tests/test_pex_bootstrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from pex.common import open_zip
from pex.interpreter import PythonInterpreter
from pex.pex_bootstrapper import find_compatible_interpreters, get_pex_info
from pex.testing import IS_PYPY, ensure_python_interpreter, write_simple_pex
from pex.testing import IS_PYPY, PY27, PY35, PY36, ensure_python_interpreter, write_simple_pex


def test_get_pex_info():
Expand All @@ -34,39 +34,25 @@ def test_get_pex_info():

@pytest.mark.skipif(IS_PYPY)
def test_find_compatible_interpreters():
pex_python_path = ':'.join([
ensure_python_interpreter('2.7.9'),
ensure_python_interpreter('2.7.10'),
ensure_python_interpreter('2.7.11'),
ensure_python_interpreter('3.4.2'),
ensure_python_interpreter('3.5.4'),
ensure_python_interpreter('3.6.2'),
ensure_python_interpreter('3.6.3')
])
py27 = ensure_python_interpreter(PY27)
py35 = ensure_python_interpreter(PY35)
py36 = ensure_python_interpreter(PY36)
pex_python_path = ':'.join([py27, py35, py36])

interpreters = find_compatible_interpreters(pex_python_path, ['>3'])
assert interpreters[0].binary == pex_python_path.split(':')[3] # 3.4.2
def find_interpreters(*constraints):
return [interp.binary for interp in find_compatible_interpreters(pex_python_path, constraints)]

interpreters = find_compatible_interpreters(pex_python_path, ['<3'])
assert interpreters[0].binary == pex_python_path.split(':')[0] # 2.7.9
assert [py35, py36] == find_interpreters('>3')
assert [py27] == find_interpreters('<3')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I really like how readable these tests are.


interpreters = find_compatible_interpreters(pex_python_path, ['>3.5.4'])
assert interpreters[0].binary == pex_python_path.split(':')[5] # 3.6.2
assert [py36] == find_interpreters('>{}'.format(PY35))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of the reason I made these tests at this level of granularity was to assure that patch-level version constraints between constraints with the same major versions are respected, however I would imagine that is covered in testing elsewhere so this change is fine with me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. Yeah, the underlying comparison is done by PythonIdentity.matches which is actually not unit-tested, but it could and should be. The testing here was just way too expensive.

assert [py35] == find_interpreters('>{}, <{}'.format(PY27, PY36))
assert [py36] == find_interpreters('>=3.6')

interpreters = find_compatible_interpreters(pex_python_path, ['>3.4.2, <3.6'])
assert interpreters[0].binary == pex_python_path.split(':')[4] # 3.5.4
assert [] == find_interpreters('<2')
assert [] == find_interpreters('>4')
assert [] == find_interpreters('>{}, <{}'.format(PY27, PY35))

interpreters = find_compatible_interpreters(pex_python_path, ['>3.6.2'])
assert interpreters[0].binary == pex_python_path.split(':')[6] # 3.6.3

interpreters = find_compatible_interpreters(pex_python_path, ['<2'])
assert not interpreters

interpreters = find_compatible_interpreters(pex_python_path, ['>4'])
assert not interpreters

interpreters = find_compatible_interpreters(pex_python_path, ['<2.7.11, >2.7.9'])
assert interpreters[0].binary == pex_python_path.split(':')[1] # 2.7.10

interpreters = find_compatible_interpreters('', ['<3'])
assert interpreters[0] in PythonInterpreter.all() # All interpreters on PATH
# All interpreters on PATH.
interpreters = find_compatible_interpreters(pex_python_path='', compatibility_constraints=['<3'])
assert set(interpreters).issubset(set(PythonInterpreter.all()))