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?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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/piglit _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
