Skip to content

Commit

Permalink
V2 ./pants test.pytest selects interpreter based off of compatibili…
Browse files Browse the repository at this point in the history
…ty constraints (pantsbuild#7679)

Currently the V2 task will always use the current interpreter to run Pytest, even if the targets have different requirements. Instead, we must support choosing the interpreter for the subprocess based on the targets' compatibility (which falls back to global compatibility if missing).

Note that we let Pex do the intepreter resolution for us to simplify the Pants code. All we have to do is pass the interpreter constraints to PEX, along with exposing the `--python-setup-interpreter-search-paths`, and then Pex will handle the resolution for us.
  • Loading branch information
Eric-Arellano authored May 13, 2019
1 parent 55529b0 commit 6f42467
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 7 deletions.
33 changes: 26 additions & 7 deletions src/python/pants/backend/python/rules/python_test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@

from __future__ import absolute_import, division, print_function, unicode_literals

import os
import sys
from builtins import str

from future.utils import text_type

from pants.backend.python.subsystems.pytest import PyTest
from pants.backend.python.subsystems.python_setup import PythonSetup
from pants.engine.fs import Digest, MergedDirectories, Snapshot, UrlToFetch
from pants.engine.isolated_process import (ExecuteProcessRequest, ExecuteProcessResult,
FallibleExecuteProcessResult)
Expand All @@ -26,10 +28,24 @@ class PyTestResult(TestResult):
pass


def parse_interpreter_constraints(python_setup, python_target_adaptors):
constraints = {
constraint
for target_adaptor in python_target_adaptors
for constraint in python_setup.compatibility_or_constraints(
getattr(target_adaptor, 'compatibility', None)
)
}
constraints_args = []
for constraint in sorted(constraints):
constraints_args.extend(["--interpreter-constraint", constraint])
return constraints_args


# TODO: Support deps
# TODO: Support resources
@rule(PyTestResult, [TransitiveHydratedTarget, PyTest])
def run_python_test(transitive_hydrated_target, pytest):
@rule(PyTestResult, [TransitiveHydratedTarget, PyTest, PythonSetup])
def run_python_test(transitive_hydrated_target, pytest, python_setup):
target_root = transitive_hydrated_target.root

# TODO: Inject versions and digests here through some option, rather than hard-coding it.
Expand All @@ -50,30 +66,31 @@ def run_python_test(transitive_hydrated_target, pytest):
for py_req in maybe_python_req_lib.adaptor.requirements:
all_requirements.append(str(py_req.requirement))

# TODO: This should be configurable, both with interpreter constraints, and for remote execution.
# TODO(#7061): This str() can be removed after we drop py2!
python_binary = text_type(sys.executable)
interpreter_constraint_args = parse_interpreter_constraints(
python_setup, python_target_adaptors=[target.adaptor for target in all_targets]
)

# TODO: This is non-hermetic because the requirements will be resolved on the fly by
# pex27, where it should be hermetically provided in some way.
output_pytest_requirements_pex_filename = 'pytest-with-requirements.pex'
requirements_pex_argv = [
python_binary,
'./{}'.format(pex_snapshot.files[0]),
# TODO(#7061): This text_type() can be removed after we drop py2!
'--python', text_type(python_binary),
'-e', 'pytest:main',
'-o', output_pytest_requirements_pex_filename,
# Sort all user requirement strings to increase the chance of cache hits across invocations.
] + [
] + interpreter_constraint_args + [
# TODO(#7061): This text_type() wrapping can be removed after we drop py2!
text_type(req)
# Sort all user requirement strings to increase the chance of cache hits across invocations.
for req in sorted(
list(pytest.get_requirement_strings())
+ list(all_requirements))
]
requirements_pex_request = ExecuteProcessRequest(
argv=tuple(requirements_pex_argv),
env={'PATH': os.pathsep.join(python_setup.interpreter_search_paths)},
input_files=pex_snapshot.directory_digest,
description='Resolve requirements for {}'.format(target_root.address.reference()),
output_files=(output_pytest_requirements_pex_filename,),
Expand Down Expand Up @@ -101,6 +118,7 @@ def run_python_test(transitive_hydrated_target, pytest):

request = ExecuteProcessRequest(
argv=(python_binary, './{}'.format(output_pytest_requirements_pex_filename)),
env={'PATH': os.pathsep.join(python_setup.interpreter_search_paths)},
input_files=merged_input_files,
description='Run pytest for {}'.format(target_root.address.reference()),
)
Expand All @@ -119,4 +137,5 @@ def rules():
return [
run_python_test,
optionable_rule(PyTest),
optionable_rule(PythonSetup),
]
13 changes: 13 additions & 0 deletions tests/python/pants_test/backend/python/rules/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_tests(
name='python_test_runner',
sources=['test_python_test_runner.py'],
dependencies=[
'src/python/pants/backend/python/rules',
'src/python/pants/backend/python/subsystems',
'src/python/pants/engine/legacy:structs',
'tests/python/pants_test/subsystem:subsystem_utils',
]
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# coding=utf-8
# Copyright 2019 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

from unittest import TestCase

from pants.backend.python.rules.python_test_runner import parse_interpreter_constraints
from pants.backend.python.subsystems.python_setup import PythonSetup
from pants.engine.legacy.structs import PythonTargetAdaptor
from pants_test.subsystem.subsystem_util import global_subsystem_instance


class TestPythonTestRunner(TestCase):

def test_interpreter_constraints_parsing(self):
python_setup = global_subsystem_instance(PythonSetup)
target_adaptors = [
# NB: This target will use the global --python-setup-interpreter-constraints.
PythonTargetAdaptor(compatibility=None),
PythonTargetAdaptor(compatibility=["CPython>=400"]),
]
self.assertEqual(
parse_interpreter_constraints(python_setup, target_adaptors),
[
"--interpreter-constraint", "CPython>=2.7,<3",
"--interpreter-constraint", "CPython>=3.6,<4",
"--interpreter-constraint", "CPython>=400"
]
)

0 comments on commit 6f42467

Please sign in to comment.