<div dir="ltr"><br><div class="gmail_extra">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<br></div><div class="gmail_extra">
<br><div class="gmail_quote">On Fri, Aug 16, 2013 at 6:38 AM, Tom Stellard <span dir="ltr"><<a href="mailto:tom@stellard.net" target="_blank">tom@stellard.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Fri, Aug 16, 2013 at 11:28:23AM +0800, Homer Hsing wrote:<br>
> If in multiple subtests, first failed, last passed, then subtests<br>
> was looked as "passed". Because "interpretResult" function overwrote<br>
> previous results. See framework/exectest.py, interpretResult():<br>
><br>
>            if piglit.startswith('subtest'):<br>
>                if not 'subtest' in results:<br>
>                    results['subtest'] = {}<br>
> HERE->           results['subtest'].update(eval(piglit[7:]))<br>
><br>
> For instance, if test result was<br>
>  ["subtest {'clz char1' : 'fail'}", "subset {'clz char1' : 'pass'}"]<br>
> then results['subtest'] would be 'pass'.<br>
><br>
> "doRun" function also overwrote old results. See framework/core.py,<br>
><br>
>          if 'subtest' in result and len(result['subtest'].keys()) > 1:<br>
>              for test in result['subtest'].keys():<br>
> HERE->           result['result'] = result['subtest'][test]<br>
><br>
> Thus if some subtests failed, but others passed, then final result might<br>
> be "PASS".<br>
><br>
> This patch fixes the bug.<br>
><br>
> Signed-off-by: Homer Hsing <<a href="mailto:homer.xing@intel.com">homer.xing@intel.com</a>><br>
<br>
</div>Reviewed-by: Tom Stellard <<a href="mailto:thomas.stellard@amd.com">thomas.stellard@amd.com</a>><br>
<div class="HOEnZb"><div class="h5"><br>
> ---<br>
>  framework/core.py     | 3 ++-<br>
>  framework/exectest.py | 7 ++++++-<br>
>  2 files changed, 8 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/framework/core.py b/framework/core.py<br>
> index 108b517..ec62231 100644<br>
> --- a/framework/core.py<br>
> +++ b/framework/core.py<br>
> @@ -471,7 +471,8 @@ class Test:<br>
><br>
>              if 'subtest' in result and len(result['subtest'].keys()) > 1:<br>
>                  for test in result['subtest'].keys():<br>
> -                    result['result'] = result['subtest'][test]<br>
> +                    if 'result' not in result or result['result'] == 'pass':<br>
> +                        result['result'] = result['subtest'][test] # safe update<br></div></div></blockquote><div><br></div><div>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.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
>                      json_writer.write_dict_item(path + '/' + test, result)<br>
>              else:<br>
>                  json_writer.write_dict_item(path, result)<br>
> diff --git a/framework/exectest.py b/framework/exectest.py<br>
> index 6ee550c..37e70bd 100644<br>
> --- a/framework/exectest.py<br>
> +++ b/framework/exectest.py<br>
> @@ -218,6 +218,11 @@ class PlainExecTest(ExecTest):<br>
>          self.command[0] = testBinDir + self.command[0]<br>
><br>
>      def interpretResult(self, out, returncode, results):<br>
> +        def safe_update(dest, src):<br>
> +            for k in src:<br>
> +                if k not in dest or dest[k] == 'pass':<br>
> +                    dest[k] = src[k]<br>
> +<br></div></div></blockquote><div><br></div><div>This has the same problem as outlined above<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">
>          outlines = out.split('\n')<br>
>          outpiglit = map(lambda s: s[7:],<br>
>                          filter(lambda s: s.startswith('PIGLIT:'), outlines))<br>
> @@ -228,7 +233,7 @@ class PlainExecTest(ExecTest):<br>
>                      if piglit.startswith('subtest'):<br>
>                          if not 'subtest' in results:<br>
>                              results['subtest'] = {}<br>
> -                        results['subtest'].update(eval(piglit[7:]))<br>
> +                        safe_update(results['subtest'], eval(piglit[7:]))<br>
>                      else:<br>
>                          results.update(eval(piglit))<br>
>                  out = '\n'.join(filter(lambda s: not s.startswith('PIGLIT:'),<br>
> --<br>
> 1.8.1.2<br>
><br>
> _______________________________________________<br>
> Piglit mailing list<br>
> <a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a><br>
> <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a><br>
_______________________________________________<br>
Piglit mailing list<br>
<a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a><br>
</div></div></blockquote></div><br></div></div>