Add timeouts to tests.

- A per-target test timeout can be configured in the `BUILD` file with `timeout=SECONDS`
- Because pants may run many targets tests as once, the timeouts for all the tests are summed for the entire run of tests timeout
- If one of the targets has no timeout, then all the tests run with no timeout
- A default timeout can be configured with the `--timeout-default` option
- Timeouts can be [disabled]/enabled with `--[no-]timeouts`
- Sets the default timeout to 1 minute for java & python tests in `pants.ini`

Testing Done:
./pants test tests/python/pants_test:all
./pants test tests/python/pants_test/backend/jvm/tasks:junit_run_integration

- Added tests:
   * tests/python/pants_test/util:timeout
   * tests/python/pants_test/backend/python/tasks:pytest_run
   * tests/python/pants_test/backend/jvm/tasks:junit_run
   * tests/python/pants_test/backend/core/tasks:test_task_mixin

Pull Request: pantsbuild#2243

Reviewed at https://rbcommons.com/s/twitter/r/2919/

pants.ini

@@ -231,3 +231,11 @@ resolver_cache_dir: %(pants_bootstrapdir)s/python_cache/requirements
 # Default to debug keystore installed with SDK.
 # You can change this to point to a config.ini holding the definition of your keys.
 keystore_config_location: %(pants_configdir)s/android/keystore/default_config.ini
+
+[test.pytest]
+timeouts: true
+default_timeout: 60
+
+[test.junit]
+timeouts: true
+default_timeout: 60

src/python/pants/backend/core/tasks/test_task_mixin.py

@@ -7,6 +7,9 @@
 
 from abc import abstractmethod
 
+from pants.base.exceptions import TestFailedTaskError
+from pants.util.timeout import Timeout, TimeoutReached
+
 
 class TestTaskMixin(object):
   """A mixin to combine with test runner tasks.
@@ -19,6 +22,16 @@ class TestTaskMixin(object):
   def register_options(cls, register):
     super(TestTaskMixin, cls).register_options(register)
     register('--skip', action='store_true', help='Skip running tests.')
+    register('--timeouts', action='store_true', default=True,
+             help='Enable test target timeouts. If timeouts are enabled then tests with a timeout= parameter '
+             'set on their target will time out after the given number of seconds if not completed. '
+             'If no timeout is set, then either the default timeout is used or no timeout is configured. '
+             'In the current implementation, all the timeouts for the test targets to be run are summed and '
+             'all tests are run with the total timeout covering the entire run of tests. If a single target '
+             'in a test run has no timeout and there is no default, the entire run will have no timeout. This '
+             'should change in the future to provide more granularity.')
+    register('--timeout-default', action='store', default=0, type=int,
+             help='The default timeout (in seconds) for a test if timeout is not set on the target.')
 
   def execute(self):
     """Run the task."""
@@ -28,7 +41,49 @@ def execute(self):
       all_targets = self._get_targets()
       for target in test_targets:
         self._validate_target(target)
-      self._execute(all_targets)
+
+      timeout = self._timeout_for_targets(test_targets)
+      try:
+        with Timeout(timeout):
+          self._execute(all_targets)
+      except TimeoutReached:
+        raise TestFailedTaskError(failed_targets=test_targets)
+
+  def _timeout_for_target(self, target):
+    return getattr(target, 'timeout', None)
+
+  def _timeout_for_targets(self, targets):
+    """Calculate the total timeout based on the timeout configuration for all the targets.
+
+    Because the timeout wraps all the test targets rather than individual tests, we have to somehow
+    aggregate all the target specific timeouts into one value that will cover all the tests. If some targets
+    have no timeout configured (or set to 0), their timeout will be set to the default timeout.
+    If there is no default timeout, or if it is set to zero, there will be no timeout, if any of the test targets
+    have a timeout set to 0 or no timeout configured.
+
+    :param targets: list of test targets
+    :return: timeout to cover all the targets, in seconds
+    """
+
+    if not self.get_options().timeouts:
+      return None
+
+    timeout_default = self.get_options().timeout_default
+
+    # Gather up all the timeouts.
+    timeouts = [self._timeout_for_target(target) for target in targets]
+
+    # If any target's timeout is None or 0, then set it to the default timeout
+    timeouts_w_default = [timeout or timeout_default for timeout in timeouts]
+
+    # Even after we've done that, there may be a 0 or None in the timeout list if the
+    # default timeout is set to 0 or None. So if that's the case, then the timeout is
+    # disabled
+    if 0 in timeouts_w_default or None in timeouts_w_default:
+      return None
+    else:
+      # Sum the timeouts for all the targets, using the default timeout where one is not set
+      return sum(timeouts_w_default)
 
   def _get_targets(self):
     """This is separated out so it can be overridden for testing purposes.

src/python/pants/backend/jvm/targets/java_tests.py

@@ -14,7 +14,7 @@
 class JavaTests(JvmTarget):
   """Tests JVM sources with JUnit."""
 
