<div dir="ltr">On 28 June 2013 06:49, Dylan Baker <span dir="ltr"><<a href="mailto:baker.dylan.c@gmail.com" target="_blank">baker.dylan.c@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Splits the code that finds problems, regressions, fixes, changes, and<br>
skips out of the constructor. Instead of incurring the overhead of<br>
calculating these every time whether they are needed or not the code can<br>
now be smarter about it's path, only generating the ones it needs, if<br>
any.<br>
<br>
Signed-off-by: Dylan Baker <<a href="mailto:baker.dylan.c@gmail.com">baker.dylan.c@gmail.com</a>><br></blockquote><div><br></div><div>I just noticed that this commit prevents new tests from showing up on the "changes" page.  In other words, if a test went from the "Not Run" state to some other state, that used to show up in "changes", but it doesn't anymore.<br>
<br></div><div>Was that an intentional change?  To me "changes" means "everything that's different from one test run to the next", so I'd prefer to see new tests show up in "changes".<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
---<br>
 framework/summary.py | 125 ++++++++++++++++++++++++++++-----------------------<br>
 1 file changed, 69 insertions(+), 56 deletions(-)<br>
<br>
diff --git a/framework/summary.py b/framework/summary.py<br>
index 5d4fe57..2f23686 100644<br>
--- a/framework/summary.py<br>
+++ b/framework/summary.py<br>
@@ -1,4 +1,3 @@<br>
-#<br>
 # Permission is hereby granted, free of charge, to any person<br>
 # obtaining a copy of this software and associated documentation<br>
 # files (the "Software"), to deal in the Software without<br>
@@ -487,7 +486,8 @@ class NewSummary:<br>
<br>
         The constructor of the summary class has an attribute for each HTML<br>
         summary page, which are fed into the index.mako file to produce HTML<br>
