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

V2 ./pants test.pytest selects interpreter based off of compatibility constraints #7679

Merged
merged 15 commits into from May 13, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 26 additions & 7 deletions src/python/pants/backend/python/rules/python_test_runner.py
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):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bikeshed time! In rules files, should we have helper functions like this appear above or below the main @rule and def rules()?

I have no preference and really don't know what's best.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Bikeshed time! In rules files, should we have helper functions like this appear above or below the main @rule and def rules()?

IMO, it should appear nearest to the @rule that it is being used by. If there were more rules in this file, they might appear above/below a static function for that reason.

But it doesn't really matter.

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 @@ -116,4 +134,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
@@ -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',
]
)
@@ -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"
]
)