[Piglit] [V2 PATCH 04/12] summary.py: Split functionality out of NewSummary constructor
Dylan Baker
baker.dylan.c at gmail.com
Fri Jul 12 13:33:13 PDT 2013
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 <stereotype441 at gmail.com> wrote:
> On 28 June 2013 06:49, Dylan Baker <baker.dylan.c at 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.dylan.c at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130712/a56f819b/attachment-0001.html>
More information about the Piglit
mailing list