Revert "make GoTest subclass PartitionedTestRunnerTaskMixin to test t…

…ransitively" (pantsbuild#7212)

See pantsbuild#7188 and pantsbuild#7145.

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!")
 }

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:

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))

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:

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.
     """

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,

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

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: