Thanks for reviewing and pointing out the problems. I have refined the patch, and sent it out. The new version is http://lists.freedesktop.org/archives/piglit/2013-August/006799.html
From: Dylan Baker [mailto:[email protected]] Sent: Saturday, August 17, 2013 1:15 AM To: Tom Stellard Cc: Xing, Homer; [email protected] Subject: Re: [Piglit] [PATCH] CL: don't reporting failed cases as PASS 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 <[email protected]<mailto:[email protected]>> 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 <[email protected]<mailto:[email protected]>> Reviewed-by: Tom Stellard <[email protected]<mailto:[email protected]>> > --- > 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 > [email protected]<mailto:[email protected]> > http://lists.freedesktop.org/mailman/listinfo/piglit _______________________________________________ Piglit mailing list [email protected]<mailto:[email protected]> http://lists.freedesktop.org/mailman/listinfo/piglit
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
