[Piglit] [V2 PATCH 04/12] summary.py: Split functionality out of NewSummary constructor
Paul Berry
stereotype441 at gmail.com
Fri Jul 12 13:17:30 PDT 2013
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/a1be948c/attachment.html>
More information about the Piglit
mailing list