[Piglit] [PATCH 2/2] framework/test/base.py: try to use subprocess32
Dylan Baker
baker.dylan.c at gmail.com
Wed Oct 14 14:35:55 PDT 2015
On Wed, Oct 14, 2015 at 04:27:56PM +0100, Thomas Wood wrote:
> 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.
>
If you have subprocess32 it will be killed when the timeout expires.
This is a backport of the python3 behavior, from the python3 docs:
"""
The timeout argument is passed to Popen.communicate(). If the timeout
expires, the child process will be killed and waited for. The
TimeoutExpired exception will be re-raised after the child process has
terminated.
"""
Am I missing something here?
>
> >
> > # 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
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20151014/2f5c3dd6/attachment.sig>
More information about the Piglit
mailing list