[Piglit] [PATCH] framework: Add support for a test timeout

Daniel Vetter daniel.vetter at ffwll.ch
Wed Nov 13 09:16:34 PST 2013


i-g-t tests can take a long time, and for a bunch of reasons (bad
tuning on disparate set of platforms, stuck in the kernel which often
can be recovered by interrupting with a signal, ...) that sometimes
extends to a good approximation of forever.

Hence this adds a per-test timeout value and a bit of infrastructure
to adjust the results. Test results adjusting is done after calling
interpretResult so that we don't have to replicate this all over the
place. This might need to be adjusted for the piglit-native subtest
stuff, but otoh igt is a bit special with it's crazy long-running
tests. So I think we can fix this once it's actually needed.

The default timeout is None, so this is purely opt-in.

Note on the implementation: SIG_ALARM doesn't exists on Windows and
stackoverflow overwhelmingly recommended to go with this thread-based
approach here. But only tested on my Linux box here.

I've also timed quick.tests run a bit and the overhead due to the
additional thread we launch seems to be in the wash. So I didn't opt
to make the thread launching optional if there's no timeout limit.

v2: Also add all the boilerplate needed to handle the new test status
in summaries. For the color I've opted for two shades of light blue,
they nicely stick out from the current selection.

v3: Fix GLSLParserTest and ShaderTest. They both derive from
PlainExecTest but only call the __init__ function of the Test
baseclass. I haven't really figured why this is and also not really
what command I should pass to PlainExecTest.__init__, so just
replicated the init code for now.

v4: Initialize timeout earlier for tests where we use the ENOENT
handling to skip them.

Fix up the out, err passing from the thread. Apparently my idea of how
python closures work was completely misguided - like with a function
call a closure captures a copy of of a reference pointing at the
original object. So assinging anything to them won't have any effect
outside of the lambda. Hence we need to jump through hoops and pass an
array around. Nicer fix would be to create a class or something, but
that seems like overkill.

Cc: Dylan Baker <baker.dylan.c at gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 framework/exectest.py         | 28 +++++++++++++++++++++++++---
 framework/glsl_parser_test.py |  1 +
 framework/shader_test.py      |  1 +
 framework/status.py           | 39 ++++++++++++++++++++++++++-------------
 templates/index.css           |  5 ++++-
 5 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/framework/exectest.py b/framework/exectest.py
index 986d8032cb37..e239940fe891 100644
--- a/framework/exectest.py
+++ b/framework/exectest.py
@@ -23,6 +23,7 @@
 import errno
 import os
 import subprocess
+import threading
 import shlex
 import types
 import re
@@ -72,6 +73,7 @@ class ExecTest(Test):
         self.command = command
         self.split_command = os.path.split(self.command[0])[1]
         self.env = {}
+        self.timeout = None
 
         if isinstance(self.command, basestring):
             self.command = shlex.split(str(self.command))
@@ -113,7 +115,7 @@ class ExecTest(Test):
                 else:
                     if env.dmesg:
                         old_dmesg = read_dmesg()
-                    (out, err, returncode) = \
+                    (out, err, returncode, timeout) = \
                         self.get_command_result(command, fullenv)
                     if env.dmesg:
                         dmesg_diff = get_dmesg_diff(old_dmesg, read_dmesg())
@@ -172,6 +174,9 @@ class ExecTest(Test):
             elif returncode != 0:
                 results['note'] = 'Returncode was {0}'.format(returncode)
 
+            if timeout:
+                results['result'] = 'timeout'
+
             if env.valgrind:
                 # If the underlying test failed, simply report
                 # 'skip' for this valgrind test.
@@ -196,6 +201,7 @@ class ExecTest(Test):
             results['returncode'] = returncode
             results['command'] = ' '.join(self.command)
             results['dmesg'] = dmesg_diff
+            results['timeout'] = timeout
 
             self.handleErr(results, err)
 
@@ -217,13 +223,29 @@ class ExecTest(Test):
 
     def get_command_result(self, command, fullenv):
         try:
+            timeout = False
             proc = subprocess.Popen(command,
                                     stdout=subprocess.PIPE,
                                     stderr=subprocess.PIPE,
                                     env=fullenv,
                                     universal_newlines=True)
-            out, err = proc.communicate()
+            output = ['', '']
+
+            def thread_fn():
+                output[0], output[1] = proc.communicate()
+
+            thread = threading.Thread(target=thread_fn)
+            thread.start()
+
+            thread.join(self.timeout)
+
+            if thread.is_alive():
+                proc.terminate()
+                thread.join()
+                timeout = True
+
             returncode = proc.returncode
+            out, err = output
         except OSError as e:
             # Different sets of tests get built under
             # different build configurations.  If
@@ -237,7 +259,7 @@ class ExecTest(Test):
                 returncode = None
             else:
                 raise e
-        return out, err, returncode
+        return out, err, returncode, timeout
 
 
 class PlainExecTest(ExecTest):
diff --git a/framework/glsl_parser_test.py b/framework/glsl_parser_test.py
index 3b1a8936c65a..74034e871dcf 100755
--- a/framework/glsl_parser_test.py
+++ b/framework/glsl_parser_test.py
@@ -197,6 +197,7 @@ class GLSLParserTest(PlainExecTest):
         self.__command = None
         self.__filepath = filepath
         self.result = None
