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

[Dylan Baker]

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.


On Sat, May 25, 2013 at 2:42 AM, Kenneth Graunke <kenneth at whitecape.org>wrote:

> 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__":
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130525/516a8df7/attachment.html>