[Piglit] [PATCH 2/4] summary, py: Refactor HTMLIndex class into generator function

Dylan Baker baker.dylan.c at gmail.com
Mon Nov 3 17:05:04 PST 2014


This is an ugly class that should have been a function from the start.
This patch uses a generator, and at the same time reduces the complexity
of both the template and the python function, leaving cleaner, easier to
read code.

Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
---
 framework/summary.py | 412 +++++++++++++++++++++++++--------------------------
 templates/index.mako |  36 +----
 2 files changed, 202 insertions(+), 246 deletions(-)

diff --git a/framework/summary.py b/framework/summary.py
index f4fd80d..0dbeb82 100644
--- a/framework/summary.py
+++ b/framework/summary.py
@@ -18,7 +18,7 @@
 # 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.
- 
+
 from __future__ import print_function
 import os
 import os.path as path
@@ -30,6 +30,8 @@ import tempfile
 import datetime
 import re
 import getpass
+import textwrap
+
 from mako.template import Template
 
 # a local variable status exists, prevent accidental overloading by renaming
@@ -58,215 +60,202 @@ def normalize_href(href):
     return href.replace('\\', '/')
 
 
-class HTMLIndex(list):
-    """
-    Builds HTML output to be passed to the index mako template, which will be
-    rendered into HTML pages. It does this by parsing the lists provided by the
-    Summary object, and returns itself, an object with one accessor, a list of
-    html strings that will be printed by the mako template.
-    """
-
-    def __init__(self, summary, page):
-        """
-        Steps through the list of groups and tests from all of the results and
-        generates a list of dicts that are passed to mako and turned into HTML
-        """
-
-        def returnList(open, close):
-            """
-            As HTMLIndex iterates through the groups and tests it uses this
-            function to determine which groups to close (and thus reduce the
-            depth of the next write) and which ones to open (thus increasing
-            the depth)
-
-            To that end one of two things happens, the path to the previous
-            group (close) and the next group (open) are equal, in that event we
-            don't want to open and close, becasue that will result in a
-            sawtooth pattern of a group with one test followed by the same
-            group with one test, over and over.  Instead we simply return two
-            empty lists, which will result in writing a long list of test
-            results.  The second option is that the paths are different, and
-            the function determines any commonality between the paths, and
-            returns the differences as close (the groups which are completly
-            written) and open (the new groups to write).
-            """
-            common = []
-
-            # Open and close are lists, representing the group hierarchy, open
-            # being the groups that need are soon to be written, and close
-            # representing the groups that have finished writing.
-            if open == close:
-                return [], []
-            else:
-                for i, j in itertools.izip_longest(open, close):
-                    if i != j:
-                        for k in common:
-                            open.remove(k)
-                            close.remove(k)
-                        return open, close
-                    else:
-                        common.append(i)
-
-        # set a starting depth of 1, 0 is used for 'all' so 1 is the
-        # next available group
-        depth = 1
-
-        # Current dir is a list representing the groups currently being
-        # written.
-        currentDir = []
-
-        # Add a new 'tab' for each result
-        self._newRow()
-        self.append({'type': 'other', 'text': '<td />'})
-        for each in summary.results:
-            href = posixpath.join(escape_pathname(each.name), "index.html")
-            href = normalize_href(href)
-            self.append({'type': 'other',
-                         'text': '<td class="head"><b>%(name)s</b><br />'
-                                 '(<a href="%(href)s">info</a>)'
-                                 '</td>' % {'name': each.name,
-                                            'href': href}})
-        self._endRow()
-
-        # Add the toplevel 'all' group
-        self._newRow()
-        self._groupRow("head", 0, 'all')
-        for each in summary.results:
-            self._groupResult(summary.fractions[each.name]['all'],
-                              summary.status[each.name]['all'])
-        self._endRow()
-
-        # Add the groups and tests to the out list
-        for key in sorted(page):
-
-            # Split the group names and test names, then determine
-            # which groups to close and which to open
-            openList = key.replace('\\', '/').split('/')
-            test = openList.pop()
-            openList, closeList = returnList(openList, list(currentDir))
-
-            # Close any groups in the close list
-            # for each group closed, reduce the depth by one
-            for i in reversed(closeList):
-                currentDir.remove(i)
-                depth -= 1
-
-            # Open new groups
-            for localGroup in openList:
-                self._newRow()
-
-                # Add the left-most column: the name of the group
-                self._groupRow("head", depth, localGroup)
-
-                # Add the group that we just opened to the currentDir, which
-                # will then be used to add that group to the HTML list. If
-                # there is a KeyError (the group doesn't exist), use (0, 0)
-                # which will get skip. This sets the group coloring correctly
-                currentDir.append(localGroup)
-                for each in summary.results:
-                    # Decide which fields need to be updated
-                    self._groupResult(
-                        summary.fractions[each.name][path.join(*currentDir)],
-                        summary.status[each.name][path.join(*currentDir)])
-
-                # After each group increase the depth by one
-                depth += 1
-                self._endRow()
-
-            # Add the tests for the current group
-            self._newRow()
-
-            # Add the left-most column: the name of the test
-            self._testRow("group", depth, test)
-
-            # Add the result from each test result to the HTML summary If there
-            # is a KeyError (a result doesn't contain a particular test),
-            # return Not Run, with clas skip for highlighting
-            for each in summary.results:
-                # If the "group" at the top of the key heirachy contains
-                # 'subtest' then it is really not a group, link to that page
-                try:
-                    if each.tests[path.dirname(key)]['subtest']:
-                        href = path.dirname(key)
-                except KeyError:
-                    href = key
-
-                href = escape_filename(href)
-
-                try:
-                    self._testResult(escape_pathname(each.name), href,
-                                     summary.status[each.name][key])
-                except KeyError:
-                    self.append({'type': 'other',
-                                 'text': '<td class="skip">Not Run</td>'})
-            self._endRow()
-
-    def _newRow(self):
-        self.append({'type': 'newRow'})
+def _group_row(css, indent, text):
+    """Write a new group row."""
+
+    template = Template(textwrap.dedent("""\
+    <td>
+      <div class="${css}" style="margin-left: ${indent}em">
+        <b>${text}</b>
+      </div>
+    </td>
+    """))
+
+    return template.render(css=css, indent=(indent * 1.75), text=text)
+
+
+def _test_row(css, indent, text):
+    """Start a new row of tests."""
+    template = Template(textwrap.dedent("""\
+    <td>
+      <div class="${css}" style="margin-left: ${indent}em">
+        ${text}
+      </div>
+    </td>
+    """))
+
+    return template.render(css=css, indent=(indent * 1.75), text=text)
+
+
+def _group_result(css, text):
+    """Write a group result entry."""
+    template = Template(textwrap.dedent("""\
+    <td class="${css}">
+      <b>${text}</b>
+    </td>
+    """))
+
+    if css is so.NOTRUN:
+        css = 'skip'
+
+    return template.render(css=css, text='/'.join(str(t) for t in text))
+
+
+def _test_result(href, group, text, exclude):
+    """Write a new test entry."""
+    template = Template(textwrap.dedent("""\
+    <td class="${css}">
+    % if href is not None:
+      <a href="${href}">${text}</a>
+    % else:
+      ${text}
+    % endif
+    </td>
+    """))
+
+    if text is so.NOTRUN:
+        css = 'skip'
+        href = None
+    else:
+        css = text
+        if css in exclude:
+            href = None
+        else:
+            href = normalize_href(posixpath.join(group, href + '.html'))
 
