[Piglit] [PATCH 2/2] core: don't report warn if stderr is not empty

[Marek Olšák]

The warn status is not dumped completely. Instead, the warn status can
only be returned by tests explicitly. The contents of stderr are not
analyzed at all. I think it was useless in 99% cases anyway.

Marek

On Tue, Nov 19, 2013 at 6:05 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> On Tuesday, November 19, 2013 02:57:33 AM Marek Olšák wrote:
>> I don't understand your argument. This patch makes stderr to have no
>> effect on test results, that is the only way to get "warn" is to have
>> a test which actually returns PIGLIT_WARN.
>>
>> Marek
>>
>> On Tue, Nov 19, 2013 at 12:02 AM, Dylan Baker <baker.dylan.c at gmail.com>
> wrote:
>> > On Monday, November 18, 2013 03:33:34 PM Marek Olšák wrote:
>> >> From: Marek Olšák <marek.olsak at amd.com>
>> >>
>> >> The usefulness of this is questionable to say the least.
>> >> ---
>> >>
>> >>  framework/core.py     | 72
>> >>
>> >> --------------------------------------------------- framework/exectest.py
>> >> |
>> >>
>> >>  2 --
>> >>  2 files changed, 74 deletions(-)
>> >>
>> >> diff --git a/framework/core.py b/framework/core.py
>> >> index d3e0ea9..7312764 100644
>> >> --- a/framework/core.py
>> >> +++ b/framework/core.py
>> >>
>> >> @@ -438,8 +438,6 @@ class Environment:
>> >>  class Test:
>> >> -    ignoreErrors = []
>> >> -
>> >>
>> >>      def __init__(self, runConcurrent=False):
>> >>          '''
>> >>
>> >>                  'runConcurrent' controls whether this test will
>> >>
>> >> @@ -498,31 +496,6 @@ class Test:
>> >>          else:
>> >>              status("dry-run")
>> >>
>> >> -    # Returns True iff the given error message should be ignored
>> >> -    def isIgnored(self, error):
>> >> -        for pattern in Test.ignoreErrors:
>> >> -            if pattern.search(error):
>> >> -                return True
>> >> -
>> >> -        return False
>> >> -
>> >> -    # Default handling for stderr messages
>> >> -    def handleErr(self, results, err):
>> >> -        errors = filter(lambda s: len(s) > 0,
>> >> -                        map(lambda s: s.strip(), err.split('\n')))
>> >> -
>> >> -        ignored = [s for s in errors if self.isIgnored(s)]
>> >> -        errors = [s for s in errors if s not in ignored]
>> >> -
>> >> -        if len(errors) > 0:
>> >> -            results['errors'] = errors
>> >> -
>> >> -            if results['result'] == 'pass':
>> >> -                results['result'] = 'warn'
>> >> -
>> >> -        if len(ignored) > 0:
>> >> -            results['errors_ignored'] = ignored
>> >> -
>> >>
>> >>  class Group(dict):
>> >>      pass
>> >>
>> >> @@ -644,51 +617,6 @@ def load_results(filename):
>> >>      return testrun
>> >>
>> >> -# Error messages to be ignored
>> >> -Test.ignoreErrors = map(re.compile,
>> >> -                        ["couldn't open libtxc_dxtn.so",
>> >> -                         "compression/decompression available",
>> >> -                         "Mesa: .*build",
>> >> -                         "Mesa: CPU.*",
>> >> -                         "Mesa: .*cpu detected.",
>> >> -                         "Mesa: Test.*",
>> >> -                         "Mesa: Yes.*",
>> >> -                         "libGL: XF86DRIGetClientDriverName.*",
>> >> -                         "libGL: OpenDriver: trying.*",
>> >> -                         "libGL: Warning in.*drirc*",
>> >> -                         "ATTENTION.*value of option.*",
>> >> -                         "drmOpen.*",
>> >> -                         "Mesa: Not testing OS support.*",
>> >> -                         "Mesa: User error:.*",
>> >> -                         "Mesa: Initializing .* optimizations",
>> >> -                         "debug_get_.*",
>> >> -                         "util_cpu_caps.*",
>> >> -                         "Mesa: 3Dnow! detected",
>> >> -                         "r300:.*",
>> >> -                         "radeon:.*",
>> >> -                         "Warning:.*",
>> >> -                         "0 errors, .*",
>> >> -                         "Mesa.*",
>> >> -                         "no rrb",
>> >> -                         "; ModuleID.*",
>> >> -                         "%.*",
>> >> -                         ".*failed to translate tgsi opcode.*to SSE",
>> >> -                         ".*falling back to interpreter",
>> >> -                         "GLSL version is .*, but requested version .*
>> >> is " -                         "required",
>> >> -                         "kCGErrorIllegalArgument: CGSOrderWindowList",
>> >> -                         "kCGErrorFailure: Set a breakpoint @ "
>> >> -                         "CGErrorBreakpoint\(\) to catch errors as they
>> >> are " -                         "logged.",
>> >> -                         "stw_(init|cleanup).*",
>> >> -                         "OpenGLInfo..*",
>> >> -                         "AdapterInfo..*",
>> >> -                         "frameThrottleRate.*",
>> >> -                         ".*DeviceName.*",
>> >> -                         "No memory leaks detected.",
>> >> -                         "libGL: Can't open configuration file.*"])
>> >> -
>> >> -
>> >>
>> >>  def parse_listfile(filename):
>> >>      """
>> >>      Parses a newline-seperated list in a text file and returns a python
>> >>
>> >> list diff --git a/framework/exectest.py b/framework/exectest.py
>> >> index e239940..7b1c058 100644
>> >> --- a/framework/exectest.py
>> >> +++ b/framework/exectest.py
>> >>
>> >> @@ -203,8 +203,6 @@ class ExecTest(Test):
>> >>              results['dmesg'] = dmesg_diff
>> >>              results['timeout'] = timeout
>> >>
>> >> -            self.handleErr(results, err)
>> >> -
>> >>
>> >>          else:
>> >>              results = TestResult()
>> >
>> >>              if 'result' not in results:
>> > I'm nacking this patch (I haven't looked close enough at the first one to
>> > have comments), I dont have s3tc installed on all of my test machines,
>> > and without it every single test would be warn.
>
> Sorry I'm completely misunderstanding the purpose of this patch. The plan is
> to completely dump the warn status? I'm don't understand what we gain by
> getting rid of this (other then a minor decrease in runtime), what makes this
> so dubious?