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

[Dylan Baker]

On Monday 19 August 2013 08:59:51 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. For example, if we have the follow subtest
> results in order: "pass, warn, fail, crash"; then the final result will
> be "crash". And "skip" will not overwrite "pass".
> 
> Signed-off-by: Homer Hsing <homer.xing at intel.com>
> ---
>  framework/core.py     |  6 +++++-
>  framework/exectest.py | 10 +++++++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/framework/core.py b/framework/core.py
> index 108b517..f0edbe7 100644
> --- a/framework/core.py
> +++ b/framework/core.py
> @@ -470,8 +470,12 @@ class Test:
>              status(result['result'])
> 
>              if 'subtest' in result and len(result['subtest'].keys()) > 1:
> +                def serious_level(result):
> +                    return {'skip':0, 'pass':1, 'warn':2, 'crash':3,
> 'fail':4}.get(result, 0) +

Order should be: skip, pass, warn, fail, crash
Crash is a much more serious problem than fail.

>                  for test in result['subtest'].keys():
> -                    result['result'] = result['subtest'][test]
> +                    if serious_level(result['subtest'][test]) >=
> serious_level(result['result']): +                        result['result']
> = result['subtest'][test] 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..42c3ebb 100644
> --- a/framework/exectest.py
> +++ b/framework/exectest.py
> @@ -218,6 +218,14 @@ class PlainExecTest(ExecTest):
>          self.command[0] = testBinDir + self.command[0]
> 
>      def interpretResult(self, out, returncode, results):
> +        def serious_level(result):
> +            return {'skip':0, 'pass':1, 'warn':2, 'crash':3,
> 'fail':4}.get(result, 0) +

same as above

> +        def safe_update(dest, src):
> +            for k in src:
> +                if k not in dest or serious_level(src[k]) >=
> serious_level(dest[k]): +                    dest[k] = src[k]
> +
>          outlines = out.split('\n')
>          outpiglit = map(lambda s: s[7:],
>                          filter(lambda s: s.startswith('PIGLIT:'),
> outlines)) @@ -228,7 +236,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:'),

With the above changes this patch is
Reviewed-by: Dylan Baker <baker.dylan.c at gmail.com>