On Tuesday, April 15, 2014 19:37:48 Ilia Mirkin wrote:
> On Tue, Apr 15, 2014 at 7:12 PM, Dylan Baker
<[email protected]> 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 <[email protected]>
> > ---
> >
> >  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?idh0214 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
> > [email protected]
> > http://lists.freedesktop.org/mailman/listinfo/piglit

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to