Skip to content

Commit

Permalink
Consume the PyTest Subsystem to stabilize the pytest version. (pantsb…
Browse files Browse the repository at this point in the history
…uild#7193)

### Problem

Master is currently broken with python 2 due to a floating pytest version resulting in a lovely new Py2k warning overnight.

### Solution

Pin the v2 pytest runner to the pytest requirement string(s) provided by the `PyTest` `Subsystem`.

### Result

Less-floaty pytest versions.
  • Loading branch information
Stu Hood authored Feb 1, 2019
1 parent 0cc4467 commit 619586e
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/python/pants/backend/python/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from pants.backend.python.python_artifact import PythonArtifact
from pants.backend.python.python_requirement import PythonRequirement
from pants.backend.python.python_requirements import PythonRequirements
from pants.backend.python.rules.python_test_runner import run_python_test
from pants.backend.python.rules import python_test_runner
from pants.backend.python.targets.python_app import PythonApp
from pants.backend.python.targets.python_binary import PythonBinary
from pants.backend.python.targets.python_distribution import PythonDistribution
Expand Down Expand Up @@ -81,4 +81,4 @@ def register_goals():


def rules():
return (run_python_test,)
return tuple(python_test_runner.rules())
12 changes: 11 additions & 1 deletion src/python/pants/backend/python/rules/BUILD
Original file line number Diff line number Diff line change
@@ -1 +1,11 @@
python_library()
python_library(
dependencies=[
'src/python/pants/backend/python/subsystems',
'src/python/pants/engine/legacy:graph',
'src/python/pants/engine:fs',
'src/python/pants/engine:isolated_process',
'src/python/pants/engine:rules',
'src/python/pants/engine:selectors',
'src/python/pants/rules/core',
],
)
23 changes: 15 additions & 8 deletions src/python/pants/backend/python/rules/python_test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@
import sys
from builtins import str

from pants.backend.python.subsystems.pytest import PyTest
from pants.engine.fs import Digest, MergedDirectories, Snapshot, UrlToFetch
from pants.engine.isolated_process import (ExecuteProcessRequest, ExecuteProcessResult,
FallibleExecuteProcessResult)
from pants.engine.legacy.graph import TransitiveHydratedTarget
from pants.engine.rules import rule
from pants.engine.rules import optionable_rule, rule
from pants.engine.selectors import Get, Select
from pants.rules.core.core_test_model import Status, TestResult

Expand All @@ -26,8 +27,8 @@ class PyTestResult(TestResult):

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

# TODO: Inject versions and digests here through some option, rather than hard-coding it.
Expand Down Expand Up @@ -59,17 +60,16 @@ def run_python_test(transitive_hydrated_target):
# TODO: This should be configurable, both with interpreter constraints, and for remote execution.
python_binary = sys.executable

# 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 = [
'./{}'.format(pex_snapshot.files[0].path),
'--python', python_binary,
'-e', 'pytest:main',
'-o', output_pytest_requirements_pex_filename,
# TODO: This is non-hermetic because pytest will be resolved on the fly by pex27, where it should be hermetically provided in some way.
# We should probably also specify a specific version.
'pytest',
# Sort all the requirement strings to increase the chance of cache hits across invocations.
] + sorted(all_requirements)
# Sort all user requirement strings to increase the chance of cache hits across invocations.
] + list(pytest.get_requirement_strings()) + sorted(all_requirements)
requirements_pex_request = ExecuteProcessRequest(
argv=tuple(requirements_pex_argv),
input_files=pex_snapshot.directory_digest,
Expand Down Expand Up @@ -112,3 +112,10 @@ def run_python_test(transitive_hydrated_target):
status = Status.SUCCESS if result.exit_code == 0 else Status.FAILURE

yield PyTestResult(status=status, stdout=result.stdout.decode('utf-8'))


def rules():
return [
run_python_test,
optionable_rule(PyTest),
]
4 changes: 2 additions & 2 deletions src/python/pants/init/engine_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from pants.backend.docgen.targets.doc import Page
from pants.backend.jvm.targets.jvm_app import JvmApp
from pants.backend.jvm.targets.jvm_binary import JvmBinary
from pants.backend.python.rules.python_test_runner import run_python_test
from pants.backend.python.rules import python_test_runner
from pants.backend.python.targets.python_app import PythonApp
from pants.backend.python.targets.python_binary import PythonBinary
from pants.backend.python.targets.python_library import PythonLibrary
Expand Down Expand Up @@ -356,7 +356,7 @@ def setup_legacy_graph_extended(
create_graph_rules(address_mapper) +
create_options_parsing_rules() +
# TODO: This should happen automatically, but most tests (e.g. tests/python/pants_test/auth) fail if it's not here:
[run_python_test] +
python_test_runner.rules() +
rules
)

Expand Down
6 changes: 6 additions & 0 deletions tests/python/pants_test/rules/test_test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def test_passing_python_test(self):
============================= test session starts ==============================
platform SOME_TEXT
rootdir: SOME_TEXT
plugins: SOME_TEXT
collected 1 item
testprojects/tests/python/pants/dummies/test_pass.py . [100%]
Expand All @@ -90,6 +91,7 @@ def test_failing_python_test(self):
============================= test session starts ==============================
platform SOME_TEXT
rootdir: SOME_TEXT
plugins: SOME_TEXT
collected 1 item
testprojects/tests/python/pants/dummies/test_fail.py F [100%]
Expand Down Expand Up @@ -117,6 +119,7 @@ def test_source_dep(self):
============================= test session starts ==============================
platform SOME_TEXT
rootdir: SOME_TEXT
plugins: SOME_TEXT
collected 1 item
testprojects/tests/python/pants/dummies/test_with_source_dep.py . [100%]
Expand All @@ -135,6 +138,7 @@ def test_thirdparty_dep(self):
============================= test session starts ==============================
platform SOME_TEXT
rootdir: SOME_TEXT
plugins: SOME_TEXT
collected 1 item
testprojects/tests/python/pants/dummies/test_with_thirdparty_dep.py . [100%]
Expand All @@ -155,6 +159,7 @@ def test_mixed_python_tests(self):
============================= test session starts ==============================
platform SOME_TEXT
rootdir: SOME_TEXT
plugins: SOME_TEXT
collected 1 item
testprojects/tests/python/pants/dummies/test_fail.py F [100%]
Expand All @@ -171,6 +176,7 @@ def test_fail():
============================= test session starts ==============================
platform SOME_TEXT
rootdir: SOME_TEXT
plugins: SOME_TEXT
collected 1 item
testprojects/tests/python/pants/dummies/test_pass.py . [100%]
Expand Down

0 comments on commit 619586e

Please sign in to comment.