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

[Ilia Mirkin]

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.

> +
>          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