[Piglit] [PATCH v2 04/11] piglit-summary-py: Use the new summary class to generate HTML

[Kenneth Graunke]

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 <baker.dylan.c at gmail.com>
> ---
>   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 <kenneth at whitecape.org>

>       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__":
>