-        files<br>
+        files. resultfiles is a list of paths to JSON results generated by<br>
+        piglit-run.<br>
         """<br>
<br>
         def buildDictionary(summary):<br>
@@ -603,25 +603,6 @@ class NewSummary:<br>
<br>
             return counts, status<br>
<br>
-        def status_to_number(status):<br>
-            """<br>
-            small helper function to convert named statuses into number, since<br>
-            number can more easily be compared using logical/mathematical<br>
-            operators. The use of this is to look for regressions in status.<br>
-            """<br>
-            if status == 'pass':<br>
-                return 1<br>
-            elif status == 'warn':<br>
-                return 2<br>
-            elif status == 'fail':<br>
-                return 3<br>
-            elif status == 'skip':<br>
-                return 4<br>
-            elif status == 'crash':<br>
-                return 5<br>
-            elif status == 'special':<br>
-                return 0<br>
-<br>
         # Create a Result object for each piglit result and append it to the<br>
         # results list<br>
         self.results = [Result(i) for i in resultfiles]<br>
@@ -642,45 +623,75 @@ class NewSummary:<br>
             # Create a list with all the test names in it<br>
             self.tests['all'] = list(set(self.tests['all']) | set(each.tests))<br>
<br>
-        # Create lists similar to self.tests['all'], but for the other root<br>
-        # pages, (regressions, skips, ect)<br>
+    def __generate_lists(self, lists):<br>
+        """<br>
+        Private: Generate the lists of changes, problems, regressions, fixes,<br>
+        and skips<br>
+<br>
+        lists is a list contianing any of the following: changes, problems,<br>
+        skips, fixes (which will also generate regressions)<br>
+<br>
+        This method has different code paths to allow the exclusion of certain<br>
+        lists being generated. This is both useful for speeding up HTML<br>
+        generation when a page isn't needed (regressions with only one test<br>
+        file is provided), and for JUnit and text which only need a limited<br>
+        subset of these lists<br>
+        """<br>
+        def find_regressions(status):<br>
+            """<br>
+            Helper function to convert named statuses into number, since number can<br>
+            more easily be compared using logical/mathematical operators. The use of<br>
+            this is to look for regressions in status.<br>
+            """<br>
+            if status == 'pass':<br>
+                return 1<br>
+            elif status == 'warn':<br>
+                return 2<br>
+            elif status == 'fail':<br>
+                return 3<br>
+            elif status == 'skip':<br>
+                return 4<br>
+            elif status == 'crash':<br>
+                return 5<br>
+            elif status == 'special':<br>
+                return 0<br>
+<br>
         for test in self.tests['all']:<br>
             status = []<br>
             for each in self.results:<br>
                 try:<br>
-                    status.append(status_to_number(each.tests[test]['result']))<br>
+                    status.append(find_regressions(each.tests[test]['result']))<br>
                 except KeyError:<br>
-                    status.append(status_to_number("special"))<br>
-<br>
-            # Check and append self.tests['changes']<br>
-            # A set cannot contain duplicate entries, so creating a set out<br>
-            # the list will reduce it's length to 1 if all entries are the<br>
-            # same, meaning it is not a change<br>
-            if len(set(status)) > 1:<br>
-                self.tests['changes'].append(test)<br>
-<br>
-            # Problems<br>
-            # If the result contains a value other than 1 (pass) or 4 (skip)<br>
-            # it is a problem. Skips are not problems becasuse they have<br>
-            # Their own page.<br>
-            if [i for e in [2, 3, 5] for i in status if e is i]:<br>
-                self.tests['problems'].append(test)<br>
-<br>
-            # skipped<br>
-            if 4 in status:<br>
-                self.tests['skipped'].append(test)<br>
-<br>
-            # fixes and regressions<br>
-            # check each member against the next member. If the second member<br>
-            # has a greater value it is a regression, unless the first value i<br>
-            # 0, which means it cannot be compared<br>
-            # Fixes on the other hand are a former non 1 value, followed by<br>
-            # a value of 1<br>
-            for i in xrange(len(status) - 1):<br>
-                if status[i] < status[i + 1] and status[i] != 0:<br>
-                    self.tests['regressions'].append(test)<br>
-                if status[i] > 1 and status[i + 1] == 1:<br>
-                    self.tests['fixes'].append(test)<br>
+                    status.append(find_regressions("special"))<br>
+<br>
+                if 'changes' in lists:<br>
+                    # Check and append self.tests['changes']<br>
+                    # A set cannot contain duplicate entries, so creating a set<br>
+                    # out the list will reduce it's length to 1 if all entries<br>
+                    # are the same, meaning it is not a change<br>
+                    if len(set(status)) > 1 and not 0 in status:<br>
+                        self.tests['changes'].append(test)<br>
+<br>
+                if 'problems' in lists:<br>
+                    # If the result contains a value other than 1 (pass) or 4<br>
+                    # (skip) it is a problem. Skips are not problems becasuse<br>
+                    # they have Their own page.<br>
+                    if [i for e in [2, 3, 5] for i in status if e is i]:<br>
+                        self.tests['problems'].append(test)<br>
+<br>
+                if 'skipped' in lists:<br>
+                    # Find all tests with a status of skip<br>
+                    if 4 in status:<br>
+                        self.tests['skipped'].append(test)<br>
+<br>
+                if 'fixes' in lists:<br>
+                    # Find both fixes and regressions, and append them to the<br>
+                    # proper lists<br>
+                    for i in xrange(len(status) - 1):<br>
+                        if status[i] < status[i + 1] and status[i] != 0:<br>
+                            self.tests['regressions'].append(test)<br>
+                        if status[i] > 1 and status[i + 1] == 1:<br>
+                            self.tests['fixes'].append(test)<br>
<br>
     def generateHTML(self, destination, exclude):<br>
         """<br>
@@ -762,12 +773,14 @@ class NewSummary:<br>
<br>
         # A list of pages to be generated<br>
         # If there is only one set of results, then there cannot be changes,<br>
-        # regressions or fixes, so don't generate those pages<br>
+        # regressions or fixes, so don't generate those pages.<br>
         if len(self.results) > 1:<br>
             pages = ['changes', 'problems', 'skipped', 'fixes', 'regressions']<br>
         else:<br>
             pages = ['problems', 'skipped']<br>
<br>
+        self.__generate_lists(pages)<br>
+<br>
         # Index.html is a bit of a special case since there is index, all, and<br>
         # alltests, where the other pages all use the same name. ie,<br>
         # changes.html, self.changes, and page=changes.<br>
<span class="HOEnZb"><font color="#888888">--<br>
1.8.1.4<br>
<br>
_______________________________________________<br>
Piglit mailing list<br>
<a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a><br>
</font></span></blockquote></div><br></div></div>