-  def __init__(self, cwd=None, test_platform=None, payload=None, **kwargs):
+  def __init__(self, cwd=None, test_platform=None, payload=None, timeout=None, **kwargs):
     """
     :param str cwd: working directory (relative to the build root) for the tests under this
       target. If unspecified (None), the working directory will be controlled by junit_run's --cwd.
@@ -27,6 +27,7 @@ def __init__(self, cwd=None, test_platform=None, payload=None, **kwargs):
     payload.add_fields({
       'test_platform': PrimitiveField(test_platform)
     })
+    self._timeout = timeout
     super(JavaTests, self).__init__(payload=payload, **kwargs)
 
     # TODO(John Sirois): These could be scala, clojure, etc.  'jvm' and 'tests' are the only truly
@@ -38,3 +39,7 @@ def test_platform(self):
     if self.payload.test_platform:
       return JvmPlatform.global_instance().get_platform_by_name(self.payload.test_platform)
     return self.platform
+
+  @property
+  def timeout(self):
+    return self._timeout

src/python/pants/backend/python/targets/python_tests.py

@@ -13,15 +13,20 @@
 class PythonTests(PythonTarget):
   """Tests a Python library."""
 
-  def __init__(self, coverage=None, **kwargs):
+  def __init__(self, coverage=None, timeout=None, **kwargs):
     """
     :param coverage: the module(s) whose coverage should be generated, e.g.
       'twitter.common.log' or ['twitter.common.log', 'twitter.common.http']
     """
     self._coverage = maybe_list(coverage) if coverage is not None else []
+    self._timeout = timeout
     super(PythonTests, self).__init__(**kwargs)
     self.add_labels('python', 'tests')
 
   @property
   def coverage(self):
     return self._coverage
+
+  @property
+  def timeout(self):
+    return self._timeout

src/python/pants/backend/python/tasks/BUILD

@@ -44,6 +44,7 @@ python_library(
     'src/python/pants/util:meta',
     'src/python/pants/util:strutil',
     'src/python/pants/util:xml_parser',
+    'src/python/pants/util:timeout',
   ]
 )
 

src/python/pants/util/BUILD

@@ -67,3 +67,8 @@ python_library(
   name = 'xml_parser',
   sources = ['xml_parser.py'],
 )
+
+python_library(
+  name = 'timeout',
+  sources = ['timeout.py'],
+)

src/python/pants/util/timeout.py

@@ -0,0 +1,61 @@
+# coding=utf-8
+# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
+                        unicode_literals, with_statement)
+
+import sys
+import thread
+import threading
+
+
+class TimeoutReached(Exception):
+  def __init__(self, seconds):
+    super(TimeoutReached, self).__init__("Timeout of {} seconds reached".format(seconds))
+
+
+class Timeout(object):
+  """Timeout generator. If seconds is None or 0, then the there is no timeout.
+
+    try:
+      with Timeout(seconds):
+        <do stuff>
+    except TimeoutReached:
+      <handle timeout>
+  """
+
+  def __init__(self, seconds, threading_timer=threading.Timer):
+
+    # self._triggered is not protected by a mutex because boolean set/get is atomic in all the Python
+    # implementations we care about.
+    self._triggered = False
+    self._seconds = seconds
+
+    if self._seconds:
+      self._timer = threading_timer(self._seconds, self._handle_timeout)
+    else:
+      self._timer = None
+
+  def _handle_timeout(self):
+    sys.stderr.flush()  # Python 3 stderr is likely buffered.
+    self._triggered = True
+    thread.interrupt_main()  # Raises KeyboardInterrupt.
+
+  def __enter__(self):
+    if self._timer is not None:
+      self._timer.start()
+
+  def __exit__(self, type_, value, traceback):
+    """If triggered, raise TimeoutReached.
+
+    Rather than converting a KeyboardInterrupt to TimeoutReached here, we just check self._triggered,
+    which helps us in the case where the thread we are trying to timeout isn't the main thread. Of
+    course in that case the executing doesn't get interrupted at the appropriate time, but at least
+    the exception still gets raised.
+    """
+    if self._triggered:
+      raise TimeoutReached(self._seconds)
+
+    elif self._timer is not None:
+      self._timer.cancel()

tests/python/pants_test/backend/core/tasks/BUILD

@@ -145,5 +145,6 @@ python_tests(
   dependencies=[
     'src/python/pants/backend/core/tasks:test_task_mixin',
     'tests/python/pants_test/tasks:task_test_base',
+    '3rdparty/python:mock',
   ]
 )

tests/python/pants_test/backend/core/tasks/test_test_task_mixin.py

@@ -5,11 +5,24 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
+from mock import patch
+
 from pants.backend.core.tasks.task import TaskBase
 from pants.backend.core.tasks.test_task_mixin import TestTaskMixin
+from pants.base.exceptions import TestFailedTaskError
+from pants.util.timeout import TimeoutReached
 from pants_test.tasks.task_test_base import TaskTestBase
 
 
+class DummyTestTarget(object):
+  def __init__(self, name, timeout=None):
+    self.name = name
+    self.timeout = timeout
+
+targetA = DummyTestTarget('TargetA')
+targetB = DummyTestTarget('TargetB', timeout=1)
+
+
 class TestTaskMixinTest(TaskTestBase):
   @classmethod
   def task_type(cls):
@@ -20,12 +33,12 @@ def _execute(self, all_targets):
         self.call_list.append(['_execute', all_targets])
 
       def _get_targets(self):
-        return ['TargetA', 'TargetB']
+        return [targetA, targetB]
 
       def _test_target_filter(self):
         def target_filter(target):
           self.call_list.append(['target_filter', target])
-          if target == 'TargetA':
+          if target.name == 'TargetA':
             return False
           else:
             return True
@@ -38,21 +51,89 @@ def _validate_target(self, target):
     return TestTaskMixinTask
 
   def test_execute_normal(self):
-    self.task = self.create_task(self.context())
+    task = self.create_task(self.context())
 
-    self.task.execute()
+    task.execute()
 
     # Confirm that everything ran as expected
-    self.assertIn(['target_filter', 'TargetA'], self.task.call_list)
-    self.assertIn(['target_filter', 'TargetB'], self.task.call_list)
-    self.assertIn(['_validate_target', 'TargetB'], self.task.call_list)
-    self.assertIn(['_execute', ['TargetA', 'TargetB']], self.task.call_list)
+    self.assertIn(['target_filter', targetA], task.call_list)
+    self.assertIn(['target_filter', targetB], task.call_list)
+    self.assertIn(['_validate_target', targetB], task.call_list)
+    self.assertIn(['_execute', [targetA, targetB]], task.call_list)
 
   def test_execute_skip(self):
     # Set the skip option
     self.set_options(skip=True)
-    self.task = self.create_task(self.context())
-    self.task.execute()
+    task = self.create_task(self.context())
+    task.execute()
 
     # Ensure nothing got called
-    self.assertListEqual(self.task.call_list, [])
+    self.assertListEqual(task.call_list, [])
+
+  def test_get_timeouts_no_default(self):
+    """If there is no default and one of the targets has no timeout, then there is no timeout for the entire run."""
+
+    self.set_options(timeouts=True, timeout_default=None)
+    task = self.create_task(self.context())
+
+    self.assertIsNone(task._timeout_for_targets([targetA, targetB]))
+
+  def test_get_timeouts_disabled(self):
+    """If timeouts are disabled, there is no timeout for the entire run."""
+
+    self.set_options(timeouts=False, timeout_default=2)
+    task = self.create_task(self.context())
+
+    self.assertIsNone(task._timeout_for_targets([targetA, targetB]))
+
+  def test_get_timeouts_w_default(self):
+    """If there is a default timeout, use that for targets which have no timeout set."""
+
+    self.set_options(timeouts=True, timeout_default=2)
+    task = self.create_task(self.context())
+
+    self.assertEquals(task._timeout_for_targets([targetA, targetB]), 3)
+
+
+class TestTaskMixinTimeoutTest(TaskTestBase):
+  @classmethod
+  def task_type(cls):
+    class TestTaskMixinTask(TestTaskMixin, TaskBase):
+      call_list = []
+
+      def _execute(self, all_targets):
+        self.call_list.append(['_execute', all_targets])
+
+      def _get_targets(self):
+        return [targetB]
+
+      def _test_target_filter(self):
+        def target_filter(target):
+          return True
+
+        return target_filter
+
+      def _validate_target(self, target):
+        self.call_list.append(['_validate_target', target])
+
+    return TestTaskMixinTask
+
+  def test_timeout(self):
+    self.set_options(timeouts=True)
+    task = self.create_task(self.context())
+
+    with patch('pants.backend.core.tasks.test_task_mixin.Timeout') as mock_timeout:
+      mock_timeout().__exit__.side_effect = TimeoutReached(1)
+
+      with self.assertRaises(TestFailedTaskError):
+        task.execute()
+
+      mock_timeout.assert_called_with(1)
+
+  def test_timeout_disabled(self):
+    self.set_options(timeouts=False)
+    task = self.create_task(self.context())
+
+    with patch('pants.backend.core.tasks.test_task_mixin.Timeout') as mock_timeout:
+      task.execute()
+      mock_timeout.assert_called_with(None)

tests/python/pants_test/backend/jvm/tasks/BUILD

@@ -244,6 +244,7 @@ python_tests(
   name = 'junit_run',
   sources = ['test_junit_run.py'],
   dependencies = [
+    '3rdparty/python:mock',
     'src/python/pants/backend/core/targets:common',
     'src/python/pants/backend/jvm/targets:java',
     'src/python/pants/backend/jvm/tasks:junit_run',