+        self.timeout = None
 
     def __get_config(self):
         """Extract the config section from the test file.
diff --git a/framework/shader_test.py b/framework/shader_test.py
index ea1d46265e25..f5a2bf95c5e7 100755
--- a/framework/shader_test.py
+++ b/framework/shader_test.py
@@ -145,6 +145,7 @@ class ShaderTest(PlainExecTest):
         self.__gl_api = None
 
         self.env = {}
+        self.timeout = None
 
     def __report_failure(self, message):
         if self.__run_standalone:
diff --git a/framework/status.py b/framework/status.py
index 2fe153eeb01a..c9509db42a40 100644
--- a/framework/status.py
+++ b/framework/status.py
@@ -27,27 +27,30 @@ warn
 dmesg-fail
 fail
 crash
+timeout
 skip
 
 
 The following are regressions:
 
-pass|warn|dmesg-warn|fail|dmesg-fail|crash -> skip
-pass|warn|dmesg-warn|fail|dmesg-fail -> crash|skip
-pass|warn|dmesg-warn|fail -> dmesg-fail|crash|skip
-pass|warn|dmesg-warn -> fail|dmesg-fail|crash|skip
-pass|warn -> dmesg-warn|fail|dmesg-fail|crash|skip
-pass -> warn|dmesg-warn|fail|dmesg-fail|crash|skip
+pass|warn|dmesg-warn|fail|dmesg-fail|crash|timeout -> skip
+pass|warn|dmesg-warn|fail|dmesg-fail|crash -> timeout|skip
+pass|warn|dmesg-warn|fail|dmesg-fail -> crash|timeout|skip
+pass|warn|dmesg-warn|fail -> dmesg-fail|crash|timeout|skip
+pass|warn|dmesg-warn -> fail|dmesg-fail|crash|timeout|skip
+pass|warn -> dmesg-warn|fail|dmesg-fail|crash|timeout|skip
+pass -> warn|dmesg-warn|fail|dmesg-fail|crash|timeout|skip
 
 
 The following are fixes:
 
-skip -> pass|warn|dmesg-warn|fail|dmesg-fail|crash
-crash|skip - >pass|warn|dmesg-warn|fail|dmesg-fail
-dmesg-fail|crash|skip -> pass|warn|dmesg-warn|fail
-fail|dmesg-fail|crash|skip -> pass|warn|dmesg-warn
-dmesg-warn|fail|dmesg-fail|crash|skip -> pass|warn
-warn|dmesg-warn|fail|dmesg-fail|crash|skip -> pass
+skip -> pass|warn|dmesg-warn|fail|dmesg-fail|crash|timeout
+timeout|skip -> pass|warn|dmesg-warn|fail|dmesg-fail|crash
+crash|timeout|skip - >pass|warn|dmesg-warn|fail|dmesg-fail
+dmesg-fail|crash|timeout|skip -> pass|warn|dmesg-warn|fail
+fail|dmesg-fail|crash|timeout|skip -> pass|warn|dmesg-warn
+dmesg-warn|fail|dmesg-fail|crash|timeout|skip -> pass|warn
+warn|dmesg-warn|fail|dmesg-fail|crash|timeout|skip -> pass
 
 
 NotRun -> * and * -> NotRun is a change, but not a fix or a regression. This is
@@ -71,6 +74,7 @@ def status_lookup(status):
                    'crash': Crash,
                    'dmesg-warn': DmesgWarn,
                    'dmesg-fail': DmesgFail,
+                   'timeout' : Timeout,
                    'notrun': NotRun}
 
     try:
@@ -205,9 +209,18 @@ class Crash(Status):
         pass
 
 
+class Timeout(Status):
+    name = 'timeout'
+    value = 50
+    fraction = (0, 0)
+
+    def __init__(self):
+        pass
+
+
 class Skip(Status):
     name = 'skip'
-    value = 50
+    value = 60
     fraction = (0, 0)
 
     def __init__(self):
diff --git a/templates/index.css b/templates/index.css
index 577370cec677..3389738abb55 100644
--- a/templates/index.css
+++ b/templates/index.css
@@ -36,7 +36,7 @@ td:first-child > div {
 	background-color: #c8c838
 }
 
-td.skip, td.warn, td.fail, td.pass, td.trap, td.abort, td.crash, td.dmesg-warn, td.dmesg-fail {
+td.skip, td.warn, td.fail, td.pass, td.trap, td.abort, td.crash, td.dmesg-warn, td.dmesg-fail, td.timeout {
 	text-align: right;
 }
 
@@ -67,6 +67,9 @@ tr:nth-child(even) td.fail  { background-color: #e00505; }
 tr:nth-child(odd)  td.dmesg-fail  { background-color: #ff2020; }
 tr:nth-child(even) td.dmesg-fail  { background-color: #e00505; }
 
+tr:nth-child(odd)  td.timeout  { background-color: #83bdf6; }
+tr:nth-child(even) td.timeout  { background-color: #4a9ef2; }
+
 tr:nth-child(odd)  td.trap  { background-color: #111111; }
 tr:nth-child(even) td.trap  { background-color: #000000; }
 tr:nth-child(odd)  td.abort { background-color: #111111; }
-- 
1.8.1.4



More information about the Piglit mailing list