-    def _endRow(self):
-        self.append({'type': 'endRow'})
+    return template.render(css=css, text=text, href=href)
 
-    def _groupRow(self, cssclass, depth, groupname):
-        """
-        Helper function for appending new groups to be written out
-        in HTML.
 
-        This particular function is used to write the left most
-        column of the summary. (the one with the indents)
-        """
-        self.append({'type': "groupRow",
-                     'class': cssclass,
-                     'indent': (1.75 * depth),
-                     'text': groupname})
+def html_generator(summary, page, exclude):
+    """Generator that returns contents of HTML file."""
+    new_row = '<tr>'
+    end_row = '</tr>'
 
-    def _groupResult(self, value, css):
+    def returnList(open, close):
         """
-        Helper function for appending the results of groups to the
-        HTML summary file.
+        As HTMLIndex iterates through the groups and tests it uses this
+        function to determine which groups to close (and thus reduce the
+        depth of the next write) and which ones to open (thus increasing
+        the depth)
+
+        To that end one of two things happens, the path to the previous
+        group (close) and the next group (open) are equal, in that event we
+        don't want to open and close, becasue that will result in a
+        sawtooth pattern of a group with one test followed by the same
+        group with one test, over and over.  Instead we simply return two
+        empty lists, which will result in writing a long list of test
+        results.  The second option is that the paths are different, and
+        the function determines any commonality between the paths, and
+        returns the differences as close (the groups which are completly
+        written) and open (the new groups to write).
         """
