[Piglit] [RFC 01/10] framework: refactor subtest handling
Nicolai Hähnle
nhaehnle at gmail.com
Wed Oct 11 10:26:50 UTC 2017
From: Nicolai Hähnle <nicolai.haehnle at amd.com>
The old design of TestResult and TestrunResult has two related limitations
that prevent us from relaxing process isolation more than what is done for
shader tests:
1. A TestResult can only have subtests one level below its 'root' group.
2. If one TestResult with subtests covers a group, then no other
TestResult can have tests in the same group.
This patch relaxes both constraints in the following way:
- The conceptual specialness of subtests is reduced. A TestResult is
simply a container for one or more tests that happen to have been
tested as part of the same execution (so the tests share pid and
stdout/stderr).
- TestResult now has a 'root' attribute. Without subtests, this is just
the name of the test. With subtests, it is the common prefix of all
subtests. Subtest names are relative to the root, and can be multiple
levels deep.
- TestrunResult stores a plain list of TestResults in the 'results'
attribute. For test lookups, the '_tests' dictionary caches the mapping
of tests to result object.
The users of those classes are adjusted accordingly, especially for
summary generation.
This patch does not yet change the on-disk format of results.
TODO:
- unit tests need to be adjusted
- junit backend needs to be adjusted
---
framework/grouptools.py | 18 +++++++
framework/results.py | 116 ++++++++++++++++++++++++++++++++------------
framework/summary/common.py | 7 +--
framework/summary/html_.py | 26 ++++++----
templates/index.mako | 28 +++--------
templates/test_result.mako | 4 +-
6 files changed, 130 insertions(+), 69 deletions(-)
diff --git a/framework/grouptools.py b/framework/grouptools.py
index f28241d3c..2ee74ed9f 100644
--- a/framework/grouptools.py
+++ b/framework/grouptools.py
@@ -146,20 +146,38 @@ def split(group):
"""Split the group into a list of elements.
If input is '' return an empty list
"""
if group == '':
return []
return group.split(SEPARATOR)
+def relative(root, test):
+ """Return the subgroup/subtest path of test relative to root.
+
+ Satisfies join(root, relative(root, test)) == test.
+
+ If root == test, return ''.
+
+ """
+ assert test.startswith(root), (test, root)
+ if test == root:
+ return ''
+
+ assert test[len(root)] == SEPARATOR
+ assert len(test) >= len(root) + 2
+
+ return test[len(root) + 1:]
+
+
def from_path(path):
"""Create a group from a path.
This function takes a path, and creates a group out of it.
This safely handles both Windows and Unix style paths.
"""
assert isinstance(path, six.string_types), 'Type must be string or unicode'
diff --git a/framework/results.py b/framework/results.py
index 99dd3735b..a44029f3e 100644
--- a/framework/results.py
+++ b/framework/results.py
@@ -141,25 +141,26 @@ class TimeAttribute(object):
if '__type__' in dict_:
del dict_['__type__']
return cls(**dict_)
class TestResult(object):
"""An object represting the result of a single test."""
__slots__ = ['returncode', '_err', '_out', 'time', 'command', 'traceback',
'environment', 'subtests', 'dmesg', '__result', 'images',
- 'exception', 'pid']
+ 'exception', 'pid', 'root']
err = StringDescriptor('_err')
out = StringDescriptor('_out')
def __init__(self, result=None):
+ self.root = ''
self.returncode = None
self.time = TimeAttribute()
self.command = str()
self.environment = str()
self.subtests = Subtests()
self.dmesg = str()
self.images = None
self.traceback = None
self.exception = None
self.pid = []
@@ -183,20 +184,38 @@ class TestResult(object):
return max(six.itervalues(self.subtests))
return self.__result
@result.setter
def result(self, new):
try:
self.__result = status.status_lookup(new)
except exceptions.PiglitInternalError as e:
raise exceptions.PiglitFatalError(str(e))
+ def get_result(self, key):
+ """Return the result of a (sub-)test covered by this object.
+
+ KeyError is raised if the test key is not covered.
+
+ """
+ relative = grouptools.relative(self.root, key)
+
+ if relative == '':
+ if self.subtests:
+ raise KeyError(key)
+ return self.result
+
+ try:
+ return self.subtests[relative]
+ except KeyError:
+ raise KeyError(key)
+
def to_json(self):
"""Return the TestResult as a json serializable object."""
obj = {
'__type__': 'TestResult',
'command': self.command,
'environment': self.environment,
'err': self.err,
'out': self.out,
'result': self.result,
'returncode': self.returncode,
@@ -268,20 +287,24 @@ class Totals(dict):
def __bool__(self):
# Since totals are prepopulated, calling 'if not <Totals instance>'
# will always result in True, this will cause it to return True only if
# one of the values is not zero
for each in six.itervalues(self):
if each != 0:
return True
return False
+ def add(self, other):
+ for each in status.ALL:
+ self[each] += other[each]
+
def to_json(self):
"""Convert totals to a json object."""
result = copy.copy(self)
result['__type__'] = 'Totals'
return result
@classmethod
def from_dict(cls, dict_):
"""Convert a dictionary into a Totals object."""
tots = cls(dict_)
@@ -294,71 +317,90 @@ class TestrunResult(object):
"""The result of a single piglit run."""
def __init__(self):
self.name = None
self.uname = None
self.options = {}
self.glxinfo = None
self.wglinfo = None
self.clinfo = None
self.lspci = None
self.time_elapsed = TimeAttribute()
- self.tests = collections.OrderedDict()
+ self.results = []
+ self._tests = {}
self.totals = collections.defaultdict(Totals)
+ def get_testresult(self, key):
+ """Get the TestResult object that contains the test.
+
+ If the test does not exist, return None.
+
+ The returned object may contain the given test only as a subtest.
+
+ Arguments:
+ key -- the key name of the test whose TestResult object is returned
+
+ """
+ return self._tests.get(key)
+
def get_result(self, key):
- """Get the result of a test or subtest.
+ """Get the result of a test.
- If neither a test nor a subtest instance exist, then raise the original
- KeyError generated from looking up <key> in the tests attribute. It is
- the job of the caller to handle this error.
+ KeyError is raised when the test does not exist.
Arguments:
key -- the key name of the test to return
"""
- try:
- return self.tests[key].result
- except KeyError as e:
- name, test = grouptools.splitname(key)
- try:
- return self.tests[name].subtests[test]
- except KeyError:
- raise e
+ return self._tests[key].get_result(key)
+
+ @property
+ def testnames(self):
+ """Return an iterable of all test names.
+ """
+ return six.iterkeys(self._tests)
def calculate_group_totals(self):
"""Calculate the number of passes, fails, etc at each level."""
- for name, result in six.iteritems(self.tests):
+ worklist = collections.defaultdict(Totals)
+
+ for result in self.results:
# If there are subtests treat the test as if it is a group instead
# of a test.
if result.subtests:
- for res in six.itervalues(result.subtests):
+ for subtest, res in six.iteritems(result.subtests):
res = str(res)
- temp = name
-
- self.totals[temp][res] += 1
- while temp:
- temp = grouptools.groupname(temp)
- self.totals[temp][res] += 1
- self.totals['root'][res] += 1
+ name = grouptools.groupname(grouptools.join(result.root, subtest))
+ worklist[name][res] += 1
else:
res = str(result.result)
- while name:
+ name = grouptools.groupname(result.root)
+ worklist[name][res] += 1
+
+ while worklist:
+ # Sorting the keys in reverse orders means that sub-groups appear
+ # before their parent group.
+ for name in sorted(six.iterkeys(worklist), reverse=True):
+ total = worklist.pop(name)
+
+ if name:
+ self.totals[name].add(total)
name = grouptools.groupname(name)
- self.totals[name][res] += 1
- self.totals['root'][res] += 1
+ worklist[name].add(total)
+ else:
+ self.totals['root'].add(total)
def to_json(self):
if not self.totals:
self.calculate_group_totals()
rep = copy.copy(self.__dict__)
- rep['tests'] = collections.OrderedDict((k, t.to_json())
- for k, t in six.iteritems(self.tests))
+ rep['tests'] = collections.OrderedDict((t.root, t.to_json())
+ for t in self.results)
rep['__type__'] = 'TestrunResult'
return rep
@classmethod
def from_dict(cls, dict_, _no_totals=False):
"""Convert a dictionary into a TestrunResult.
This method is meant to be used for loading results from json or
similar formats
@@ -372,20 +414,34 @@ class TestrunResult(object):
value = dict_.get(name)
if value:
setattr(res, name, value)
# Since this is used to load partial metadata when writing final test
# results there is no guarantee that this will have a "time_elapsed"
# key
if 'time_elapsed' in dict_:
setattr(res, 'time_elapsed',
TimeAttribute.from_dict(dict_['time_elapsed']))
- res.tests = collections.OrderedDict((n, TestResult.from_dict(t))
- for n, t in six.iteritems(dict_['tests']))
+
+ for root, result_dict in six.iteritems(dict_['tests']):
+ result = TestResult.from_dict(result_dict)
+ result.root = root
+ res.results.append(result)
+
+ if result.subtests:
+ for subtest in six.iterkeys(result.subtests):
+ fullname = grouptools.join(root, subtest)
+ assert fullname not in res._tests
+ res._tests[fullname] = result
+ else:
+ if result.root in res._tests:
+ # This can happen with old resumed test results.
+ print('Warning: duplicate results for {}'.format(result.root))
+ res._tests[result.root] = result
if not 'totals' in dict_ and not _no_totals:
res.calculate_group_totals()
else:
res.totals = {n: Totals.from_dict(t) for n, t in
six.iteritems(dict_['totals'])}
return res
diff --git a/framework/summary/common.py b/framework/summary/common.py
index b5e38b4f1..594eedc45 100644
--- a/framework/summary/common.py
+++ b/framework/summary/common.py
@@ -90,26 +90,21 @@ class Names(object):
def __single(self, comparator):
"""Helper for simplifying comparators using find_single."""
return find_single(self.__results, self.all, comparator)
@lazy_property
def all(self):
"""A set of all tests in all runs."""
all_ = set()
for res in self.__results:
- for key, value in six.iteritems(res.tests):
- if not value.subtests:
- all_.add(key)
- else:
- for subt in six.iterkeys(value.subtests):
- all_.add(grouptools.join(key, subt))
+ all_.update(res.testnames)
return all_
@lazy_property
def changes(self):
def handler(names, name, prev, cur):
"""Handle missing tests.
For changes we want literally anything where the first result
isn't the same as the second result.
diff --git a/framework/summary/html_.py b/framework/summary/html_.py
index 512b42c24..a269ea802 100644
--- a/framework/summary/html_.py
+++ b/framework/summary/html_.py
@@ -34,21 +34,21 @@ import tempfile
import traceback
import mako
from mako.lookup import TemplateLookup
import six
# a local variable status exists, prevent accidental overloading by renaming
# the module
from framework import backends, exceptions, core
-from .common import Results, escape_filename, escape_pathname
+from .common import Results, escape_pathname
from .feature import FeatResults
__all__ = [
'html',
'feat'
]
_TEMP_DIR = os.path.join(
tempfile.gettempdir(),
getpass.getuser(),
@@ -70,21 +70,21 @@ _TEMPLATES = TemplateLookup(
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"))
shutil.copy(os.path.join(_TEMPLATE_DIR, "result.css"),
os.path.join(destination, "result.css"))
-def _make_testrun_info(results, destination, exclude=None):
+def _make_testrun_info(results, destination, result_html_map, exclude=None):
"""Create the pages for each results file."""
exclude = exclude or {}
result_css = os.path.join(destination, "result.css")
index = os.path.join(destination, "index.html")
for each in results.results:
name = escape_pathname(each.name)
try:
core.check_dir(os.path.join(destination, name), True)
except exceptions.PiglitException:
@@ -99,62 +99,66 @@ def _make_testrun_info(results, destination, exclude=None):
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))
# Then build the individual test results
- for key, value in six.iteritems(each.tests):
- html_path = os.path.join(destination, name,
- escape_filename(key + ".html"))
+ for idx, value in enumerate(each.results):
+ html_relative_path = os.path.join(name,
+ "{}.html".format(idx))
+ html_path = os.path.join(destination, html_relative_path)
temp_path = os.path.dirname(html_path)
+ result_html_map[value] = html_relative_path
+
if value.result not in exclude:
core.check_dir(temp_path)
try:
with open(html_path, 'wb') as out:
out.write(_TEMPLATES.get_template(
'test_result.mako').render(
- testname=key,
value=value,
css=os.path.relpath(result_css, temp_path),
index=os.path.relpath(index, temp_path)))
except OSError as e:
traceback.print_exc()
-def _make_comparison_pages(results, destination, exclude):
+def _make_comparison_pages(results, destination, result_html_map, exclude):
"""Create the pages of comparisons."""
pages = frozenset(['changes', 'problems', 'skips', 'fixes',
'regressions', 'enabled', 'disabled'])
# Index.html is a bit of a special case since there is index, all, and
# alltests, where the other pages all use the same name. ie,
# changes.html, changes, and page=changes.
with open(os.path.join(destination, "index.html"), 'wb') as out:
out.write(_TEMPLATES.get_template('index.mako').render(
results=results,
+ result_html_map=result_html_map,
page='all',
pages=pages,
exclude=exclude))
# Generate the rest of the pages
for page in pages:
with open(os.path.join(destination, page + '.html'), 'wb') as out:
# If there is information to display display it
if sum(getattr(results.counts, page)) > 0:
out.write(_TEMPLATES.get_template('index.mako').render(
results=results,
+ result_html_map=result_html_map,
pages=pages,
page=page,
exclude=exclude))
# otherwise provide an empty page
else:
out.write(
_TEMPLATES.get_template('empty_status.mako').render(
page=page, pages=pages))
@@ -172,22 +176,24 @@ def html(results, destination, exclude):
Basically all this does is takes the information provided by the
constructor, and passes it to mako templates to generate HTML files.
The beauty of this approach is that mako is leveraged to do the
heavy lifting, this method just passes it a bunch of dicts and lists
of dicts, which mako turns into pretty HTML.
"""
results = Results([backends.load(i) for i in results])
_copy_static_files(destination)
- _make_testrun_info(results, destination, exclude)
- _make_comparison_pages(results, destination, exclude)
+ result_html_map = {}
+ _make_testrun_info(results, destination, result_html_map, exclude)
+ _make_comparison_pages(results, destination, result_html_map, exclude)
def feat(results, destination, feat_desc):
"""Produce HTML feature readiness summary."""
feat_res = FeatResults([backends.load(i) for i in results], feat_desc)
_copy_static_files(destination)
- _make_testrun_info(feat_res, destination)
+ result_html_map = {}
+ _make_testrun_info(feat_res, result_html_map, destination)
_make_feature_info(feat_res, destination)
diff --git a/templates/index.mako b/templates/index.mako
index 49a8b1c86..1f263ab42 100644
--- a/templates/index.mako
+++ b/templates/index.mako
@@ -35,25 +35,20 @@
den = 0
for k, v in six.iteritems(result.totals[group]):
if v > 0:
s = status.status_lookup(k)
num += s.fraction[0] * v
den += s.fraction[1] * v
return '{}/{}'.format(num, den)
- 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('\\', '/')
%>
@@ -138,52 +133,43 @@
## add each group's totals
% for res in results.results:
<td class="${group_result(res, group)}">
<b>${group_fraction(res, group)}</b>
</td>
% endfor
<% depth += 1 %>
</tr><tr>
% endfor
% endif
-
+
<td>
<div class="group" style="margin-left: ${depth * 1.75}em">
${grouptools.testname(test)}
</div>
</td>
% for res in results.results:
<%
- # Get the raw result, if it's none check to see if it's a subtest, if that's still None
- # then declare it not run
+ # Get the raw result
# This very intentionally uses posix path, we're generating urls, and while
# some windows based browsers support \\ as a url separator, *nix systems do not,
# which would make a result generated on windows non-portable
- raw = res.tests.get(test)
+ raw = res.get_testresult(test)
if raw is not None:
- result = raw.result
- href = normalize_href(posixpath.join(escape_pathname(res.name),
- escape_filename(test)))
+ result = raw.get_result(test)
+ href = normalize_href(result_html_map[raw])
else:
- raw = res.tests.get(grouptools.groupname(test))
- name = grouptools.testname(test)
- if raw is not None and name in raw.subtests:
- result = raw.subtests[name]
- href = normalize_href(posixpath.join(escape_pathname(res.name),
- escape_filename(grouptools.groupname(test))))
- else:
- result = status.NOTRUN
+ result = status.NOTRUN
del raw # we don't need this, so don't let it leak
%>
<td class="${str(result)}">
% if str(result) not in exclude and result is not status.NOTRUN:
- <a href="${href}.html">
+ <a href="${href}">
${str(result)}
</a>
% else:
${str(result)}
% endif
</td>
% endfor
</tr>
% endfor
</table>
diff --git a/templates/test_result.mako b/templates/test_result.mako
index ff08797bc..7cd70891e 100644
--- a/templates/test_result.mako
+++ b/templates/test_result.mako
@@ -1,21 +1,21 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//END"
"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>${testname} - Details</title>
+ <title>${value.root} - Details</title>
<link rel="stylesheet" href="${css}" type="text/css" />
</head>
<body>
- <h1>Results for ${testname}</h1>
+ <h1>Results for ${value.root}</h1>
<h2>Overview</h2>
<div>
<p><b>Result:</b> ${value.result}</p>
</div>
<p><a href="${index}">Back to summary</a></p>
<h2>Details</h2>
<table>
<tr>
<th>Detail</th>
<th>Value</th>
--
2.11.0
More information about the Piglit
mailing list