Skip to content

Commit

Permalink
Support running python tests in the pex chroot.
Browse files Browse the repository at this point in the history
With the new `--chroot` option, tests are run with `CWD` set to the
`py.test` pex chroot making access of un-declared file dependencies
~impossible and ensuring all files involved in `py.test` execution are
part of the test cache key.

Fixes pantsbuild#5019

NB: Not ready for review.
  • Loading branch information
jsirois committed Nov 2, 2017
1 parent eda7664 commit 248b01a
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 12 deletions.
1 change: 1 addition & 0 deletions pants.ini
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ keystore_config_location: %(pants_configdir)s/android/keystore/default_config.in


[test.pytest]
chroot: true
timeouts: true
timeout_default: 60

Expand Down
5 changes: 3 additions & 2 deletions src/python/pants/backend/python/tasks2/gather_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
from pex.pex import PEX
from pex.pex_builder import PEXBuilder

from pants.backend.python.tasks2.pex_build_util import dump_sources, has_python_sources
from pants.backend.python.tasks2.pex_build_util import (dump_sources, has_python_sources,
has_resources)
from pants.invalidation.cache_manager import VersionedTargetSet
from pants.task.task import Task
from pants.util.dirutil import safe_concurrent_creation
Expand Down Expand Up @@ -40,7 +41,7 @@ def prepare(cls, options, round_manager):
round_manager.require_data('python') # For codegen.

def execute(self):
targets = self.context.targets(predicate=has_python_sources)
targets = self.context.targets(predicate=lambda t: has_python_sources(t) or has_resources(t))
interpreter = self.context.products.get_data(PythonInterpreter)

with self.invalidated(targets) as invalidation_check:
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/backend/python/tasks2/pex_build_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,19 @@
from pants.backend.python.targets.python_tests import PythonTests
from pants.base.build_environment import get_buildroot
from pants.base.exceptions import TaskError
from pants.build_graph.resources import Resources
from pants.build_graph.files import Files
from pants.python.python_repos import PythonRepos


def has_python_sources(tgt):
# We'd like to take all PythonTarget subclasses, but currently PythonThriftLibrary and
# PythonAntlrLibrary extend PythonTarget, and until we fix that (which we can't do until
# we remove the old python pipeline entirely) we want to ignore those target types here.
return isinstance(tgt, (PythonLibrary, PythonTests, PythonBinary, Resources))
return isinstance(tgt, (PythonLibrary, PythonTests, PythonBinary))


def has_resources(tgt):
return isinstance(tgt, Resources)
return isinstance(tgt, Files)


def has_python_requirements(tgt):
Expand All @@ -44,7 +44,7 @@ def dump_sources(builder, tgt, log):
for relpath in tgt.sources_relative_to_source_root():
try:
src = os.path.join(buildroot, tgt.target_base, relpath)
if isinstance(tgt, Resources):
if has_resources(tgt):
builder.add_resource(src, relpath)
else:
builder.add_source(src, relpath)
Expand Down
26 changes: 22 additions & 4 deletions src/python/pants/backend/python/tasks2/pytest_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from pants.base.fingerprint_strategy import DefaultFingerprintStrategy
from pants.base.hash_utils import Sharder
from pants.base.workunit import WorkUnitLabel
from pants.build_graph.files import Files
from pants.build_graph.target import Target
from pants.invalidation.cache_manager import VersionedTargetSet
from pants.task.task import Task
Expand Down Expand Up @@ -142,6 +143,11 @@ def register_options(cls, register):
'will run in its own pytest invocation, which will be slower, but isolates '
'tests from process-wide state created by tests in other targets.')

register('--chroot', advanced=True, fingerprint=True, type=bool, default=False,
help='Run tests in a chroot. Any loose files tests depend on via `{}` dependencies '
'will be copied to the chroot.'
.format(Files.alias()))

# NB: We always produce junit xml privately, and if this option is specified, we then copy
# it to the user-specified directory, post any interaction with the cache to retrieve the
# privately generated and cached xml files. As such, this option is not part of the
Expand Down Expand Up @@ -718,8 +724,11 @@ def _run_pytest(self, workdirs, targets):
return PytestResult.rc(0)

buildroot = get_buildroot()
source_chroot = os.path.relpath(
self.context.products.get_data(GatherSources.PYTHON_SOURCES).path(), buildroot)

# TODO(John Sirois): This is a hack around understanding py.test's view of the fs, correct
# our conftest.py source mapping scheme instead.
source_chroot = self.context.products.get_data(GatherSources.PYTHON_SOURCES).path()

sources_map = {} # Path from chroot -> Path from buildroot.
for t in targets:
for p in t.sources_relative_to_source_root():
Expand Down Expand Up @@ -787,8 +796,17 @@ def _pex_run(self, pex, workunit_name, args, env):
process = self._spawn(pex, workunit, args, setsid=False, env=env)
return process.wait()

@property
def _run_in_chroot(self):
return self.get_options().chroot

def _spawn(self, pex, workunit, args, setsid=False, env=None):
env = env or {}
process = pex.run(args, blocking=False, setsid=setsid, env=env,
stdout=workunit.output('stdout'), stderr=workunit.output('stderr'))
process = pex.run(args,
with_chroot=self._run_in_chroot,
blocking=False,
setsid=setsid,
env=env,
stdout=workunit.output('stdout'),
stderr=workunit.output('stderr'))
return SubprocessProcessHandler(process)
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from pants.backend.python.tasks2.resolve_requirements import ResolveRequirements
from pants.backend.python.tasks2.resolve_requirements_task_base import ResolveRequirementsTaskBase
from pants.build_graph.address import Address
from pants.build_graph.resources import Resources
from pants.build_graph.files import Files
from pants.invalidation.cache_manager import VersionedTargetSet
from pants.util.dirutil import safe_concurrent_creation

Expand Down Expand Up @@ -101,7 +101,7 @@ def extra_requirements(self):
def create_pex(self, pex_info=None):
"""Returns a wrapped pex that "merges" the other pexes via PEX_PATH."""
relevant_targets = self.context.targets(
lambda tgt: isinstance(tgt, (PythonRequirementLibrary, PythonTarget, Resources)))
lambda tgt: isinstance(tgt, (PythonRequirementLibrary, PythonTarget, Files)))
with self.invalidated(relevant_targets) as invalidation_check:

# If there are no relevant targets, we still go through the motions of resolving
Expand Down

0 comments on commit 248b01a

Please sign in to comment.