[Piglit] [PATCH 2/3] framework: Add test timeouts

Dylan Baker baker.dylan.c at gmail.com
Thu Mar 5 14:04:02 PST 2015


This implements a test timeout that works on windows and *nix, and can
be used with concurrent test execution (unlike the currently implemented
timeouts which only work on *nix and are not safe to use with concurrent
tests).

This is not only possible but trivially easy because python3.3 adds a
timeout parameter to subprocess.Popen.join, which means all that is
needed is to add the timeout attribute to the class, set it, and handle
the exception that is raised if the timeout is exceeded.

Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
---
 framework/test/base.py             | 114 +++++++------------------------------
 framework/test/gleantest.py        |   4 ++
 framework/test/glsl_parser_test.py |   1 +
 framework/test/piglit_test.py      |   8 ++-
 tests/quick.py                     |   1 +
 5 files changed, 34 insertions(+), 94 deletions(-)

diff --git a/framework/test/base.py b/framework/test/base.py
index 23fd522..f06fb99 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -29,9 +29,6 @@ import subprocess
 import time
 import sys
 import traceback
-from datetime import datetime
-import threading
-import signal
 import itertools
 import abc
 import copy
@@ -46,56 +43,6 @@ __all__ = [
 ]
 
 
-class ProcessTimeout(threading.Thread):
-    """ Timeout class for test processes
-
-    This class is for terminating tests that run for longer than a certain
-    amount of time. Create an instance by passing it a timeout in seconds
-    and the Popen object that is running your test. Then call the start
-    method (inherited from Thread) to start the timer. The Popen object is
-    killed if the timeout is reached and it has not completed. Wait for the
-    outcome by calling the join() method from the parent.
-
-    """
-
-    def __init__(self, timeout, proc):
-        threading.Thread.__init__(self)
-        self.proc = proc
-        self.timeout = timeout
-        self.status = 0
-
-    def run(self):
-        start_time = datetime.now()
-        delta = 0
-
-        # poll() returns the returncode attribute, which is either the return
-        # code of the child process (which could be zero), or None if the
-        # process has not yet terminated.
-
-        while delta < self.timeout and self.proc.poll() is None:
-            time.sleep(1)
-            delta = (datetime.now() - start_time).total_seconds()
-
-        # if the test is not finished after timeout, first try to terminate it
-        # and if that fails, send SIGKILL to all processes in the test's
-        # process group
-
-        if self.proc.poll() is None:
-            self.status = 1
-            self.proc.terminate()
-            time.sleep(5)
-
-        if self.proc.poll() is None:
-            self.status = 2
-            if hasattr(os, 'killpg'):
-                os.killpg(self.proc.pid, signal.SIGKILL)
-            self.proc.wait()
-
-    def join(self):
-        threading.Thread.join(self)
-        return self.status
-
-
 class Test(object, metaclass=abc.ABCMeta):
     """ Abstract base class for Test classes
 
@@ -117,8 +64,8 @@ class Test(object, metaclass=abc.ABCMeta):
     """
     OPTS = Options()
     __slots__ = ['run_concurrent', 'env', 'result', 'cwd', '_command',
-                 '_test_hook_execute_run', '__proc_timeout']
-    timeout = 0
+                 '_test_hook_execute_run']
+    timeout = None
 
     def __init__(self, command, run_concurrent=False):
         assert isinstance(command, list), command
@@ -128,7 +75,6 @@ class Test(object, metaclass=abc.ABCMeta):
         self.env = {}
         self.result = TestResult({'result': 'fail'})
         self.cwd = None
-        self.__proc_timeout = None
 
         # This is a hook for doing some testing on execute right before
         # self.run is called.
@@ -213,11 +159,7 @@ class Test(object, metaclass=abc.ABCMeta):
         self.interpret_result()
 
         if self.result['returncode'] is not None and self.result['returncode'] < 0:
-            # check if the process was terminated by the timeout
-            if self.timeout > 0 and self.__proc_timeout.join() > 0:
-                self.result['result'] = 'timeout'
-            else:
-                self.result['result'] = 'crash'
+            self.result['result'] = 'crash'
         elif self.result['returncode'] != 0 and self.result['result'] == 'pass':
             self.result['result'] = 'warn'
 
@@ -242,10 +184,6 @@ class Test(object, metaclass=abc.ABCMeta):
         """
         return False
 
-    def __set_process_group(self):
-        if hasattr(os, 'setpgrp'):
-            os.setpgrp()
-
     def _run_command(self):
         """ Run the test command and get the result
 
