[Piglit] [PATCH 2/2] results.py: Add update_recursive method to TestResult

[Ilia Mirkin]

On Wed, Jul 2, 2014 at 4:02 AM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> This method is used during PiglitTest.interpret_result() to update
> subtest entries safely. I chose this approach since it is robust,
> reusable, and it felt more natural as a part of TestResult than putting
> it in PiglitTest.interpret_result()
>
> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>

Subject talks about update_recursive, but the method is called
recursive_update. Seems easier to just fix the subject, but I don't
really care which way you go as long as they line up.

> ---
>  framework/exectest.py |  2 +-
>  framework/results.py  | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/framework/exectest.py b/framework/exectest.py
> index e4a2344..fcc29af 100644
> --- a/framework/exectest.py
> +++ b/framework/exectest.py
> @@ -279,6 +279,6 @@ class PiglitTest(Test):
>          outpiglit = (s[7:] for s in outlines if s.startswith('PIGLIT:'))
>
>          for piglit in outpiglit:
> -            self.result.update(json.loads(piglit))
> +            self.result.recursive_update(json.loads(piglit))
>          self.result['out'] = '\n'.join(
>              s for s in outlines if not s.startswith('PIGLIT:'))
> diff --git a/framework/results.py b/framework/results.py
> index a715b29..88d962d 100644
> --- a/framework/results.py
> +++ b/framework/results.py
> @@ -248,6 +248,38 @@ class TestResult(dict):
>              # normally
>              pass
>
> +    def recursive_update(self, dictionary):
> +        """ Recursively update the TestResult
> +
> +        The problem with using self.update() is this:
> +        >>> t = TestResult()
> +        >>> t.update({'subtest': {'test1': 'pass'}})
> +        >>> t.update({'subtest': {'test2': 'pass'}})
> +        >>> t['subtest']
> +        {'test2': 'pass'}
> +
> +        This function is different, because it recursively updates self, it
> +        doesn't clobber existing entires in the same way
> +        >>> t = TestResult()
> +        >>> t.recursive_update({'subtest': {'test1': 'pass'}})
> +        >>> t.recursive_update({'subtest': {'test2': 'pass'}})
> +        >>> t['subtest']
> +        {'test1': 'pass', 'test2': 'pass'}
> +
> +        Arguments:
> +        dictionary -- a dictionary instance to update the TestResult with
> +
> +        """
> +        def update(d, u):
> +            for k, v in u.iteritems():
> +                if isinstance(v, dict):
> +                    d[k] = update(d.get(k, {}), v)
> +                else:
> +                    d[k] = u[k]

Why not

d[k] = v

With that fixed, series is

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>

Thanks for the quick fix!

> +            return d
> +
> +        update(self, dictionary)
> +
>
>  class TestrunResult(object):
>      def __init__(self, resultfile=None):
> --
> 2.0.0
>