[Piglit] [PATCH 1/1] results.py: Print warning when overwriting subtest results

[Jan Vesely]

On Wed, 2014-09-03 at 19:33 -0400, Ilia Mirkin wrote:
> On Wed, Sep 3, 2014 at 7:16 PM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> > Signed-off-by: Jan Vesely <jan.vesely at rutgers.edu>
> > ---
> >
> > There is none reported for quick.py profile, and tons for cl.py
> >
> >  framework/results.py | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/framework/results.py b/framework/results.py
> > index ddadcc1..7d3a375 100644
> > --- a/framework/results.py
> > +++ b/framework/results.py
> > @@ -465,15 +465,17 @@ class TestResult(dict):
> >          dictionary -- a dictionary instance to update the TestResult with
> >
> >          """
> > -        def update(d, u):
> > +        def update(d, u, check):
> >              for k, v in u.iteritems():
> >                  if isinstance(v, dict):
> > -                    d[k] = update(d.get(k, {}), v)
> > +                    d[k] = update(d.get(k, {}), v, True)
> >                  else:
> > +                   if check and d.has_key(k):
> > +                       print("Warning: duplicate subtest: {} value: {} old value: {}".format(k, v, d[k]))
> 
> It would appear that you are mixing tabs and spaces. Don't do that,
> python can get mad at you. (Or gmail is messing up, in which case, my
> bad.)
> 
> Also, "k in d" is preferable to "d.has_key(k)".

thanks, I'll fix these

> 
> At a quick glance I don't understand the purpose of "check". Why can't
> it always just be true (and thus removed)?

that was my original implementation, but something sets the 'result' key
(probably constructor), so it would always report replacing it.

> 
> Printing stuff right there goes against the logging logic... I think
> TestResults are also used during test execution, not just at summary
> time... Dylan may have ideas on how to expose the issue properly.

These need to be detected at execution time otherwise only the last
result is stored in the results file and we loose information (and hide
possible failures).
if there is a better way to report these, I don;t mind changing it.

thanks,
jan

> 
> >                      d[k] = v
> >              return d
> >
> > -        update(self, dictionary)
> > +        update(self, dictionary, False)
> >
> >
> >  class TestrunResult(object):
> > --
> > 1.9.3
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit

-- 
Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140903/d9d727ee/attachment.sig>