On 05/17/2013 09:31 AM, Dylan Baker wrote:
Drops almost all of the code used in this file, all of that
functionality is now handled in the summary class. Instead, the majority
of this file is just the argparse instance, and a few short lines
calling the new summary class.

V2: Fixes the -l/--list syntax

Signed-off-by: Dylan Baker <[email protected]>
---
  piglit-summary-html.py | 284 ++++++-------------------------------------------
  1 file changed, 32 insertions(+), 252 deletions(-)

diff --git a/piglit-summary-html.py b/piglit-summary-html.py
index 86555e3..77d63d0 100755
--- a/piglit-summary-html.py
+++ b/piglit-summary-html.py
[snip]
      parser.add_argument("resultsFiles",
-                                            metavar = "<Results Files>",
-                                            nargs   = "+",
-                                            help    = "Results files to include in 
HTML")
+                        metavar = "<Results Files>",
+                        nargs   = "*",
+                        help    = "Results files to include in HTML")

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.

Assuming you make this change, this patch is:
Reviewed-by: Kenneth Graunke <[email protected]>

      args = parser.parse_args()

-    core.checkDir(args.summaryDir, not args.overwrite)
+    # If args.list and args.resultsFiles are empty, then give an error
+    if not args.list and not args.resultsFiles:
+        raise ValueError("Missing required options -l or <resultsFiles>")
+
+    # if overwrite is requested delete the output directory
+    if path.exists(args.summaryDir) and args.overwrite:
+        shutil.rmtree(args.summaryDir)

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.

Traditionally, I've been in favor of just removing the option - I instead do:

rm -rf summary; ./piglit-summary-html.py summary <results>

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.


-    results = []
-    for result_dir in args.resultsFiles:
-        results.append(core.loadTestResults(result_dir))
+    # If the requested directory doesn't exist, create it or throw an error
+    checkDir(args.summaryDir, not args.overwrite)

-    summary = framework.summary.Summary(results)
-    for j in range(len(summary.testruns)):
-        tr = summary.testruns[j]
-        tr.codename = filter(lambda s: s.isalnum(), tr.name)
-        dirname = args.summaryDir + '/' + tr.codename
-        core.checkDir(dirname, False)
-        writeTestrunHtml(tr, dirname + '/index.html')
-        for test in summary.allTests():
-            filename = dirname + '/' + testPathToHtmlFilename(test.path)
-            writeResultHtml(test, test.results[j], filename)
+    # Add the the contents of the list files to the hand assigned results
+    if args.list:
+        for each in parse_listfile(args.list):
+            args.resultsFiles.append(path.expanduser(each[0]))

-    writefile(os.path.join(args.summaryDir, 'result.css'), 
readfile(os.path.join(templatedir, 'result.css')))
-    writefile(os.path.join(args.summaryDir, 'index.css'), 
readfile(os.path.join(templatedir, 'index.css')))
-    writeSummaryHtml(summary, args.summaryDir, 'all')
-    writeSummaryHtml(summary, args.summaryDir, 'problems')
-    writeSummaryHtml(summary, args.summaryDir, 'changes')
-    writeSummaryHtml(summary, args.summaryDir, 'regressions')
-    writeSummaryHtml(summary, args.summaryDir, 'fixes')
-    writeSummaryHtml(summary, args.summaryDir, 'skipped')
+    # Create the HTML output
+    output = summary.NewSummary(args.resultsFiles)
+    output.generateHTML(args.summaryDir)


  if __name__ == "__main__":


_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to