[Piglit] [PATCH 1/2] framework/results.py: Copy 'tests' internally to OrderedDict

[Dylan Baker]

First, thanks for doing this it's a nice improvement.

I have a few comments below, and a few on the next patch as well.

Quoting Tomi Sarvela (2017-02-09 03:07:04)
> To preserve the original test running order, use OrderedDict
> when copying 'tests' values over.
> ---
>  framework/results.py | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/framework/results.py b/framework/results.py
> index 1a73c11..45a5727 100644
> --- a/framework/results.py
> +++ b/framework/results.py
> @@ -24,7 +24,9 @@
>  from __future__ import (
>      absolute_import, division, print_function, unicode_literals
>  )
> -import collections

First, this is an unrelated change since we already have collections imported,
and creates a lot of churn in the patch. Second, I don't think this is an
improvement, since it puts more things in the root namespace which are not local
functions or frequently used.

> +from collections import defaultdict
> +from collections import OrderedDict
> +from collections import MutableMapping
>  import copy
>  import datetime
>  
> @@ -38,7 +40,7 @@ __all__ = [
>  ]
>  
>  
> -class Subtests(collections.MutableMapping):
> +class Subtests(MutableMapping):
>      """A dict-like object that stores Statuses as values."""
>      def __init__(self, dict_=None):
>          self.__container = {}
> @@ -301,8 +303,8 @@ class TestrunResult(object):
>          self.clinfo = None
>          self.lspci = None
>          self.time_elapsed = TimeAttribute()
> -        self.tests = collections.OrderedDict()
> -        self.totals = collections.defaultdict(Totals)
> +        self.tests = OrderedDict()
> +        self.totals = defaultdict(Totals)
>  
>      def get_result(self, key):
>          """Get the result of a test or subtest.
> @@ -350,7 +352,7 @@ class TestrunResult(object):
>          if not self.totals:
>              self.calculate_group_totals()
>          rep = copy.copy(self.__dict__)
> -        rep['tests'] = {k: t.to_json() for k, t in six.iteritems(self.tests)}
> +        rep['tests'] = OrderedDict([ ( k, t.to_json() ) for k, t in six.iteritems(self.tests)] )

This is not an "OrderedDict comprehension", what this actually does is create a
list in memeory, and then copy that into an OrderedDict. It should be sufficient
to do:
OrderedDict(k, t.to_json() for k, t in six.iteritems(self.tests))

Please also note that we use PEP8 style in the python portion of the code, there
are no spaces around braces or parens.

>          rep['__type__'] = 'TestrunResult'
>          return rep
>  
> @@ -378,8 +380,8 @@ class TestrunResult(object):
>          if 'time_elapsed' in dict_:
>              setattr(res, 'time_elapsed',
>                      TimeAttribute.from_dict(dict_['time_elapsed']))
> -        res.tests = {n: TestResult.from_dict(t)
> -                     for n, t in six.iteritems(dict_['tests'])}
> +        res.tests = OrderedDict([ (n, TestResult.from_dict(t))
> +                     for n, t in six.iteritems(dict_['tests']) ])

The same here as above

>  
>          if not 'totals' in dict_ and not _no_totals:
>              res.calculate_group_totals()
> -- 
> 2.9.3
> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20170209/ad85b4c6/attachment.sig>