On 05/17/2013 09:31 AM, Dylan Baker wrote:
With 10,000+ tests all living in the same folder it can be hard to
identify a single file, especially when trying to work with the HTML
generation itself. This patch changes the behavior so that each test
has a directory tree for the group results with tests under it (ie
spec/GLSL 1.0/foo/bar/test.html)
A side effect of this is that the group names of tests don't have to be
altered to create a valid name (since the slashes in the group names
just become the slashes in the folder structure) and this was the single
most expensive operation of HTML generation (about 1/3 of the total
time). This is a tidy little speedup.
I'm nervous about skipping sanitizing of output filenames entirely. It
seems kind of dangerous. For example, I could (accidentally or
maliciously) create a file named tests/shaders/~.shader_test or
tests/shaders/$HOME.frag and it'll automatically create tests of similar
names. I suppose since '/' can't be part of a filename, it might work
out fine, but I'd rather not have to think about it...
Regarding the speed, as I mentioned earlier, I think your implementation
of sanitizePath could be a lot more efficient. tr/translate would be
much faster, for example.
You could probably also just sanitize it once (instead of twice) and
pass the path name around. Dunno.
I'm not opposed to putting things in subdirectories.
Signed-off-by: Dylan Baker <[email protected]>
---
framework/summary.py | 76 ++++++++++++++++------------------------------
templates/test_result.mako | 6 ++--
2 files changed, 30 insertions(+), 52 deletions(-)
diff --git a/framework/summary.py b/framework/summary.py
index 867dbad..9f2a924 100644
--- a/framework/summary.py
+++ b/framework/summary.py
@@ -472,8 +472,7 @@ class BuildHTMLIndex(list):
"""
self.append({'type': 'testResult',
'class': text,
- 'href': path.join(self._sanitizePath(group),
- self._sanitizePath(href + ".html")),
+ 'href': path.join(group, href + ".html"),
'text': text})
def _subtestResult(self, group, text):
@@ -489,29 +488,6 @@ class BuildHTMLIndex(list):
'class': text,
'text': text})
- # This will be removed from here in a later patch
- def _sanitizePath(self, path):
- """
- Private: convert the testname to only contain valid characters for
- a path
-
- This attempts to faithfully reconstruct the output of
- pigli-summary-html.testPathToHtmlFilename, but without the use of
- lambda. The advantage to not using lambda is readability, and the
- desire to make future changes to the way that sub-tests are written
- """
- valid_chars = frozenset("_-.%s%s" % (string.ascii_letters,
- string.digits))
- out = []
- for c in path:
- if c in valid_chars:
- out.append(c)
- elif c == "/":
- out.append("__")
- else:
- out.append("_")
- return ''.join(out)
-
class NewSummary:
"""
@@ -635,10 +611,17 @@ class NewSummary:
output_encoding = "utf-8",
module_directory = ".makotmp")
+ # Although these are only referenced once, that refernce is in a loop
+ # and they are less expensive to find once and store than find
+ # 10,000+ times
This is kind of a "You don't say" moment - not doing things in a loop is
of course faster. I don't think you need to explain it. If you'd like
to keep the comment, there's a typo: "reference"
+ resultCss = path.join(destination, "result.css")
+ index = path.join(destination, "index.html")
+
# Iterate across the tests creating the various test specific files
for each in self.results:
os.mkdir(path.join(destination, each.name))
+ # Create an index file for each test run
file = open(path.join(destination, each.name, "index.html"), 'w')
file.write(testindex.render(name = each.name,
time = each.time,
@@ -649,15 +632,32 @@ class NewSummary:
# Then build the individual test results
for key, value in each.tests.iteritems():
+
+ # This is also improves performance, by cutting the number of
+ # times path.join and path.dirname is by 3, that's 20,000+
+ # times!
This English is very broken. I'm not sure it needs explaining either.
+ tPath = path.join(destination, each.name, path.dirname(key))
+
+ # os.makedirs is very annoying, it throws an OSError if the
+ # path requested already exists, so do this check to esnure
+ # that it doesn't
+ if not path.exists(tPath):
+ os.makedirs(tPath)
If you want, you could use checkDir. But this is also fine.
"ensure" (typo)
+
file = open(path.join(destination,
each.name,
- self._sanitizePath(key + ".html")), 'w')
+ key + ".html"),
+ 'w')
file.write(testfile.render(testname = key,
status = value['result'],
returncode = value['returncode'],
time = value['time'],
info = value['info'],
- command = value['command']))
+ command = value['command'],
+ css = path.relpath(resultCss,
+ tPath),
+ index = index))
+
file.close()
I'd probably use "with open(...) as file:" syntax here, but that's just
me...it's your call.
# Finally build the root html files: index, regressions, etc
@@ -811,25 +811,3 @@ class NewSummary:
assert(len(currentStatus) == 1)
return counts, status
-
- def _sanitizePath(self, path):
- """
- Private: convert the testname to only contain valid characters for
- a path
-
- This attempts to faithfully reconstruct the output of
- pigli-summary-html.testPathToHtmlFilename, but without the use of
- lambda. The advantage to not using lambda is readability, and the
- desire to make future changes to the way that sub-tests are written
- """
- valid_chars = frozenset("_-.%s%s" % (string.ascii_letters,
- string.digits))
- out = []
- for c in path:
- if c in valid_chars:
- out.append(c)
- elif c == "/":
- out.append("__")
- else:
- out.append("_")
- return ''.join(out)
diff --git a/templates/test_result.mako b/templates/test_result.mako
index 1f944cb..7de0af7 100644
--- a/templates/test_result.mako
+++ b/templates/test_result.mako
@@ -5,7 +5,7 @@
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8"
/>
<title>${testname} - Details</title>
- <link rel="stylesheet" href="../result.css" type="text/css" />
+ <link rel="stylesheet" href="${css}" type="text/css" />
</head>
<body>
<h1>Results for ${testname}</h1>
@@ -14,7 +14,7 @@
<p><b>Status:</b> ${status}</p>
<p><b>Result:</b> ${status}</p>
</div>
- <p><a href="../index.html">Back to summary</a></p>
+ <p><a href="${index}">Back to summary</a></p>
<h2>Details</h2>
<table>
<tr>
@@ -42,6 +42,6 @@
</td>
</tr>
</table>
- <p><a href="../index.html">Back to summary</a></p>
+ <p><a href="${index}">Back to summary</a></p>
</body>
</html>
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit
