[Piglit] [PATCH v2 6/6] framework/test/base.py: use subprocess32 for timeouts.
baker.dylan.c at gmail.com
baker.dylan.c at gmail.com
Fri Oct 23 18:17:24 PDT 2015
From: Dylan Baker <baker.dylan.c at gmail.com>
Subprocess32 provides a backport of (ironically) python 3.3's subprocess
module, which has a timeout parameter. When the timeout runs out then an
exception is raised, and when that exception is caught we can kill the
process.
This is fairly similar to the way the current timeout mechanism works,
except that the current mechanism is not thread safe. Since one of the
major features of piglit is that it offer's processes isolated
concurrency of tests, it makes sense to make the effort to provide a
timeout mechanism that can be used outside of IGT, which is a
non-concurrent test suite.
This patch look pretty substantial. It's not as big as it looks, since
it adds some fairly big tests.
---
framework/test/base.py | 127 +++++++++++-------------------------------
framework/tests/base_tests.py | 99 ++++++++++++++++++++++++++++++--
tox.ini | 7 ++-
3 files changed, 131 insertions(+), 102 deletions(-)
diff --git a/framework/test/base.py b/framework/test/base.py
index 4312cbf..03cff4f 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -25,16 +25,18 @@
from __future__ import print_function, absolute_import
import errno
import os
-import subprocess
import time
import sys
import traceback
-from datetime import datetime
-import threading
-import signal
import itertools
import abc
import copy
+import signal
+
+try:
+ import subprocess32 as subprocess
+except ImportError:
+ import subprocess
from framework import exceptions
from framework.core import Options
@@ -45,6 +47,7 @@ __all__ = [
'Test',
'TestIsSkip',
'TestRunError',
+ 'TimeoutMixin',
'ValgrindMixin',
'WindowResizeMixin',
]
@@ -107,7 +110,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
@@ -250,6 +252,9 @@ class Test(object):
raise TestRunError("Test executable not found.\n", 'skip')
else:
raise e
+ except Exception as e:
+ e.proc = proc
+ raise e
# The setter handles the bytes/unicode conversion
self.result.out = out
@@ -332,57 +337,7 @@ class ValgrindMixin(object):
self.result.result = 'fail'
-if not sys.platform.startswith('win'):
- 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
-
-
+if not sys.platform.startswith('win') and subprocess.__name__ == 'subprocess32':
class TimeoutMixin(object): # pylint: disable=too-few-public-methods
"""Mixin poviding timeout on *nix platforms
@@ -399,12 +354,6 @@ if not sys.platform.startswith('win'):
"""
timeout = 0
- def __init__(self, *args, **kwargs):
- super(TimeoutMixin, self).__init__(*args, **kwargs)
- self.__proc_timeout = None
-
- # This level of duplication sucks, but there doesn't seem to be a good
- # way around it without introducing even worse hacks.
def _run_command(self, communicate_args=None):
"""Run the test command and get the result
@@ -415,43 +364,29 @@ if not sys.platform.startswith('win'):
stderr, cwd, universal newlines.
"""
+ args = communicate_args or {}
+ args['timeout'] = self.timeout
+
try:
- proc = subprocess.Popen(self.command,
- **self._make_popen_args())
- # 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(**(communicate_args or {}))
- returncode = proc.returncode
- 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.
- if e.errno == errno.ENOENT:
- raise TestRunError("Test executable not found.\n", 'skip')
- else:
- raise e
-
- # The setter handles the bytes/unicode conversion
- self.result.out = out
- self.result.err = err
- self.result.returncode = returncode
+ super(TimeoutMixin, self)._run_command(communicate_args=args)
+ except subprocess.TimeoutExpired as e: # pylint: disable=no-member
+ e.proc.terminate()
+
+ if e.proc.poll() is None:
+ time.sleep(3)
+ os.killpg(os.getsid(e.proc.pid), signal.SIGKILL)
+
+ self.result.out, self.result.err = e.proc.communicate()
+
+ raise TestRunError(
+ 'Test run time exceeded timeout out {}\n'.format(self.timeout),
+ 'timeout')
def _make_popen_args(self):
popen_args = super(TimeoutMixin, self)._make_popen_args()
- popen_args['preexec_fn'] = os.setpgrp
+ popen_args['start_new_session'] = True
return popen_args
-
- def interpret_result(self):
- if (_is_crash_returncode(self.result.returncode) and
- self.timeout > 0 and self.__proc_timeout.join() > 0):
- self.result.result = 'timeout'
- else:
- # Only call super if we don't set the result to timeout.
- super(TimeoutMixin, self).interpret_result()
+else:
+ class TimeoutMixin(object):
+ pass
diff --git a/framework/tests/base_tests.py b/framework/tests/base_tests.py
index 6e5a734..08bb0d0 100644
--- a/framework/tests/base_tests.py
+++ b/framework/tests/base_tests.py
@@ -21,10 +21,19 @@
""" Tests for the exectest module """
from __future__ import print_function, absolute_import
+import tempfile
+import textwrap
+import os
+import mock
import nose.tools as nt
from nose.plugins.attrib import attr
+try:
+ import psutil
+except ImportError:
+ pass
+
import framework.tests.utils as utils
from framework.test.base import (
Test, WindowResizeMixin, ValgrindMixin, TimeoutMixin, TestRunError
@@ -61,10 +70,92 @@ def test_run_return_early():
@attr('slow')
+ at nt.timed(5)
+def test_timeout_kill_children():
+ """test.base.TimeoutMixin: kill children if terminate fails
+
+ This creates a process that forks multiple times, and then checks that the
+ children have been killed.
+
+ This test could leave processes running if it fails.
+
+ """
+ utils.module_check('subprocess32')
+ utils.module_check('psutil')
+
+ class Mixin(object):
+ def __init__(self, *args, **kwargs):
+ super(Mixin, self).__init__(*args, **kwargs)
+ self.proc = None
+
+ def _run_command(self, *args, **kwargs):
+ try:
+ super(Mixin, self)._run_command(*args, **kwargs)
+ except Exception as e:
+ self.proc = e.proc
+
+ # This test is aimed at testing the os.killpg functionality
+ e.proc.terminate = mock.Mock()
+
+ # Communicating will cause the pid to be released, and this
+ # will break the test
+ e.proc.communicate = mock.Mock(return_value=('out', 'err'))
+ raise e
+
+ class _Test(TimeoutMixin, Mixin, utils.Test):
+ timeout = 1
+
+ with tempfile.NamedTemporaryFile() as f:
+ # Create a process with two subproccesses (not threads) that will run
+ # for a long time.
+ f.write(textwrap.dedent("""\
+ import time
+ from multiprocessing import Process
+
+ def p():
+ for _ in range(100):
+ time.sleep(1)
+
+ a = Process(target=p)
+ b = Process(target=p)
+ a.start()
+ b.start()
+ a.join()
+ b.join()
+ """))
+ f.seek(0) # we'll need to read the file back
+
+ test = _Test(['python', f.name])
+ test.run()
+
+ proc = psutil.Process(os.getsid(test.proc.pid))
+ nt.ok_(not proc.children(),
+ msg='Test process had children when it should not have')
+
+
+ at nt.timed(5)
def test_timeout():
- """test.base.Test.run(): Sets status to 'timeout' when timeout exceeded"""
+ """test.base.TimeoutMixin: Stops running test after timeout expires
+
+ This is a little bit of extra time here, but without a sleep of 60 seconds
+ if the test runs 5 seconds it's run too long
+
+ """
+ utils.module_check('subprocess32')
utils.binary_check('sleep', 1)
- class _Test(TimeoutMixin, Test):
+ class _Test(TimeoutMixin, TestTest):
+ pass
+
+ test = _Test(['sleep', '60'])
+ test.timeout = 1
+ test.run()
+
+
+def test_timeout_timeout():
+ """test.base.TimeoutMixin: Sets status to 'timeout' when timeout exceeded"""
+ utils.module_check('subprocess32')
+ utils.binary_check('sleep', 1)
+ class _Test(TimeoutMixin, TestTest):
pass
test = _Test(['sleep', '60'])
@@ -73,10 +164,10 @@ def test_timeout():
nt.eq_(test.result.result, 'timeout')
- at attr('slow')
def test_timeout_pass():
- """test.base.Test.run(): Doesn't change status when timeout not exceeded
+ """test.base.TimeoutMixin: Doesn't change status when timeout not exceeded
"""
+ utils.module_check('subprocess32')
utils.binary_check('true')
class _Test(TimeoutMixin, Test):
def interpret_result(self):
diff --git a/tox.ini b/tox.ini
index fe58771..dbb3ddf 100644
--- a/tox.ini
+++ b/tox.ini
@@ -13,17 +13,20 @@ commands = nosetests generated_tests/test_generators.py
[testenv:py27-noaccel]
passenv=HOME
deps =
+ coverage
mako
+ mock
nose
- coverage
commands = nosetests framework/tests --attr="!privileged" --with-cover --cover-package=framework --cover-tests
[testenv:py27-accel]
passenv=HOME
deps =
+ coverage
mako
+ mock
nose
- coverage
+ psutil
simplejson
lxml
backports.lzma
--
2.6.1
More information about the Piglit
mailing list