[Piglit] [PATCH] CL: don't reporting failed cases as PASS

[Dylan Baker]

First, this is not a CL specific problem. This is part of the core piglit
framework. Please only use CL: in the header if fixing a CL test or CL
specific framework

On Fri, Aug 16, 2013 at 6:38 AM, Tom Stellard <tom at stellard.net> wrote:

> On Fri, Aug 16, 2013 at 11:28:23AM +0800, Homer Hsing wrote:
> > If in multiple subtests, first failed, last passed, then subtests
> > was looked as "passed". Because "interpretResult" function overwrote
> > previous results. See framework/exectest.py, interpretResult():
> >
> >            if piglit.startswith('subtest'):
> >                if not 'subtest' in results:
> >                    results['subtest'] = {}
> > HERE->           results['subtest'].update(eval(piglit[7:]))
> >
> > For instance, if test result was
> >  ["subtest {'clz char1' : 'fail'}", "subset {'clz char1' : 'pass'}"]
> > then results['subtest'] would be 'pass'.
> >
> > "doRun" function also overwrote old results. See framework/core.py,
> >
> >          if 'subtest' in result and len(result['subtest'].keys()) > 1:
> >              for test in result['subtest'].keys():
> > HERE->           result['result'] = result['subtest'][test]
> >
> > Thus if some subtests failed, but others passed, then final result might
> > be "PASS".
> >
> > This patch fixes the bug.
> >
> > Signed-off-by: Homer Hsing <homer.xing at intel.com>
>
> Reviewed-by: Tom Stellard <thomas.stellard at amd.com>
>
> > ---
> >  framework/core.py     | 3 ++-
> >  framework/exectest.py | 7 ++++++-
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/framework/core.py b/framework/core.py
> > index 108b517..ec62231 100644
> > --- a/framework/core.py
> > +++ b/framework/core.py
> > @@ -471,7 +471,8 @@ class Test:
> >
> >              if 'subtest' in result and len(result['subtest'].keys()) >
> 1:
> >                  for test in result['subtest'].keys():
> > -                    result['result'] = result['subtest'][test]
> > +                    if 'result' not in result or result['result'] ==
> 'pass':
> > +                        result['result'] = result['subtest'][test] #
> safe update
>

This isn't really fixing the problem. For example if we have the follow
subtest results in order: pass, warn, fail, crash; then the final result
will be warn, even though crash is the most serious status, and the one
that should be displayed. It also means that if a subtest has status
'skip', that will overwrite 'pass', which is also not a desireable behavior.


> >                      json_writer.write_dict_item(path + '/' + test,
> result)
> >              else:
> >                  json_writer.write_dict_item(path, result)
> > diff --git a/framework/exectest.py b/framework/exectest.py
> > index 6ee550c..37e70bd 100644
> > --- a/framework/exectest.py
> > +++ b/framework/exectest.py
> > @@ -218,6 +218,11 @@ class PlainExecTest(ExecTest):
> >          self.command[0] = testBinDir + self.command[0]
> >
> >      def interpretResult(self, out, returncode, results):
> > +        def safe_update(dest, src):
> > +            for k in src:
> > +                if k not in dest or dest[k] == 'pass':
> > +                    dest[k] = src[k]
> > +
>

This has the same problem as outlined above


> >          outlines = out.split('\n')
> >          outpiglit = map(lambda s: s[7:],
> >                          filter(lambda s: s.startswith('PIGLIT:'),
> outlines))
> > @@ -228,7 +233,7 @@ class PlainExecTest(ExecTest):
> >                      if piglit.startswith('subtest'):
> >                          if not 'subtest' in results:
> >                              results['subtest'] = {}
> > -                        results['subtest'].update(eval(piglit[7:]))
> > +                        safe_update(results['subtest'],
> eval(piglit[7:]))
> >                      else:
> >                          results.update(eval(piglit))
> >                  out = '\n'.join(filter(lambda s: not
> s.startswith('PIGLIT:'),
> > --
> > 1.8.1.2
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130816/2768426e/attachment-0001.html>