Skip to content

Commit

Permalink
Fixup tests involving pexrc. (#6446)
Browse files Browse the repository at this point in the history
Previously these tests weren't reliably reading a pexrc and had fragile
workarounds and paper-overs as a result. pexrc_util now reliably injects
a pexrc with a temporary HOME.

Also cleanup the affected tests - use @skipIf, respect 100 cols, ensure
path comparisons with judicious application of `os.realpath`.
  • Loading branch information
jsirois committed Sep 6, 2018
1 parent 5697ee6 commit c06e831
Show file tree
Hide file tree
Showing 21 changed files with 326 additions and 319 deletions.
Expand Up @@ -5,6 +5,7 @@ python_tests(
name='mypy_integration',
sources=['test_mypy_integration.py'],
dependencies=[
'tests/python/pants_test/backend/python:interpreter_selection_utils',
'tests/python/pants_test:int-test',
],
tags={'integration'},
Expand Down
Expand Up @@ -4,6 +4,7 @@

from __future__ import absolute_import, division, print_function, unicode_literals

from pants_test.backend.python.interpreter_selection_utils import PY_3, has_python_version
from pants_test.pants_run_integration_test import PantsRunIntegrationTest


Expand All @@ -15,7 +16,7 @@ def test_mypy(self):
'--',
'--follow-imports=silent'
]
if self.has_python_version('3'):
if has_python_version(PY_3):
# Python 3.x is available. Test that we see an error in this integration test.
with self.pants_results(cmd) as pants_run:
self.assert_success(pants_run)
Expand Down
32 changes: 16 additions & 16 deletions src/python/pants/backend/python/interpreter_cache.py
Expand Up @@ -37,13 +37,13 @@ class UnsatisfiableInterpreterConstraintsError(TaskError):
"""Indicates a python interpreter matching given constraints could not be located."""

@staticmethod
def _matches(interpreter, filters):
return any(interpreter.identity.matches(filt) for filt in filters)
def _matches(interpreter, filters=()):
return not filters or any(interpreter.identity.matches(filt) for filt in filters)

@classmethod
def _matching(cls, interpreters, filters):
def _matching(cls, interpreters, filters=()):
for interpreter in interpreters:
if cls._matches(interpreter, filters):
if cls._matches(interpreter, filters=filters):
yield interpreter

@classmethod
Expand Down Expand Up @@ -103,13 +103,13 @@ def select_interpreter_for_targets(self, targets):
# Return the lowest compatible interpreter.
return min(allowed_interpreters)

def _interpreter_from_path(self, path, filters):
def _interpreter_from_path(self, path, filters=()):
try:
executable = os.readlink(os.path.join(path, 'python'))
except OSError:
return None
interpreter = PythonInterpreter.from_binary(executable, include_site_extras=False)
if self._matches(interpreter, filters):
if self._matches(interpreter, filters=filters):
return self._resolve(interpreter)
return None

Expand All @@ -119,29 +119,29 @@ def _setup_interpreter(self, interpreter, cache_target_path):
os.symlink(interpreter.binary, os.path.join(safe_path, 'python'))
return self._resolve(interpreter, safe_path)

def _setup_cached(self, filters):
def _setup_cached(self, filters=()):
"""Find all currently-cached interpreters."""
for interpreter_dir in os.listdir(self._cache_dir):
path = os.path.join(self._cache_dir, interpreter_dir)
if os.path.isdir(path):
pi = self._interpreter_from_path(path, filters)
pi = self._interpreter_from_path(path, filters=filters)
if pi:
self._logger('Detected interpreter {}: {}'.format(pi.binary, str(pi.identity)))
yield pi

def _setup_paths(self, paths, filters):
def _setup_paths(self, paths, filters=()):
"""Find interpreters under paths, and cache them."""
for interpreter in self._matching(PythonInterpreter.all(paths), filters):
for interpreter in self._matching(PythonInterpreter.all(paths), filters=filters):
identity_str = str(interpreter.identity)
cache_path = os.path.join(self._cache_dir, identity_str)
pi = self._interpreter_from_path(cache_path, filters)
pi = self._interpreter_from_path(cache_path, filters=filters)
if pi is None:
self._setup_interpreter(interpreter, cache_path)
pi = self._interpreter_from_path(cache_path, filters)
pi = self._interpreter_from_path(cache_path, filters=filters)
if pi:
yield pi

def setup(self, paths=(), filters=(b'',)):
def setup(self, paths=(), filters=()):
"""Sets up a cache of python interpreters.
:param paths: The paths to search for a python interpreter; the system ``PATH`` by default.
Expand All @@ -168,14 +168,14 @@ def unsatisfied_filters(interpreters):

interpreters = []
with OwnerPrintingInterProcessFileLock(path=os.path.join(self._cache_dir, '.file_lock')):
interpreters.extend(self._setup_cached(filters))
interpreters.extend(self._setup_cached(filters=filters))
if unsatisfied_filters(interpreters):
interpreters.extend(self._setup_paths(setup_paths, filters))
interpreters.extend(self._setup_paths(setup_paths, filters=filters))

for filt in unsatisfied_filters(interpreters):
self._logger('No valid interpreters found for {}!'.format(filt))

matches = list(self._matching(interpreters, filters))
matches = list(self._matching(interpreters, filters=filters))
if len(matches) == 0:
self._logger('Found no valid interpreters!')

Expand Down
Expand Up @@ -5,6 +5,7 @@
from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

import os
import sys

from interpreter_selection.python_3_selection_testing.lib_py2 import say_something
Expand All @@ -15,7 +16,7 @@

def main():
v = sys.version_info
print(sys.executable)
print(os.path.realpath(sys.executable))
print('%d.%d.%d' % v[0:3])
return say_something()

Expand Down
Expand Up @@ -5,6 +5,7 @@
from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

import os
import sys

from interpreter_selection.python_3_selection_testing.lib_py23 import say_something
Expand All @@ -15,7 +16,7 @@

def main():
v = sys.version_info
print(sys.executable)
print(os.path.realpath(sys.executable))
print('%d.%d.%d' % v[0:3])
return say_something()

Expand Down
Expand Up @@ -2,6 +2,7 @@
# Copyright 2017 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import os
import sys

from interpreter_selection.python_3_selection_testing.lib_py3 import say_something
Expand All @@ -12,7 +13,7 @@

def main():
v = sys.version_info
print(sys.executable)
print(os.path.realpath(sys.executable))
print('%d.%d.%d' % v[0:3])
return say_something()

Expand Down
@@ -1,7 +1,7 @@
python_binary(
name='test_bin',
source='main.py',
compatibility=['CPython>3'],
compatibility=['CPython>=3.6'],
dependencies=[
':reqlib'
]
Expand Down
@@ -1,3 +1,5 @@
from concurrent.futures import Future


print(Future)
print('Successful.')
@@ -1,3 +1,5 @@
import jupyter


print(jupyter)
print('Successful.')
15 changes: 12 additions & 3 deletions tests/python/pants_test/backend/python/BUILD
Expand Up @@ -31,13 +31,14 @@ python_tests(
'3rdparty/python:future',
'3rdparty/python:mock',
'3rdparty/python:pex',
'src/python/pants/backend/python:interpreter_cache',
':interpreter_selection_utils',
'src/python/pants/backend/python/subsystems',
'src/python/pants/backend/python:interpreter_cache',
'src/python/pants/util:contextutil',
'tests/python/pants_test:test_base',
'tests/python/pants_test:int-test',
'tests/python/pants_test/testutils:git_util',
'tests/python/pants_test/testutils:pexrc_util',
'tests/python/pants_test:int-test',
'tests/python/pants_test:test_base',
],
timeout=1200
)
Expand All @@ -64,3 +65,11 @@ python_library(
'tests/python/pants_test:int-test',
]
)

python_library(
name='interpreter_selection_utils',
sources=['interpreter_selection_utils.py'],
dependencies=[
'src/python/pants/util:process_handler'
]
)
@@ -0,0 +1,79 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

import os
from unittest import skipIf

from pants.util.process_handler import subprocess


PY_27 = '2.7'
PY_3 = '3'
PY_36 = '3.6'


def has_python_version(version):
"""Returns `True` if the current system has the specified version of python.
:param string version: A python version string, such as 2.7, 3.
"""
return python_interpreter_path(version) is not None


def python_interpreter_path(version):
"""Returns the interpreter path if the current system has the specified version of python.
:param string version: A python version string, such as 2.7, 3.
:returns: the normalized path to the interpreter binary if found; otherwise `None`
:rtype: string
"""
try:
command = ['python{}'.format(version), '-c', 'import sys; print(sys.executable)']
py_path = subprocess.check_output(command).decode('utf-8').strip()
return os.path.realpath(py_path)
except subprocess.CalledProcessError:
return None


def skip_unless_pythons(*versions):
"""A decorator that only runs the decorated test method if the specified pythons are present.
:param string *versions: Python version strings, such as 2.7, 3.
"""
missing_versions = [v for v in versions if not has_python_version(v)]
if len(missing_versions) == 1:
return skipIf(True, 'Could not find python {} on system. Skipping.'.format(missing_versions[0]))
elif len(missing_versions) > 1:
return skipIf(True,
'Skipping due to the following missing required pythons: {}'
.format(', '.join(missing_versions)))
else:
return skipIf(False, 'All required pythons present, continuing with test!')


def skip_unless_python27(func):
"""A test skip decorator that only runs a test method if python2.7 is present."""
return skip_unless_pythons(PY_27)(func)


def skip_unless_python3(func):
"""A test skip decorator that only runs a test method if python3 is present."""
return skip_unless_pythons(PY_3)(func)


def skip_unless_python36(func):
"""A test skip decorator that only runs a test method if python3.6 is present."""
return skip_unless_pythons(PY_36)(func)


def skip_unless_python27_and_python3(func):
"""A test skip decorator that only runs a test method if python2.7 and python3 are present."""
return skip_unless_pythons(PY_27, PY_3)(func)


def skip_unless_python27_and_python36(func):
"""A test skip decorator that only runs a test method if python2.7 and python3.6 are present."""
return skip_unless_pythons(PY_27, PY_36)(func)
5 changes: 3 additions & 2 deletions tests/python/pants_test/backend/python/tasks/BUILD
Expand Up @@ -93,9 +93,10 @@ python_tests(
'src/python/pants/base:build_environment',
'src/python/pants/util:contextutil',
'src/python/pants/util:process_handler',
'tests/python/pants_test:int-test',
'tests/python/pants_test/backend/python:pants_requirement_integration_test_base',
'tests/python/pants_test/backend/python/tasks:python_task_test_base',
'tests/python/pants_test/backend/python:interpreter_selection_utils',
'tests/python/pants_test/backend/python:pants_requirement_integration_test_base',
'tests/python/pants_test:int-test',
],
tags={'integration'},
timeout=2400
Expand Down
Expand Up @@ -8,6 +8,8 @@

from pants.util.contextutil import temporary_dir
from pants.util.process_handler import subprocess
from pants_test.backend.python.interpreter_selection_utils import (PY_3, PY_27, skip_unless_python3,
skip_unless_python27)
from pants_test.pants_run_integration_test import PantsRunIntegrationTest


Expand Down Expand Up @@ -35,23 +37,20 @@ def test_conflict_via_config(self):
self.assertIn('Unable to detect a suitable interpreter for compatibilities',
pants_run.stdout_data)

@skip_unless_python3
def test_select_3(self):
self._maybe_test_version('3')
self._test_version(PY_3)

@skip_unless_python27
def test_select_27(self):
self._maybe_test_version('2.7')
self._test_version(PY_27)

def _maybe_test_version(self, version):
if self.has_python_version(version):
print('Found python {}. Testing interpreter selection against it.'.format(version))
echo = self._echo_version(version)
v = echo.split('.') # E.g., 2.7.13.
self.assertTrue(len(v) > 2, 'Not a valid version string: {}'.format(v))
expected_components = version.split('.')
self.assertEqual(expected_components, v[:len(expected_components)])
else:
print('No python {} found. Skipping.'.format(version))
self.skipTest('No python {} on system'.format(version))
def _test_version(self, version):
echo = self._echo_version(version)
v = echo.split('.') # E.g., 2.7.13.
self.assertTrue(len(v) > 2, 'Not a valid version string: {}'.format(v))
expected_components = version.split('.')
self.assertEqual(expected_components, v[:len(expected_components)])

def _echo_version(self, version):
with temporary_dir() as distdir:
Expand All @@ -71,10 +70,10 @@ def _echo_version(self, version):
(stdout_data, _) = proc.communicate()
return stdout_data

def _build_pex(self, binary_target, config=None, args=None, version='2.7'):
def _build_pex(self, binary_target, config=None, args=None, version=PY_27):
# By default, Avoid some known-to-choke-on interpreters.
if version == '3':
constraint = '["CPython>=3.3"]'
if version == PY_3:
constraint = '["CPython>=3.4,<4"]'
else:
constraint = '["CPython>=2.7,<3"]'
args = list(args) if args is not None else [
Expand Down

0 comments on commit c06e831

Please sign in to comment.