[Piglit] [PATCH v2 4/6] framework/test/base.py: split protected method in Test

[baker.dylan.c at gmail.com]

From: Dylan Baker <baker.dylan.c at gmail.com>

This is groundwork to split timeout into a Mixin, which is going to be a
cleaner solution than trying to build timeout into the base class.

The reason this is going to be cleaner is that for python 2.x it is much
simpler and cleaner to use subprocess32 with it's backported timeout
feature. This only works on *nix. Even if one wanted to implement
windows support on python 3.3+ (which is coming eventually) there will
be numerous implementation differences to account for the difference in
the way windows and *nix handle processes.

I'm not 100% happy with the API, but even with more extensive splits and
rewrites I couldn't come up with anything cleaner that didn't result in
a lot of duplication. I did come up with some cleaner approaches, but
they resulted in even more duplication than this requires, since the
duplication in this patch will be thrown out in subsequent patches.

Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
---
 framework/test/base.py | 51 +++++++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/framework/test/base.py b/framework/test/base.py
index bf00396..c111f13 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -259,13 +259,8 @@ class Test(object):
         if hasattr(os, 'setpgrp'):
             os.setpgrp()
 
-    def _run_command(self):
-        """ Run the test command and get the result
-
-        This method sets environment options, then runs the executable. If the
-        executable isn't found it sets the result to skip.
-
-        """
+    def _make_popen_args(self):
+        """Create arguemnts for popen."""
         # Setup the environment for the test. Environment variables are taken
         # from the following sources, listed in order of increasing precedence:
         #
@@ -279,26 +274,36 @@ class Test(object):
         # Piglit considers environment variables set in all.py (3) to be test
         # requirements.
         #
-        fullenv = dict()
-        for key, value in itertools.chain(os.environ.iteritems(),
-                                          self.OPTS.env.iteritems(),
-                                          self.env.iteritems()):
-            fullenv[key] = str(value)
+        popen_args = {
+            'env': {k: str(v) for k, v in itertools.chain(
+                os.environ.iteritems(),
+                self.OPTS.env.iteritems(),
+                self.env.iteritems())},
+            'stdout': subprocess.PIPE,
+            'stderr': subprocess.PIPE,
+            'cwd': self.cwd,
+            'universal_newlines': True,
+        }
 
         # preexec_fn is not supported on Windows platforms
-        if sys.platform == 'win32':
-            preexec_fn = None
-        else:
-            preexec_fn = self.__set_process_group
+        if sys.platform != 'win32':
+            popen_args['preexec_fn'] = self.__set_process_group
 
+        return popen_args
+
+    def _run_command(self, communicate_args=None):
+        """ Run the test command and get the result
+
+        This method sets environment options, then runs the executable. If the
+        executable isn't found it sets the result to skip.
+
+        This will populate the following popen arguments automatically: stdout,
+        stderr, cwd, universal newlines.
+
+        """
         try:
             proc = subprocess.Popen(self.command,
-                                    stdout=subprocess.PIPE,
-                                    stderr=subprocess.PIPE,
-                                    cwd=self.cwd,
-                                    env=fullenv,
-                                    universal_newlines=True,
-                                    preexec_fn=preexec_fn)
+                                    **self._make_popen_args)
             # create a ProcessTimeout object to watch out for test hang if the
             # process is still going after the timeout, then it will be killed
             # forcing the communicate function (which is a blocking call) to
@@ -307,7 +312,7 @@ class Test(object):
                 self.__proc_timeout = ProcessTimeout(self.timeout, proc)
                 self.__proc_timeout.start()
 
-            out, err = proc.communicate()
+            out, err = proc.communicate(**(communicate_args or {}))
             returncode = proc.returncode
         except OSError as e:
             # Different sets of tests get built under different build
-- 
2.6.1