[Piglit] [PATCH 2/2] framework/test/base.py: try to use subprocess32
Thomas Wood
thomas.wood at intel.com
Wed Oct 14 08:27:56 PDT 2015
On 9 October 2015 at 20:53, <baker.dylan.c at gmail.com> wrote:
> From: Dylan Baker <baker.dylan.c at gmail.com>
>
> This adds subprocess32 as an optional dependency to get timeouts. This
> module is a backport of the feature from python 3.2 that adds the
> timeout argument to Popen.communicate (among others).
>
> There is a caveat to this feature. Unlike the python 3.x version, this
> doesn't work on windows, although that isn't a change from the current
> state of affairs, as the current timeouts don't work on windows
>
> What this does do though, is get us timeouts that work with concurrency
> on posix systems (like Linux and OSX).
>
> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
> ---
> README | 5 +-
> framework/test/base.py | 123 +++++++++++++-----------------------------
> framework/tests/base_tests.py | 2 +
> tox.ini | 1 +
> 4 files changed, 43 insertions(+), 88 deletions(-)
>
> diff --git a/README b/README
> index be8f4df..5813af5 100644
> --- a/README
> +++ b/README
> @@ -47,9 +47,12 @@ Optionally, you can install the following:
> - lxml. An accelerated python xml library using libxml2 (http://lxml.de/)
> - simplejson. A fast C based implementation of the python json library.
> (https://simplejson.readthedocs.org/en/latest/)
> - - backports.lzma. A packport of python3's lzma module to python2,
> + - backports.lzma. A backport of python3's lzma module to python2,
> this enables fast native xz (de)compression in piglit for results files
> (https://github.com/peterjc/backports.lzma)
> + - subprocess32. A backport of python 3.2's subprocess module to python 2.
> + This provides the timeout mechanism.
> + (https://pypi.python.org/pypi/subprocess32/)
>
> Now configure the build system:
>
> diff --git a/framework/test/base.py b/framework/test/base.py
> index bf00396..0f01ccc 100644
> --- a/framework/test/base.py
> +++ b/framework/test/base.py
> @@ -29,13 +29,33 @@ import subprocess
> import time
> import sys
> import traceback
> -from datetime import datetime
> -import threading
> -import signal
> import itertools
> import abc
> import copy
>
> +try:
> + from subprocess32 import Popen, TimeoutExpired
> +except ImportError:
> + from subprocess import Popen as _Popen
> +
> + class Popen(_Popen):
> + """Wrapper to provide timout on communicate.
> +
> + This allows us to use the same code without a plethora of if statements
> + to special case windows an non subprocess32 cases.
> +
> + """
> + def communicate(self, *args, **kwargs):
> + try:
> + del kwargs['timeout']
> + except KeyError:
> + pass
> +
> + super(Popen, self).communicate(*args, **kwargs)
> +
> + class TimeoutExpired(Exception):
> + pass
> +
> from framework import exceptions
> from framework.core import Options
> from framework.results import TestResult
> @@ -62,56 +82,6 @@ class TestRunError(exceptions.PiglitException):
> self.status = status
>
>
> -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
> -
> -
> def _is_crash_returncode(returncode):
> """Determine whether the given process return code correspond to a
> crash.
> @@ -147,8 +117,7 @@ class Test(object):
> """
> OPTS = Options()
> __metaclass__ = abc.ABCMeta
> - __slots__ = ['run_concurrent', 'env', 'result', 'cwd', '_command',
> - '__proc_timeout']
> + __slots__ = ['run_concurrent', 'env', 'result', 'cwd', '_command']
> timeout = 0
>
> def __init__(self, command, run_concurrent=False):
> @@ -159,7 +128,6 @@ class Test(object):
> self.env = {}
> self.result = TestResult()
> self.cwd = None
> - self.__proc_timeout = None
>
> def execute(self, path, log, dmesg):
> """ Run a test
> @@ -205,11 +173,7 @@ class Test(object):
> """Convert the raw output of the test into a form piglit understands.
> """
> if _is_crash_returncode(self.result.returncode):
> - # 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'
>
> @@ -255,10 +219,6 @@ class Test(object):
> """
> pass
>
> - def __set_process_group(self):
> - if hasattr(os, 'setpgrp'):
> - os.setpgrp()
> -
> def _run_command(self):
> """ Run the test command and get the result
>
> @@ -285,29 +245,14 @@ class Test(object):
> self.env.iteritems()):
> 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()
> + proc = Popen(self.command,
> + stdout=subprocess.PIPE,
> + stderr=subprocess.PIPE,
> + cwd=self.cwd,
> + env=fullenv,
> + universal_newlines=True)
> + out, err = proc.communicate(timeout=self.timeout)
> returncode = proc.returncode
> except OSError as e:
> # Different sets of tests get built under different build
> @@ -317,6 +262,10 @@ class Test(object):
> raise TestRunError("Test executable not found.\n", 'skip')
> else:
> raise e
> + except TimeoutExpired:
> + raise TestRunError(
> + 'Test time out exceeded timeout of {}\n'.format(self.timeout),
> + 'timeout')
The process is not killed when the timeout occurs, so the previous
code to implement that needs to be added here. This involves sending
SIGTERM to the child process and then sending SIGKILL to the process
group if SIGTERM failed to remove the process. The child processes
were placed in their own process group by the preexec_fn parameter
being set to __set_process_group.
>
> # The setter handles the bytes/unicode conversion
> self.result.out = out
> diff --git a/framework/tests/base_tests.py b/framework/tests/base_tests.py
> index caef9e3..95d35bd 100644
> --- a/framework/tests/base_tests.py
> +++ b/framework/tests/base_tests.py
> @@ -63,6 +63,7 @@ def test_run_return_early():
> @attr('slow')
> def test_timeout():
> """test.base.Test.run(): kills tests that exceed timeout when set"""
> + utils.module_check('subprocess32')
> utils.binary_check('sleep', 1)
>
> class _Test(Test):
> @@ -79,6 +80,7 @@ def test_timeout():
> def test_timeout_pass():
> """test.base.Test.run(): Result is returned when timeout is set but not exceeded
> """
> + utils.module_check('subprocess32')
> utils.binary_check('true')
>
> def helper():
> diff --git a/tox.ini b/tox.ini
> index ca97a51..366af9b 100644
> --- a/tox.ini
> +++ b/tox.ini
> @@ -27,4 +27,5 @@ deps =
> simplejson
> lxml
> backports.lzma
> + subprocess32
> commands = nosetests framework/tests --attr="!privliged" --with-cover --cover-package=framework --cover-tests
> --
> 2.6.1
>
More information about the Piglit
mailing list