<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="ZH-CN" link="blue" vlink="purple"> <div class="WordSection1"> <p class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"Calibri","sans-serif";color:#1F497D">Thanks for reviewing and pointing out the problems. I have refined the patch, and sent it out.<o:p></o:p></span></p> <p class="MsoNormal"><span lang="EN-US" style="font-size:10.5pt;font-family:"Calibri","sans-serif";color:#1F497D">The new version is <a href="http://lists.freedesktop.org/archives/piglit/2013-August/006799.html">http://lists.freedesktop.org/archives/piglit/2013-August/006799.html</a><o:p></o:p></span></p> <p class="MsoNormal"><a name="_MailEndCompose"><span lang="EN-US" style="font-size:10.5pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></a></p> <p class="MsoNormal"><b><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif""> Dylan Baker [mailto:baker.dylan.c@gmail.com] <br> <b>Sent:</b> Saturday, August 17, 2013 1:15 AM<br> <b>To:</b> Tom Stellard<br> <b>Cc:</b> Xing, Homer; piglit@lists.freedesktop.org<br> <b>Subject:</b> Re: [Piglit] [PATCH] CL: don't reporting failed cases as PASS<o:p></o:p></span></p> <p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p> <div> <p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p> <div> <p class="MsoNormal"><span lang="EN-US">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<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p> <div> <p class="MsoNormal"><span lang="EN-US">On Fri, Aug 16, 2013 at 6:38 AM, Tom Stellard <<a href="mailto:tom@stellard.net" target="_blank">tom@stellard.net</a>> wrote:<o:p></o:p></span></p> <blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm"> <div> <p class="MsoNormal" style="margin-bottom:12.0pt"><span lang="EN-US">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>><o:p></o:p></span></p> </div> <p class="MsoNormal"><span lang="EN-US">Reviewed-by: Tom Stellard <<a href="mailto:thomas.stellard@amd.com">thomas.stellard@amd.com</a>><o:p></o:p></span></p> <div> <div> <p class="MsoNormal"><span lang="EN-US"><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<o:p></o:p></span></p> </div> </div> </blockquote> <div> <p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p> </div> <div> <p class="MsoNormal"><span lang="EN-US">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.<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span lang="EN-US"> <o:p></o:p></span></p> </div> <blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm"> <div> <div> <p class="MsoNormal"><span lang="EN-US">> 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> > +<o:p></o:p></span></p> </div> </div> </blockquote> <div> <p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p> </div> <div> <p class="MsoNormal"><span lang="EN-US">This has the same problem as outlined above<o:p></o:p></span></p> </div> <div> <p class="MsoNormal"><span lang="EN-US"> <o:p></o:p></span></p> </div> <blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm"> <div> <div> <p class="MsoNormal"><span lang="EN-US">> 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><o:p></o:p></span></p> </div> </div> </blockquote> </div> <p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p> </div> </div> </div> </body> </html>