@@ -253,10 +191,9 @@ class Test(object, metaclass=abc.ABCMeta):
         executable isn't found it sets the result to skip.
 
         """
-        # if there is an error in run command this will be set to True, if this
-        # is True then the run test method will return early. If this is set to
-        # True then self.result should be properly filled out
-        error = False
+        # If there is no error at the end of the try block this will be set to
+        # false.
+        error = True
 
         # Setup the environment for the test. Environment variables are taken
         # from the following sources, listed in order of increasing precedence:
@@ -277,44 +214,37 @@ class Test(object, metaclass=abc.ABCMeta):
                                           iter(self.env.items())):
             fullenv[key] = str(value)
 
-        # preexec_fn is not supported on Windows platforms
-        if sys.platform == 'win32':
-            preexec_fn = None
-        else:
-            preexec_fn = self.__set_process_group
-
         try:
             proc = subprocess.Popen(self.command,
                                     stdout=subprocess.PIPE,
                                     stderr=subprocess.PIPE,
                                     cwd=self.cwd,
                                     env=fullenv,
-                                    universal_newlines=True,
-                                    preexec_fn=preexec_fn)
-            # create a ProcessTimeout object to watch out for test hang if the
-            # process is still going after the timeout, then it will be killed
-            # forcing the communicate function (which is a blocking call) to
-            # return
-            if self.timeout > 0:
-                self.__proc_timeout = ProcessTimeout(self.timeout, proc)
-                self.__proc_timeout.start()
-
-            out, err = proc.communicate()
+                                    universal_newlines=True)
+
+            # For some reason pylint doesn't konw about the new python3 keyword
+            # pylint: disable=unexpected-keyword-arg
+            out, err = proc.communicate(timeout=self.timeout)
             returncode = proc.returncode
+
+            # If we get here there was no exception, thus no error.
+            error = False
         except OSError as e:
-            # Different sets of tests get built under
-            # different build configurations.  If
-            # a developer chooses to not build a test,
-            # Piglit should not report that test as having
-            # failed.
+            # Different sets of tests get built under different build
+            # configurations.  If a developer chooses to not build a test,
+            # Piglit should not report that test as having failed.
             if e.errno == errno.ENOENT:
                 self.result['result'] = 'skip'
                 out = "Test executable not found.\n"
                 err = ""
                 returncode = None
-                error = True
             else:
                 raise e
+        except subprocess.TimeoutExpired:
+            self.result['result'] = 'timeout'
+            out = 'Test Time exceeded timout of {}\n'.format(self.timeout)
+            err = ''
+            returncode = None
 
         # Popen returns bytes, but the rest of the code assumes unicode
         self.result['out'] = str(out)
diff --git a/framework/test/gleantest.py b/framework/test/gleantest.py
index 19776ba..e846636 100644
--- a/framework/test/gleantest.py
+++ b/framework/test/gleantest.py
@@ -43,6 +43,10 @@ class GleanTest(Test):
     GLOBAL_PARAMS = []
     _EXECUTABLE = os.path.join(TEST_BIN_DIR, "glean")
 
+    # Assume that no glean test will take longer than 2 minutes. quick.py
+    # overrides this to 60 since it also sets the --quick option
+    timeout = 120
+
     def __init__(self, name, **kwargs):
         super(GleanTest, self).__init__(
             [self._EXECUTABLE, "-o", "-v", "-v", "-v", "-t", "+" + name],
diff --git a/framework/test/glsl_parser_test.py b/framework/test/glsl_parser_test.py
index 3248003..109e59c 100644
--- a/framework/test/glsl_parser_test.py
+++ b/framework/test/glsl_parser_test.py
@@ -62,6 +62,7 @@ class GLSLParserTest(PiglitBaseTest):
     """
     _CONFIG_KEYS = frozenset(['expect_result', 'glsl_version',
                               'require_extensions', 'check_link'])
+    timeout = 10
 
     def __init__(self, filepath):
         os.stat(filepath)
diff --git a/framework/test/piglit_test.py b/framework/test/piglit_test.py
index 95f791d..2fcdafe 100644
--- a/framework/test/piglit_test.py
+++ b/framework/test/piglit_test.py
@@ -53,12 +53,16 @@ CL_CONCURRENT = (not sys.platform.startswith('linux') or
 
 
 class PiglitBaseTest(Test):
-    """
-    PiglitTest: Run a "native" piglit test executable
+    """PiglitTest: Run a "native" piglit test executable
 
     Expect one line prefixed PIGLIT: in the output, which contains a result
     dictionary. The plain output is appended to this dictionary
+
     """
+    # no piglit test should go over 30 seconds (if it does that can always be
+    # overwritten in all.py
+    timeout = 30
+
     def __init__(self, command, run_concurrent=True, **kwargs):
         super(PiglitBaseTest, self).__init__(command, run_concurrent, **kwargs)
 
diff --git a/tests/quick.py b/tests/quick.py
index 5393a2b..e859294 100644
--- a/tests/quick.py
+++ b/tests/quick.py
@@ -10,6 +10,7 @@ __all__ = ['profile']
 # pylint: disable=bad-continuation
 
 GleanTest.GLOBAL_PARAMS += ["--quick"]
+GleanTest.timeout = 60
 
 # Set the --quick flag on a few image_load_store_tests
 with profile.group_manager(
-- 
2.3.1



More information about the Piglit mailing list