-        # "Not Run" is not a valid css class replace it with skip
-        if css == so.NOTRUN:
-            css = 'skip'
-
-        self.append({'type': "groupResult",
-                     'class': css,
-                     'text': "%s/%s" % (value[0], value[1])})
+        common = []
 
-    def _testRow(self, cssclass, depth, groupname):
-        """
-        Helper function for appending new tests to be written out
-        in HTML.
+        # Open and close are lists, representing the group hierarchy, open
+        # being the groups that need are soon to be written, and close
+        # representing the groups that have finished writing.
+        if open == close:
+            return [], []
+        else:
+            for i, j in itertools.izip_longest(open, close):
+                if i != j:
+                    for k in common:
+                        open.remove(k)
+                        close.remove(k)
+                    return open, close
+                else:
+                    common.append(i)
+
+    # set a starting depth of 1, 0 is used for 'all' so 1 is the
+    # next available group
+    depth = 1
+
+    # Current dir is a list representing the groups currently being
+    # written.
+    currentDir = []
+
+    # Add a new 'tab' for each result
+    yield new_row
+    yield '<td />'
+    for each in summary.results:
+        href = normalize_href(
+            posixpath.join(escape_pathname(each.name), "index.html"))
+        yield ('<td class="head"><b>{0}</b><br />'
+               '(<a href="{1}">info</a>)'
+               '</td>'.format(each.name, href))
+    yield end_row
+
+    # Add the toplevel 'all' group
+    yield new_row
+    yield _group_row("head", 0, 'all')
+    for each in summary.results:
+        yield _group_result(summary.status[each.name]['all'],
+                            summary.fractions[each.name]['all'])
+    yield end_row
+
+    # Add the groups and tests to the out list
+    for key in sorted(page):
+        # Split the group names and test names, then determine
+        # which groups to close and which to open
+        openList = key.replace('\\', '/').split('/')
+        test = openList.pop()
+        openList, closeList = returnList(openList, list(currentDir))
+
+        # Close any groups in the close list
+        # for each group closed, reduce the depth by one
+        for i in reversed(closeList):
+            currentDir.remove(i)
+            depth -= 1
+
+        # Open new groups
+        for localGroup in openList:
+            yield new_row
+
+            # Add the left-most column: the name of the group
+            yield _group_row("head", depth, localGroup)
+
+            # Add the group that we just opened to the currentDir, which
+            # will then be used to add that group to the HTML list. If
+            # there is a KeyError (the group doesn't exist), use (0, 0)
+            # which will get skip. This sets the group coloring correctly
+            currentDir.append(localGroup)
+            for each in summary.results:
+                # Decide which fields need to be updated
+                yield _group_result(
+                    summary.status[each.name][path.join(*currentDir)],
+                    summary.fractions[each.name][path.join(*currentDir)])
 
-        This particular function is used to write the left most
-        column of the summary. (the one with the indents)
-        """
-        self.append({'type': "testRow",
-                     'class': cssclass,
-                     'indent': (1.75 * depth),
-                     'text': groupname})
+            # After each group increase the depth by one
+            depth += 1
+            yield end_row
 
