[Piglit] [PATCH] Add support for feature readiness.
Dylan Baker
baker.dylan.c at gmail.com
Tue Oct 27 14:51:41 PDT 2015
On Tue, Oct 27, 2015 at 05:09:31PM +0000, Thomas Wood wrote:
> On 27 October 2015 at 14:23, Gabriel Feceoru <gabriel.feceoru at intel.com> wrote:
> > This adds a new "feat-summary" command to piglit which creates a HTML table
>
> The summary command already has several format options. Couldn't this
> be added as just an additional option to the existing command? Adding
> this feature as an option to the html summary output would also avoid
> the duplicated code required to handle html output.
>
Agreed. The amount of code duplicated from the summary/_html module is
way too high. Even if you need to add a new module and include things
from the _html module I think that would be better
>
> > with the feature x DUT status (useful for multiple features, multiple DUTs).
> > Another use case is the feature status for subsequent test results (depending
> > on the meaning of that test result - DUT or build)
> > A feature readiness is defined by the piglit regexp which selects the tests
> > relevant for that feature and the acceptance percentage threshold (pass rate).
> >
> > It requires an input json file containing the list of features, in the
> > following format (this is just an example):
> > {
> > "glsl" : {
> > "tests" : "-t glsl",
>
> It would probably be easier to read if the include and excludes
> filters were split into different keys, such as "include_tests" and
> "exclude_tests". This would avoid having to add an additional parser
> for the values.
>
Agreed.
>
> > "target_rate" : "90"
> > },
> > "arb" : {
> > "tests" : "-t arb_gpu",
> > "target_rate" : "95"
> > }
> > }
> > The generated main page is feat.html.
>
> "features.html" might be a more descriptive file name.
>
>
> >
> > Signed-off-by: Gabriel Feceoru <gabriel.feceoru at intel.com>
> > ---
> > framework/programs/feat_summary.py | 234 +++++++++++++++++++++++++++++++++++++
> > piglit | 5 +
> > templates/feat.mako | 82 +++++++++++++
> > 3 files changed, 321 insertions(+)
> > create mode 100644 framework/programs/feat_summary.py
> > create mode 100644 templates/feat.mako
> >
> > diff --git a/framework/programs/feat_summary.py b/framework/programs/feat_summary.py
> > new file mode 100644
> > index 0000000..88fb901
> > --- /dev/null
> > +++ b/framework/programs/feat_summary.py
> > @@ -0,0 +1,234 @@
> > +# Permission is hereby granted, free of charge, to any person
> > +# obtaining a copy of this software and associated documentation
> > +# files (the "Software"), to deal in the Software without
> > +# restriction, including without limitation the rights to use,
> > +# copy, modify, merge, publish, distribute, sublicense, and/or
> > +# sell copies of the Software, and to permit persons to whom the
> > +# Software is furnished to do so, subject to the following
> > +# conditions:
> > +#
> > +# This permission notice shall be included in all copies or
> > +# substantial portions of the Software.
> > +#
> > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> > +# KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
> > +# WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
> > +# PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHOR(S) BE
> > +# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
> > +# AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF
> > +# OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > +# DEALINGS IN THE SOFTWARE.
You should probably add 'Copyright 2015 Intel' to the top of the
copyright.
> > +
> > +from __future__ import print_function, absolute_import
And division please, this means you may have to rework some of the later
code to use // instead of /
> > +import argparse
> > +import shutil
> > +import tempfile
> > +import getpass
> > +import os
> > +import os.path as path
> > +import sys
> > +import errno
> > +import json
Anytime you need json in piglit please use this code snippit:
try:
import simplejson as json
except ImportError:
import json
> > +import re
> > +import copy
> > +import collections
> > +
> > +from mako.lookup import TemplateLookup
> > +
> > +import framework.status as so
This is bad leftovers, it would be better to add status to the 'from
framework import' line and not use so. (replace so.PASS with
status.PASS)
> > +from framework import core, backends, exceptions, profile
> > +from . import parsers
unused import
> > +
> > +__all__ = [
> > + 'html',
> > +]
> > +
> > +_TEMP_DIR = os.path.join(
> > + tempfile.gettempdir(),
> > + "piglit-{}".format(getpass.getuser()),
> > + 'version-{}'.format(sys.version.split()[0]))
> > +_TEMPLATE_DIR = os.path.join(os.path.dirname(__file__), '../..', 'templates')
> > +_TEMPLATES = TemplateLookup(
> > + _TEMPLATE_DIR,
> > + output_encoding="utf-8",
> > + encoding_errors='replace',
> > + module_directory=os.path.join(_TEMP_DIR, "html-summary"))
> > +
> > +
> > +class FeatResults(object): # pylint: disable=too-few-public-methods
> > + """Container object for results.
> > +
> > + Has the results, feature profiles and feature computed results.
> > +
> > + """
> > + def __init__(self, results, jsonFile):
Piglit uses PEP8 for python code. Variable and function names should be
lowercase and separated by underscores, constant names should be all
caps and separated by underscores. Class names should be Camel case and
should not have underscores.
excample:
a_variable_name
a_function_name
A_CONSTANT_NAME
AClassName
> > +
> > + with open(jsonFile) as data:
> > + self.feature_data = json.load(data)
> > +
> > + self.feat_fractions = {}
> > + self.feat_status = {}
> > + self.feat_summary = {}
> > + self.features = set()
> > + self.results = results
> > +
> > + _profile_orig = profile.load_test_profile(self.results[0].options['profile'][0])
> > +
> > + for feature in self.feature_data :
> > + self.features.add(feature)
> > +
> > + argv = self.feature_data[feature]["tests"].split()
> > + parser = argparse.ArgumentParser(argv)
> > + parser.add_argument("-t", "--include-tests",
> > + default=[],
> > + action="append",
> > + metavar="<regex>")
> > + parser.add_argument("-x", "--exclude-tests",
> > + default=[],
> > + action="append",
> > + metavar="<regex>",)
> > + args = parser.parse_args(argv)
See previous comments from myself and Thomas about using the parser.
> > + opts = core.Options(exclude_filter=args.exclude_tests,
> > + include_filter=args.include_tests)
> > +
> > + self.feat_summary[feature] = dict()
> > +
> > + _profile = copy.deepcopy(_profile_orig)
> > +
> > + # Handle the case of empty test list
> > + try :
> > + _profile._prepare_test_list(opts)
> > + except exceptions.PiglitFatalError:
> > + pass
You may need to catch a fatal error, but that needs some explanation,
FatalErrors are expected to kill piglit.
> > + self.feat_summary[feature]["profile"] = _profile
> > +
This looks like it's reworked from the old summary code, which I wrote
and then threw away later. I think it should be possible to either just
use a Results object from summary/common.py and some filters applied to
the Results.names.all
> > +
> > + for results in self.results :
> > + self.feat_fractions[results.name] = {}
> > + self.feat_status[results.name] = {}
> > +
> > + for feature in self.feature_data :
> > + # Create two dictionaries that have a default factory: they return
> > + # a default value instead of a key error.
> > + # This default key must be callable
> > + self.feat_fractions[results.name][feature] = \
> > + collections.defaultdict(lambda: (0, 0))
> > + self.feat_status[results.name][feature] = \
> > + collections.defaultdict(lambda: so.NOTRUN)
> > +
> > + # short names
> > + fraction = self.feat_fractions[results.name][feature]
> > + status = self.feat_status[results.name][feature]
> > +
> > + _profile = self.feat_summary[feature]["profile"]
> > +
> > + result_set = set(results.tests)
> > + profile_set = set(_profile.test_list)
> > +
> > + common_set = profile_set & result_set
> > + passed_list = [x for x in common_set if results.tests[x].result == so.PASS]
> > +
> > + total = len(common_set)
> > + passed = len(passed_list)
> > +
> > + self.feat_fractions[results.name][feature] = (passed, total)
> > + if total == 0:
> > + self.feat_status[results.name][feature] = so.NOTRUN
> > + else:
> > + if int(100 * passed / total) >= int(self.feature_data[feature]["target_rate"]) :
> > + self.feat_status[results.name][feature] = so.PASS
> > + else :
> > + self.feat_status[results.name][feature] = so.FAIL
> > +
> > +
> > +
> > +def escape_filename(key):
> > + """Avoid reserved characters in filenames."""
> > + return re.sub(r'[<>:"|?*#]', '_', key)
> > +
> > +
> > +def escape_pathname(key):
> > + """ Remove / and \\ from names """
> > + return re.sub(r'[/\\]', '_', key)
> > +
> > +
> > +def _copy_static_files(destination):
> > + """Copy static files into the results directory."""
> > + shutil.copy(os.path.join(_TEMPLATE_DIR, "index.css"),
> > + os.path.join(destination, "index.css"))
> > +
> > +
> > +def _make_feature_info(results, destination):
> > + """Create the feature readiness page."""
> > +
> > + for each in results.results:
> > + name = escape_pathname(each.name)
> > + try:
> > + os.mkdir(os.path.join(destination, name))
> > + except OSError as e:
> > + if e.errno == errno.EEXIST:
> > + raise exceptions.PiglitFatalError(
> > + 'Two or more of your results have the same "name" '
> > + 'attribute. Try changing one or more of the "name" '
> > + 'values in your json files.\n'
> > + 'Duplicate value: {}'.format(name))
> > + else:
> > + raise e
> > +
> > + with open(os.path.join(destination, name, "index.html"), 'w') as out:
> > + out.write(_TEMPLATES.get_template('testrun_info.mako').render(
> > + name=each.name,
> > + totals=each.totals['root'],
> > + time=each.time_elapsed.delta,
> > + options=each.options,
> > + uname=each.uname,
> > + glxinfo=each.glxinfo,
> > + clinfo=each.clinfo,
> > + lspci=each.lspci))
> > +
> > + with open(os.path.join(destination, "feat.html"), 'w') as out:
> > + out.write(_TEMPLATES.get_template('feat.mako').render(
> > + results=results))
> > +
> > +
> > + at exceptions.handler
> > +def html(input_):
> > + parser = argparse.ArgumentParser()
> > + parser.add_argument("-o", "--overwrite",
> > + action="store_true",
> > + help="Overwrite existing directories")
> > + parser.add_argument("featureFile",
> > + metavar="<Feature json file>",
> > + help="Json file containing the features description")
> > + parser.add_argument("summaryDir",
> > + metavar="<Summary Directory>",
> > + help="Directory to put HTML files in")
> > + parser.add_argument("resultsFiles",
> > + metavar="<Results Files>",
> > + nargs="*",
> > + help="Results files to include in HTML")
> > + args = parser.parse_args(input_)
> > +
> > + # If args.list and args.resultsFiles are empty, then raise an error
> > + if not args.featureFile and not args.resultsFiles:
> > + raise parser.error("Missing required option -l or <resultsFiles>")
> > +
> > + # If args.list and args.resultsFiles are empty, then raise an error
> > + if not args.resultsFiles or not path.exists(args.featureFile):
> > + raise parser.error("Missing json file")
> > +
> > + # if overwrite is requested delete the output directory
> > + if path.exists(args.summaryDir) and args.overwrite:
> > + shutil.rmtree(args.summaryDir)
> > +
> > + # If the requested directory doesn't exist, create it or throw an error
> > + core.checkDir(args.summaryDir, not args.overwrite)
> > +
> > + # Create the HTML output
> > + feat_res = FeatResults([backends.load(i) for i in args.resultsFiles], args.featureFile)
> > +
> > + _copy_static_files(args.summaryDir)
> > + _make_feature_info(feat_res, args.summaryDir)
> > +
> > +
> > +
> > diff --git a/piglit b/piglit
> > index 5ae43e9..37dab6e 100755
> > --- a/piglit
> > +++ b/piglit
> > @@ -106,6 +106,7 @@ def setup_module_search_path():
> > setup_module_search_path()
> > import framework.programs.run as run
> > import framework.programs.summary as summary
> > +import framework.programs.feat_summary as feat_summary
> >
> >
> > def main():
> > @@ -121,6 +122,10 @@ def main():
> > add_help=False,
> > help="resume an interrupted piglit run")
> > resume.set_defaults(func=run.resume)
> > +
> > + parse_feat_summary = subparsers.add_parser('feat-summary', help='feature summary generators')
> > + parse_feat_summary.set_defaults(func=feat_summary.html)
> > +
This belongs under the summary subparser below
> > parse_summary = subparsers.add_parser('summary', help='summary generators')
> > summary_parser = parse_summary.add_subparsers()
> > html = summary_parser.add_parser('html',
> > diff --git a/templates/feat.mako b/templates/feat.mako
> > new file mode 100644
> > index 0000000..0f04057
> > --- /dev/null
> > +++ b/templates/feat.mako
> > @@ -0,0 +1,82 @@
> > +<%!
> > + import os
> > + import posixpath # this must be posixpath, since we want /'s not \'s
> > + import re
> > + from framework import status
> > +
> > +
> > + def featResult(result):
> > + """
please pull the first line of the docstring to the same line as the
opening """
> > + Helper function for appending the results of groups to the
> > + HTML summary file.
> > + """
> > + # "Not Run" is not a valid css class replace it with skip
> > + #if css == so.NOTRUN:
> > + # css = 'skip'
This should be deleted or uncommented.
> > +
> > + return "%s/%s" % (result[0], result[1])
Piglit uses the format method not % subsitution for formatting:
'{}/{}'.format(result[0], result[1])
Although you might use this instead (assuming len(result) == 2):
'/'.join(result)
> > +
> > +
> > + def escape_filename(key):
> > + """Avoid reserved characters in filenames."""
> > + return re.sub(r'[<>:"|?*#]', '_', key)
> > +
> > +
> > + def escape_pathname(key):
> > + """ Remove / and \\ from names """
> > + return re.sub(r'[/\\]', '_', key)
> > +
> > +
> > + def normalize_href(href):
> > + """Force backward slashes in URLs."""
> > + return href.replace('\\', '/')
> > +%>
> > +
> > +<?xml version="1.0" encoding="UTF-8"?>
> > +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
> > + "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
> > +<html xmlns="http://www.w3.org/1999/xhtml">
> > + <head>
> > + <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
> > + <title>Result summary</title>
> > + <link rel="stylesheet" href="index.css" type="text/css" />
> > + </head>
> > + <body>
> > + <h1>Feature readiness</h1>
> > + <table>
> > + <colgroup>
> > + ## Name Column
> > + <col />
> > +
> > + ## Status columns
> > + ## Create an additional column for each summary
> > + % for _ in xrange(len(results.results)):
> > + <col />
> > + % endfor
> > + </colgroup>
> > + <tr>
> > + <th/>
> > + % for res in results.results:
> > + <th class="head"><b>${res.name}</b><br />\
> > + (<a href="${normalize_href(os.path.join(escape_pathname(res.name), 'index.html'))}">info</a>)</th>
you could just use posixpath.join (you've imported it) here instead of
os.path.join to get the right result
> > + % endfor
> > + </tr>
> > + % for feature in results.features:
> > + <tr>
> > + ## Add the left most column, the feature name
> > + <td>
> > + <div class="group" style="margin-left: ${0}em">
> > + <b>${feature}</b>
> > + </div>
> > + </td>
> > + ## add each group's totals
> > + % for res in results.results:
> > + <td class="${results.feat_status[res.name][feature]}">
> > + <b>${featResult(results.feat_fractions[res.name][feature])}</b>
> > + </td>
> > + % endfor
> > + </tr>
> > + % endfor
> > + </table>
> > + </body>
> > +</html>
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20151027/efd04a0e/attachment.sig>
More information about the Piglit
mailing list