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

[Kenneth Graunke]

On 01/21/2014 06:22 AM, Dylan Baker wrote:
> This reverts commit ab2eb660925e3680e868c5f469a4ef32e6dd9a59.

You should add some justification here so it's captured in git history:

Since the addition of timeouts, a large number of people have seen JSON
corruption when running with concurrency.  The final Piglit result file
is unusable.

Other comments below...

> ---
>  framework/exectest.py         | 28 +++-------------------------
>  framework/glsl_parser_test.py |  1 -
>  framework/shader_test.py      |  1 -
>  framework/status.py           | 25 +++++++++++++------------
>  templates/index.css           |  5 +----
>  5 files changed, 17 insertions(+), 43 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..32639d2 100644
> --- a/framework/status.py
> +++ b/framework/status.py
> @@ -29,9 +29,22 @@ warn
>  dmesg-fail
>  fail
>  crash
> +<<<<<<< HEAD

Conflict marker here...you'll definitely need to fix that.

Otherwise, both patches are:
Acked-by: Kenneth Graunke <kenneth at whitecape.org>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140122/ef2390c6/attachment.pgp>