-    def _testResult(self, group, href, text):
-        """
-        Helper function for writing the results of tests
+        # Add the tests for the current group
+        yield new_row
 
-        This function writes the cells other than the left-most cell,
-        displaying pass/fail/crash/etc and formatting the cell to the
-        correct color.
-        """
-        # "Not Run" is not a valid class, if it apears set the class to skip
-        if text == so.NOTRUN:
-            css = 'skip'
-            href = None
-        else:
-            css = text
-            href = posixpath.join(group, href + ".html")
-            href = normalize_href(href)
+        # Add the left-most column: the name of the test
+        yield _test_row("group", depth, test)
 
-        self.append({'type': 'testResult',
-                     'class': css,
-                     'href': href,
-                     'text': text})
+        # Add the result from each test result to the HTML summary If there
+        # is a KeyError (a result doesn't contain a particular test),
+        # return Not Run, with clas skip for highlighting
+        for each in summary.results:
+            # If the "group" at the top of the key heirachy contains
+            # 'subtest' then it is really not a group, link to that page
+            try:
+                if each.tests[path.dirname(key)]['subtest']:
+                    href = path.dirname(key)
+            except KeyError:
+                href = key
+            href = escape_filename(href)
+
+            try:
+                yield _test_result(href,
+                                   escape_pathname(each.name),
+                                   summary.status[each.name][key],
+                                   exclude)
+            except KeyError:
+                yield '<td class="skip">Not Run</td>'
+        yield end_row
 
 
 class Summary:
@@ -518,23 +507,24 @@ class Summary:
         # alltests, where the other pages all use the same name. ie,
         # changes.html, self.changes, and page=changes.
         with open(path.join(destination, "index.html"), 'w') as out:
-            out.write(index.render(results=HTMLIndex(self, self.tests['all']),
-                                   page='all',
-                                   pages=pages,
-                                   colnum=len(self.results),
-                                   exclude=exclude))
+            out.write(index.render(
+                results=html_generator(self, self.tests['all'], exclude),
+                page='all',
+                pages=pages,
+                colnum=len(self.results),
+                exclude=exclude))
 
         # Generate the rest of the pages
         for page in pages:
             with open(path.join(destination, page + '.html'), 'w') as out:
-            # If there is information to display display it
+                # If there is information to display display it
                 if self.tests[page]:
-                    out.write(index.render(results=HTMLIndex(self,
-                                                             self.tests[page]),
-                                           pages=pages,
-                                           page=page,
-                                           colnum=len(self.results),
-                                           exclude=exclude))
+                    out.write(index.render(
+                        results=html_generator(self, self.tests[page], exclude),
+                        pages=pages,
+                        page=page,
+                        colnum=len(self.results),
+                        exclude=exclude))
                 # otherwise provide an empty page
                 else:
                     out.write(empty_status.render(page=page, pages=pages))
diff --git a/templates/index.mako b/templates/index.mako
index 5a202cc..dfc2abc 100644
--- a/templates/index.mako
+++ b/templates/index.mako
@@ -36,41 +36,7 @@
         % endfor
       </colgroup>
       % for line in results:
-        % if line['type'] == "newRow":
-        <tr>
-        % elif line['type'] == "endRow":
-        </tr>
-        % elif line['type'] == "groupRow":
-          <td>
-            <div class="${line['class']}" style="margin-left: ${line['indent']}em">
-              <b>${line['text']}</b>
-            </div>
-          </td>
-        % elif line['type'] == "testRow":
-          <td>
-            <div class="${line['class']}" style="margin-left: ${line['indent']}em">
-              ${line['text']}
-            </div>
-          </td>
-        % elif line['type'] == "groupResult":
-          <td class="${line['class']}">
-            <b>${line['text']}</b>
-          </td>
-        % elif line['type'] == "testResult":
-          <td class="${line['class']}">
-          ## If the result is in the excluded results page list from
-          ## argparse, just print the text, otherwise add the link
-          % if line['class'] not in exclude and line['href'] is not None:
-            <a href="${line['href']}">
-              ${line['text']}
-            </a>
-          % else:
-            ${line['text']}
-          % endif
-          </td>
-        % elif line['type'] == "other":
-          ${line['text']}
-        % endif
+        ${line}
       % endfor
     </table>
   </body>
-- 
2.1.3



More information about the Piglit mailing list