diff --git a/contrib/go/examples/src/go/libB/b.go b/contrib/go/examples/src/go/libB/b.go index 3caf851daf3..1cb39a3972c 100644 --- a/contrib/go/examples/src/go/libB/b.go +++ b/contrib/go/examples/src/go/libB/b.go @@ -4,16 +4,8 @@ import ( "libD" ) -func SpeakPrologue() string { - return "Hello from libB!" -} - -func SpeakEpilogue() string { - return "Bye from libB!" -} - func Speak() { - println(SpeakPrologue()) + println("Hello from libB!") libD.Speak() - println(SpeakEpilogue()) + println("Bye from libB!") } diff --git a/contrib/go/examples/src/go/libB/b_test.go b/contrib/go/examples/src/go/libB/b_test.go deleted file mode 100644 index 5552ebcc557..00000000000 --- a/contrib/go/examples/src/go/libB/b_test.go +++ /dev/null @@ -1,16 +0,0 @@ -package libB - -import ( - "testing" -) - -func TestSpeak(t *testing.T) { - got, exp := SpeakPrologue(), "Hello from libB!" - if got != exp { - t.Fatalf("got: %d, expected: %d", got, exp) - } - got2, exp2 := SpeakEpilogue(), "Bye from libB!" - if got2 != exp2 { - t.Fatalf("got: %d, expected: %d", got2, exp2) - } -} diff --git a/contrib/go/src/python/pants/contrib/go/subsystems/go_distribution.py b/contrib/go/src/python/pants/contrib/go/subsystems/go_distribution.py index aa5a26f646f..f524aa466f7 100644 --- a/contrib/go/src/python/pants/contrib/go/subsystems/go_distribution.py +++ b/contrib/go/src/python/pants/contrib/go/subsystems/go_distribution.py @@ -129,7 +129,7 @@ def execute_go_cmd(self, cmd, gopath=None, args=None, env=None, :returns: A tuple of the exit code and the go command that was run. :rtype: (int, :class:`GoDistribution.GoCommand`) """ - go_cmd = self.create_go_cmd(cmd, gopath=gopath, args=args) + go_cmd = self.GoCommand._create(self.goroot, cmd, go_env=self.go_env(gopath=gopath), args=args) if workunit_factory is None: return go_cmd.spawn(**kwargs).wait() else: diff --git a/contrib/go/src/python/pants/contrib/go/tasks/go_test.py b/contrib/go/src/python/pants/contrib/go/tasks/go_test.py index 0ca7ebf23d9..bd03af50a55 100644 --- a/contrib/go/src/python/pants/contrib/go/tasks/go_test.py +++ b/contrib/go/src/python/pants/contrib/go/tasks/go_test.py @@ -4,21 +4,15 @@ from __future__ import absolute_import, division, print_function, unicode_literals -from contextlib import contextmanager +from builtins import filter -from future.utils import text_type -from pants.base.build_environment import get_buildroot +from pants.base.exceptions import TaskError from pants.base.workunit import WorkUnitLabel -from pants.task.testrunner_task_mixin import PartitionedTestRunnerTaskMixin, TestResult -from pants.util.memo import memoized_property -from pants.util.objects import datatype -from pants.util.process_handler import SubprocessProcessHandler -from pants.util.strutil import create_path_env_var, safe_shlex_join, safe_shlex_split from pants.contrib.go.tasks.go_workspace_task import GoWorkspaceTask -class GoTest(PartitionedTestRunnerTaskMixin, GoWorkspaceTask): +class GoTest(GoWorkspaceTask): """Runs `go test` on Go packages. To run a library's tests, GoTest only requires a Go workspace to be initialized @@ -32,8 +26,6 @@ class GoTest(PartitionedTestRunnerTaskMixin, GoWorkspaceTask): @classmethod def register_options(cls, register): super(GoTest, cls).register_options(register) - # TODO: turn these into a list of individually-shlexed strings and deprecate using a single - # string! register('--build-and-test-flags', default='', fingerprint=True, help='Flags to pass in to `go test` tool.') @@ -42,72 +34,20 @@ def register_options(cls, register): def supports_passthru_args(cls): return True - def _test_target_filter(self): - return self.is_local_src - - def _validate_target(self, target): - self.ensure_workspace(target) - - class _GoTestTargetInfo(datatype([('import_path', text_type), ('gopath', text_type)])): pass - - def _generate_args_for_targets(self, targets): - """ - Generate a dict mapping target -> _GoTestTargetInfo so that the import path and gopath can be - reconstructed for spawning test commands regardless of how the targets are partitioned. - """ - return { - t: self._GoTestTargetInfo(import_path=t.import_path, gopath=self.get_gopath(t)) - for t in targets - } - - @contextmanager - def partitions(self, per_target, all_targets, test_targets): - if per_target: - def iter_partitions(): - for test_target in test_targets: - partition = (test_target,) - args = (self._generate_args_for_targets([test_target]),) - yield partition, args - else: - def iter_partitions(): - if test_targets: - partition = tuple(test_targets) - args = (self._generate_args_for_targets(test_targets),) - yield partition, args - yield iter_partitions - - def collect_files(self, *args): - pass - - @memoized_property - def _build_and_test_flags(self): - return safe_shlex_split(self.get_options().build_and_test_flags) - - def _spawn(self, workunit, go_cmd, cwd): - go_process = go_cmd.spawn(cwd=cwd, - stdout=workunit.output('stdout'), - stderr=workunit.output('stderr')) - return SubprocessProcessHandler(go_process) - - @property - def _maybe_workdir(self): - if self.run_tests_in_chroot: - return None - return get_buildroot() - - def run_tests(self, fail_fast, test_targets, args_by_target): - with self.chroot(test_targets, self._maybe_workdir) as chroot: - cmdline_args = self._build_and_test_flags + [ - args_by_target[t].import_path for t in test_targets - ] + self.get_passthru_args() - gopath = create_path_env_var( - args_by_target[t].gopath for t in test_targets - ) - go_cmd = self.go_dist.create_go_cmd('test', gopath=gopath, args=cmdline_args) - - workunit_labels = [WorkUnitLabel.TOOL, WorkUnitLabel.TEST] - with self.context.new_workunit( - name='go test', cmd=safe_shlex_join(go_cmd.cmdline), labels=workunit_labels) as workunit: - - exit_code = self.spawn_and_wait(workunit=workunit, go_cmd=go_cmd, cwd=chroot) - return TestResult.rc(exit_code) + def execute(self): + # Only executes the tests from the package specified by the target roots, so + # we don't run the tests for _all_ dependencies of said package. + targets = filter(self.is_local_src, self.context.target_roots) + for target in targets: + self.ensure_workspace(target) + self._go_test(target) + + def _go_test(self, target): + args = (self.get_options().build_and_test_flags.split() + + [target.import_path] + + self.get_passthru_args()) + result, go_cmd = self.go_dist.execute_go_cmd('test', gopath=self.get_gopath(target), args=args, + workunit_factory=self.context.new_workunit, + workunit_labels=[WorkUnitLabel.TEST]) + if result != 0: + raise TaskError('{} failed with exit code {}'.format(go_cmd, result)) diff --git a/contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_test_integration.py b/contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_test_integration.py index 7af1e5b594f..c27471dac36 100644 --- a/contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_test_integration.py +++ b/contrib/go/tests/python/pants_test/contrib/go/tasks/test_go_test_integration.py @@ -9,7 +9,6 @@ from pants.util.dirutil import safe_open from pants_test.pants_run_integration_test import PantsRunIntegrationTest -from pants_test.testutils.py2_compat import assertRegex class GoTestIntegrationTest(PantsRunIntegrationTest): @@ -18,19 +17,6 @@ def test_go_test_simple(self): 'contrib/go/examples/src/go/libA'] pants_run = self.run_pants(args) self.assert_success(pants_run) - # libA depends on libB, so both tests should be run. - assertRegex(self, pants_run.stdout_data, r'ok\s+libA') - assertRegex(self, pants_run.stdout_data, r'ok\s+libB') - - def test_no_fast(self): - args = ['test.go', - '--no-fast', - 'contrib/go/examples/src/go/libA'] - pants_run = self.run_pants(args) - self.assert_success(pants_run) - # libA depends on libB, so both tests should be run. - assertRegex(self, pants_run.stdout_data, r'ok\s+libA') - assertRegex(self, pants_run.stdout_data, r'ok\s+libB') def test_go_test_unstyle(self): with self.temporary_sourcedir() as srcdir: diff --git a/contrib/node/src/python/pants/contrib/node/tasks/node_test.py b/contrib/node/src/python/pants/contrib/node/tasks/node_test.py index 05de6e25751..a1e5b858767 100644 --- a/contrib/node/src/python/pants/contrib/node/tasks/node_test.py +++ b/contrib/node/src/python/pants/contrib/node/tasks/node_test.py @@ -39,15 +39,15 @@ def _run_node_distribution_command(self, command, workunit): This is what is ultimately used to run the Command. It must return the return code of the process. The base implementation just calls - command.run immediately. We override here to invoke TestRunnerTaskMixin.spawn_and_wait, + command.run immediately. We override here to invoke TestRunnerTaskMixin._spawn_and_wait, which ultimately invokes _spawn, which finally calls command.run. """ - return self.spawn_and_wait(command, workunit) + return self._spawn_and_wait(command, workunit) def _get_test_targets_for_spawn(self): """Overrides TestRunnerTaskMixin._get_test_targets_for_spawn. - TestRunnerTaskMixin.spawn_and_wait uses this method to know what targets are being run. + TestRunnerTaskMixin._spawn_and_wait uses this method to know what targets are being run. By default it returns all test targets - here we override it with the list self._currently_executing_test_targets, which _execute sets. """ diff --git a/src/python/pants/backend/jvm/tasks/junit_run.py b/src/python/pants/backend/jvm/tasks/junit_run.py index aeda5655e99..3a760f97d32 100644 --- a/src/python/pants/backend/jvm/tasks/junit_run.py +++ b/src/python/pants/backend/jvm/tasks/junit_run.py @@ -5,8 +5,10 @@ from __future__ import absolute_import, division, print_function, unicode_literals import fnmatch +import functools import itertools import os +import shutil import sys from abc import abstractmethod from builtins import object, range, str @@ -27,6 +29,7 @@ from pants.base.build_environment import get_buildroot from pants.base.exceptions import TargetDefinitionException, TaskError from pants.base.workunit import WorkUnitLabel +from pants.build_graph.files import Files from pants.build_graph.target import Target from pants.build_graph.target_scopes import Scopes from pants.java.distribution.distribution import DistributionLocator @@ -36,8 +39,8 @@ from pants.task.testrunner_task_mixin import PartitionedTestRunnerTaskMixin, TestResult from pants.util import desktop from pants.util.argutil import ensure_arg, remove_arg -from pants.util.contextutil import environment_as -from pants.util.dirutil import safe_delete, safe_mkdir, safe_rmtree, safe_walk +from pants.util.contextutil import environment_as, temporary_dir +from pants.util.dirutil import safe_delete, safe_mkdir, safe_mkdir_for, safe_rmtree, safe_walk from pants.util.memo import memoized_method from pants.util.meta import AbstractClass from pants.util.strutil import pluralize @@ -356,6 +359,29 @@ def _collect_test_targets(self, targets): else: return test_registry + @staticmethod + def _copy_files(dest_dir, target): + if isinstance(target, Files): + for source in target.sources_relative_to_buildroot(): + src = os.path.join(get_buildroot(), source) + dest = os.path.join(dest_dir, source) + safe_mkdir_for(dest) + shutil.copy(src, dest) + + @contextmanager + def _chroot(self, targets, workdir): + if workdir is not None: + yield workdir + else: + root_dir = os.path.join(self.workdir, '_chroots') + safe_mkdir(root_dir) + with temporary_dir(root_dir=root_dir) as chroot: + self.context.build_graph.walk_transitive_dependency_graph( + addresses=[t.address for t in targets], + work=functools.partial(self._copy_files, chroot) + ) + yield chroot + @property def _batched(self): return self._batch_size != self._BATCH_ALL @@ -420,11 +446,11 @@ def parse_error_handler(parse_error): batch_test_specs = [test.render_test_spec() for test in batch] with argfile.safe_args(batch_test_specs, self.get_options()) as batch_tests: - with self.chroot(relevant_targets, workdir) as chroot: + with self._chroot(relevant_targets, workdir) as chroot: self.context.log.debug('CWD = {}'.format(chroot)) self.context.log.debug('platform = {}'.format(platform)) with environment_as(**dict(target_env_vars)): - subprocess_result = self.spawn_and_wait( + subprocess_result = self._spawn_and_wait( executor=SubprocessExecutor(distribution), distribution=distribution, classpath=complete_classpath, diff --git a/src/python/pants/backend/python/tasks/pytest_run.py b/src/python/pants/backend/python/tasks/pytest_run.py index c8d09ca4cec..feb42223409 100644 --- a/src/python/pants/backend/python/tasks/pytest_run.py +++ b/src/python/pants/backend/python/tasks/pytest_run.py @@ -517,10 +517,10 @@ def _do_run_tests_with_args(self, pex, args): with self.context.new_workunit(name='run', cmd=' '.join(pex.cmdline(args)), labels=[WorkUnitLabel.TOOL, WorkUnitLabel.TEST]) as workunit: - rc = self.spawn_and_wait(pex, workunit=workunit, args=args, setsid=True, env=env) + rc = self._spawn_and_wait(pex, workunit=workunit, args=args, setsid=True, env=env) return PytestResult.rc(rc) except ErrorWhileTesting: - # spawn_and_wait wraps the test runner in a timeout, so it could + # _spawn_and_wait wraps the test runner in a timeout, so it could # fail with a ErrorWhileTesting. We can't just set PythonTestResult # to a failure because the resultslog doesn't have all the failures # when tests are killed with a timeout. Therefore we need to re-raise diff --git a/src/python/pants/task/testrunner_task_mixin.py b/src/python/pants/task/testrunner_task_mixin.py index e32a9a4b6b9..326c7cfe015 100644 --- a/src/python/pants/task/testrunner_task_mixin.py +++ b/src/python/pants/task/testrunner_task_mixin.py @@ -4,22 +4,16 @@ from __future__ import absolute_import, division, print_function, unicode_literals -import functools import os import re -import shutil import xml.etree.ElementTree as ET from abc import abstractmethod from builtins import filter, next, object, str -from contextlib import contextmanager -from pants.base.build_environment import get_buildroot from pants.base.exceptions import ErrorWhileTesting, TaskError from pants.build_graph.files import Files from pants.invalidation.cache_manager import VersionedTargetSet from pants.task.task import Task -from pants.util.contextutil import temporary_dir -from pants.util.dirutil import safe_mkdir, safe_mkdir_for from pants.util.memo import memoized_method, memoized_property from pants.util.process_handler import subprocess @@ -247,16 +241,16 @@ def parse_xml_file(xml_file_path): return tests_in_path def _get_test_targets_for_spawn(self): - """Invoked by spawn_and_wait to know targets being executed. Defaults to _get_test_targets(). + """Invoked by _spawn_and_wait to know targets being executed. Defaults to _get_test_targets(). - spawn_and_wait passes all its arguments through to _spawn, but it needs to know what targets - are being executed by _spawn. A caller to spawn_and_wait can override this method to return - the targets being executed by the current spawn_and_wait. By default it returns + _spawn_and_wait passes all its arguments through to _spawn, but it needs to know what targets + are being executed by _spawn. A caller to _spawn_and_wait can override this method to return + the targets being executed by the current _spawn_and_wait. By default it returns _get_test_targets(), which is all test targets. """ return self._get_test_targets() - def spawn_and_wait(self, *args, **kwargs): + def _spawn_and_wait(self, *args, **kwargs): """Spawn the actual test runner process, and wait for it to complete.""" test_targets = self._get_test_targets_for_spawn() @@ -439,29 +433,6 @@ def run_tests_in_chroot(self): """ return self.get_options().chroot - @staticmethod - def _copy_files(dest_dir, target): - if isinstance(target, Files): - for source in target.sources_relative_to_buildroot(): - src = os.path.join(get_buildroot(), source) - dest = os.path.join(dest_dir, source) - safe_mkdir_for(dest) - shutil.copy(src, dest) - - @contextmanager - def chroot(self, targets, workdir): - if workdir is not None: - yield workdir - else: - root_dir = os.path.join(self.workdir, '_chroots') - safe_mkdir(root_dir) - with temporary_dir(root_dir=root_dir) as chroot: - self.context.build_graph.walk_transitive_dependency_graph( - addresses=[t.address for t in targets], - work=functools.partial(self._copy_files, chroot) - ) - yield chroot - def _execute(self, all_targets): test_targets = self._get_test_targets() if not test_targets: diff --git a/tests/python/pants_test/task/test_testrunner_task_mixin.py b/tests/python/pants_test/task/test_testrunner_task_mixin.py index f3c7a76217c..1876ec123a0 100644 --- a/tests/python/pants_test/task/test_testrunner_task_mixin.py +++ b/tests/python/pants_test/task/test_testrunner_task_mixin.py @@ -41,7 +41,7 @@ class TestRunnerTaskMixinTask(TestRunnerTaskMixin, TaskBase): def _execute(self, all_targets): self.call_list.append(['_execute', all_targets]) - self.spawn_and_wait() + self._spawn_and_wait() def _spawn(self, *args, **kwargs): self.call_list.append(['_spawn', args, kwargs]) @@ -147,7 +147,7 @@ class TestRunnerTaskMixinTask(TestRunnerTaskMixin, TaskBase): waited_for = None def _execute(self, all_targets): - self.spawn_and_wait() + self._spawn_and_wait() def _spawn(self, *args, **kwargs): timeouts = self.get_options().timeouts @@ -236,7 +236,7 @@ class TestRunnerTaskMixinTask(TestRunnerTaskMixin, TaskBase): def _execute(self, all_targets): self.call_list.append(['_execute', all_targets]) - self.spawn_and_wait() + self._spawn_and_wait() def _spawn(self, *args, **kwargs): self.call_list.append(['_spawn', args, kwargs]) @@ -322,7 +322,7 @@ class TestRunnerTaskMixinMultipleTargetsTask(TestRunnerTaskMixin, TaskBase): wait_time = None def _execute(self, all_targets): - self.spawn_and_wait() + self._spawn_and_wait() def _spawn(self, *args, **kwargs): terminate_wait = self.get_options().timeout_terminate_wait