<div dir="ltr">I use the * option in conjunction with a check just below that. The reasoning is that it allows a list file, with no additional test arguments, otherwise even with a list you are still required to have a positional argument. I can still change that, but I think it cripples the usefulness of having a list file option if an additional test result argument is still required.</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, May 25, 2013 at 2:42 AM, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</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 05/17/2013 09:31 AM, Dylan Baker wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Drops almost all of the code used in this file, all of that<br>
functionality is now handled in the summary class. Instead, the majority<br>
of this file is just the argparse instance, and a few short lines<br>
calling the new summary class.<br>
<br>
V2: Fixes the -l/--list syntax<br>
<br>
Signed-off-by: Dylan Baker <<a href="mailto:baker.dylan.c@gmail.com" target="_blank">baker.dylan.c@gmail.com</a>><br>
---<br>
piglit-summary-html.py | 284 ++++++------------------------<u></u>-------------------<br>
1 file changed, 32 insertions(+), 252 deletions(-)<br>
<br>
diff --git a/piglit-summary-html.py b/piglit-summary-html.py<br>
index 86555e3..77d63d0 100755<br>
--- a/piglit-summary-html.py<br>
+++ b/piglit-summary-html.py<br>
</blockquote></div>
[snip]<div class="im"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
parser.add_argument("<u></u>resultsFiles",<br>
- metavar = "<Results Files>",<br>
- nargs = "+",<br>
- help = "Results files to include in HTML")<br>
+ metavar = "<Results Files>",<br>
+ nargs = "*",<br>
+ help = "Results files to include in HTML")<br>
</blockquote>
<br></div>
I think this should stay as "+". Generating a report with no data at all is probably a mistake by the user, and shouldn't be allowed.<br>
<br>
Assuming you make this change, this patch is:<br>
Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>><div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
args = parser.parse_args()<br>
<br>
- core.checkDir(args.summaryDir, not args.overwrite)<br>
+ # If args.list and args.resultsFiles are empty, then give an error<br>
+ if not args.list and not args.resultsFiles:<br>
+ raise ValueError("Missing required options -l or <resultsFiles>")<br>
+<br>
+ # if overwrite is requested delete the output directory<br>
+ if path.exists(args.summaryDir) and args.overwrite:<br>
+ shutil.rmtree(args.summaryDir)<br>
</blockquote>
<br></div>
This is a change in behavior. The old --overwrite option simply overwrote files, but it didn't delete any other data you had in there. Which was pretty awful, since it meant you kept accumulating files in your summary directory (a new directory per run name) until you had several Gb of rubbish there.<br>
<br>
Traditionally, I've been in favor of just removing the option - I instead do:<br>
<br>
rm -rf summary; ./piglit-summary-html.py summary <results><br>
<br>
But I think your change here makes it actually useful, so I'm not opposed to (a) making the change, and (b) keeping the option.<div class="HOEnZb"><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- results = []<br>
- for result_dir in args.resultsFiles:<br>
- results.append(core.<u></u>loadTestResults(result_dir))<br>
+ # If the requested directory doesn't exist, create it or throw an error<br>
+ checkDir(args.summaryDir, not args.overwrite)<br>
<br>
- summary = framework.summary.Summary(<u></u>results)<br>
- for j in range(len(summary.testruns)):<br>
- tr = summary.testruns[j]<br>
- tr.codename = filter(lambda s: s.isalnum(), <a href="http://tr.name" target="_blank">tr.name</a>)<br>
- dirname = args.summaryDir + '/' + tr.codename<br>
- core.checkDir(dirname, False)<br>
- writeTestrunHtml(tr, dirname + '/index.html')<br>
- for test in summary.allTests():<br>
- filename = dirname + '/' + testPathToHtmlFilename(test.<u></u>path)<br>
- writeResultHtml(test, test.results[j], filename)<br>
+ # Add the the contents of the list files to the hand assigned results<br>
+ if args.list:<br>
+ for each in parse_listfile(args.list):<br>
+ args.resultsFiles.append(path.<u></u>expanduser(each[0]))<br>
<br>
- writefile(os.path.join(args.<u></u>summaryDir, 'result.css'), readfile(os.path.join(<u></u>templatedir, 'result.css')))<br>
- writefile(os.path.join(args.<u></u>summaryDir, 'index.css'), readfile(os.path.join(<u></u>templatedir, 'index.css')))<br>
- writeSummaryHtml(summary, args.summaryDir, 'all')<br>
- writeSummaryHtml(summary, args.summaryDir, 'problems')<br>
- writeSummaryHtml(summary, args.summaryDir, 'changes')<br>
- writeSummaryHtml(summary, args.summaryDir, 'regressions')<br>
- writeSummaryHtml(summary, args.summaryDir, 'fixes')<br>
- writeSummaryHtml(summary, args.summaryDir, 'skipped')<br>
+ # Create the HTML output<br>
+ output = summary.NewSummary(args.<u></u>resultsFiles)<br>
+ output.generateHTML(args.<u></u>summaryDir)<br>
<br>
<br>
if __name__ == "__main__":<br>
<br>
</blockquote>
<br>
</div></div></blockquote></div><br></div>