<div dir="ltr">That was intentional.<div><br></div><div style>My logic was this:</div><div style>a "Not Run" can mean two different things: 1) the test didn't exist during one run, 2) one set of results is much more comprehensive than another (say quick.tests vs quick.tests -t <only run a few tests>)</div>
<div style><br></div><div style>In the first case I can accept the argument that a new test is in fact a change.</div><div style>In the latter case though the results would have 10000+ changes and regressions or fixes (depending on which order they were passed to summary-html). In this case these "Not Run" are actually more of "not known", and calling them a change is dangerous, as they might in fact not be changes.</div>
<div style><br></div><div style>I decided that the latter case outweighed the former. Obviously if I erred in that decision I can make changes.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jul 12, 2013 at 1:17 PM, Paul Berry <span dir="ltr"><<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="im">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><div class="gmail_extra"><div class="gmail_quote"><div class="im">
<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" target="_blank">baker.dylan.c@gmail.com</a>><br></blockquote><div><br></div></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"><div><div class="h5">
---<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>
</div></div><span class="HOEnZb"><font color="#888888"><span><font color="#888888">--<br>
1.8.1.4<br>
<br>
_______________________________________________<br>
Piglit mailing list<br>
<a href="mailto:Piglit@lists.freedesktop.org" target="_blank">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></font></span></blockquote></div><br></div></div>
</blockquote></div><br></div>