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