[Piglit] [Patch V2 2/2] Revert "framework: Add support for a test timeout"

Daniel Vetter daniel at ffwll.ch
Thu Jan 23 04:00:50 PST 2014


On Wed, Jan 22, 2014 at 11:20:56AM -0800, Dylan Baker wrote:
> This reverts commit ab2eb660925e3680e868c5f469a4ef32e6dd9a59.
> 
> This patch has a bad interaction with the locking in the json writing
> library, running concurrently with this patch enabled can cause json
> corruption that cannot easily be untangled, and even when it is is the
> result is of low trustworthiness.
> 
> v2: - Add message explaining why this patch is to be reverted
>     - fix framework/status.py docstring that was mangled after the
>       revert

Ack. Note that I've occasionally also seen case where the timeout didn't
work and python got stuck in really odd places waiting for apparently
nothing. So something's seriously screwed up here :(
-Daniel
> ---
>  framework/exectest.py         | 28 +++-------------------------
>  framework/glsl_parser_test.py |  1 -
>  framework/shader_test.py      |  1 -
>  framework/status.py           |  9 ---------
>  templates/index.css           |  5 +----
>  5 files changed, 4 insertions(+), 40 deletions(-)
> 
> diff --git a/framework/exectest.py b/framework/exectest.py
> index 15dd966..cd96f9f 100644
> --- a/framework/exectest.py
> +++ b/framework/exectest.py
> @@ -23,7 +23,6 @@
>  import errno
>  import os
>  import subprocess
> -import threading
>  import shlex
>  import types
>  import re
> @@ -73,7 +72,6 @@ class ExecTest(Test):
>          self.command = command
>          self.split_command = os.path.split(self._command[0])[1]
>          self.env = {}
> -        self.timeout = None
>  
>  
>          self.skip_test = self.check_for_skip_scenario(command)
> @@ -124,7 +122,7 @@ class ExecTest(Test):
>                  else:
>                      if env.dmesg:
>                          old_dmesg = read_dmesg()
> -                    (out, err, returncode, timeout) = \
> +                    (out, err, returncode) = \
>                          self.get_command_result(command, fullenv)
>                      if env.dmesg:
>                          dmesg_diff = get_dmesg_diff(old_dmesg, read_dmesg())
> @@ -183,9 +181,6 @@ 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.
> @@ -210,7 +205,6 @@ class ExecTest(Test):
>              results['returncode'] = returncode
>              results['command'] = ' '.join(self.command)
>              results['dmesg'] = dmesg_diff
> -            results['timeout'] = timeout
>  
>          else:
>              results = TestResult()
> @@ -230,29 +224,13 @@ 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)
> -            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
> -
> +            out, err = proc.communicate()
>              returncode = proc.returncode
> -            out, err = output
>          except OSError as e:
>              # Different sets of tests get built under
>              # different build configurations.  If
> @@ -266,7 +244,7 @@ class ExecTest(Test):
>                  returncode = None
>              else:
>                  raise e
> -        return out, err, returncode, timeout
> +        return out, err, returncode
>  
>  
>  class PlainExecTest(ExecTest):
> diff --git a/framework/glsl_parser_test.py b/framework/glsl_parser_test.py
> index 91f59cb..de73ff7 100755
> --- a/framework/glsl_parser_test.py
> +++ b/framework/glsl_parser_test.py
> @@ -197,7 +197,6 @@ 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 aebb5d7..af6f209 100755
> --- a/framework/shader_test.py
> +++ b/framework/shader_test.py
> @@ -145,7 +145,6 @@ 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 fda2e58..4264577 100644
> --- a/framework/status.py
> +++ b/framework/status.py
> @@ -64,7 +64,6 @@ def status_lookup(status):
>                     'crash': Crash,
>                     'dmesg-warn': DmesgWarn,
>                     'dmesg-fail': DmesgFail,
> -                   'timeout': Timeout,
>                     'notrun': NotRun}
>  
>      try:
> @@ -206,11 +205,3 @@ class Crash(Status):
>  
>      def __init__(self):
>          pass
> -
> -
> -class Timeout(Status):
> -    name = 'timeout'
> -    value = 50
> -
> -    def __init__(self):
> -        pass
> diff --git a/templates/index.css b/templates/index.css
> index 3389738..577370c 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.timeout {
> +td.skip, td.warn, td.fail, td.pass, td.trap, td.abort, td.crash, td.dmesg-warn, td.dmesg-fail {
>  	text-align: right;
>  }
>  
> @@ -67,9 +67,6 @@ 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.5.3
> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


More information about the Piglit mailing list