That was intentional. My logic was this: 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>)
In the first case I can accept the argument that a new test is in fact a change. 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. I decided that the latter case outweighed the former. Obviously if I erred in that decision I can make changes. On Fri, Jul 12, 2013 at 1:17 PM, Paul Berry <stereotype...@gmail.com> wrote: > On 28 June 2013 06:49, Dylan Baker <baker.dyla...@gmail.com> wrote: > >> 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 <baker.dyla...@gmail.com> >> > > 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. > > 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". > > >> --- >> 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 >> Piglit@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/piglit >> > >
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit