[Piglit] [PATCH 05/13] framework/exectest.py: Don't pass redundant args to Test.Get_command_result()

[Dylan Baker]

On Tuesday, April 15, 2014 19:37:48 Ilia Mirkin wrote:
> On Tue, Apr 15, 2014 at 7:12 PM, Dylan Baker 
<baker.dylan.c at gmail.com> wrote:
> > This puts fullenv in Test.get_command_result() (since that's the only
> > place it's actually used). Also don't pass self.command as an 
argument
> > (with an additional assignment).
> > 
> > Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> > ---
> > 
> >  framework/exectest.py | 16 ++++++----------
> >  1 file changed, 6 insertions(+), 10 deletions(-)
> > 
> > diff --git a/framework/exectest.py b/framework/exectest.py
> > index 3bbcabb..3bbaf1f 100644
> > --- a/framework/exectest.py
> > +++ b/framework/exectest.py
> > 
> > @@ -141,13 +141,7 @@ class Test(object):
> >          * For 'returncode', the value will be the numeric exit
> >          code/value.
> >          * For 'command', the value will be command line program and
> >          arguments.
> >          """
> > 
> > -        fullenv = os.environ.copy()
> > -        for e in self.env:
> > -            fullenv[e] = str(self.env[e])
> > -
> > 
> >          if self.command is not None:
> Hmmm... this test implies that the previous patch is wrong for the
> valgrind case? I guess it should only return the valgrinded command if
> self._command is not None as well.
> 
> > -            command = self.command
> > -
> > 
> >              i = 0
> >              skip = self.check_for_skip_scenario()
> > 
> >              while True:
> > @@ -156,8 +150,7 @@ class Test(object):
> >                      err = ""
> >                      returncode = None
> >                  
> >                  else:
> > -                    out, err, returncode =
> > self.get_command_result(command, -                                       
> >                            fullenv) +                    out, err,
> > returncode = self.get_command_result()> 
> >                  # https://bugzilla.gnome.org/show_bug.cgi?id=680214 is
> >                  # affecting many developers.  If we catch it
> > 
> > @@ -236,9 +229,12 @@ class Test(object):
> >          """
> >          return False
> > 
> > -    def get_command_result(self, command, fullenv):
> > +    def get_command_result(self):
> > +        fullenv = os.environ.copy()
> > +        fullenv.update(dict((k, str(v)) for k, v in
> > self.env.iteritems()))
> 
> IMHO the previous way of doing this is way more readable. And
> potentially more efficient if you merge the two approaches and use
> iteritems on it as well instead of the (implicit) iterkeys.

OK, I'll change it to a loop.
I have a background in Ruby, so I find python's lack of one-liners annoying.

> 
> > +
> > 
> >          try:
> > -            proc = subprocess.Popen(command,
> > +            proc = subprocess.Popen(self.command,
> > 
> >                                      stdout=subprocess.PIPE,
> >                                      stderr=subprocess.PIPE,
> >                                      env=fullenv,
> > 
> > --
> > 1.9.2
> > 
> > _______________________________________________
> > 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/20140416/1af183d8/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140416/1af183d8/attachment-0001.sig>