[Piglit] [Patch V2 3/6] framework.environment.py: Replace Environment.collectData static method

[Dylan Baker]

First, static methods are ugly, and python's namespaces make them even
less interesting, since a toplevel function is like a static method but
without being part of a class. Second this static method was even worse
since it had nothing to do with the class it was attached.

This is replaced with a toplevel function in the environment module.
This also removed the need for checking what OS piglit is running on,
since it blindly attempts to run all of the possible commands that we
might want using a try/except block to skip the ones that are not
returned.

V2: - Return and iterator from Environment.get_environment() instead of
      a list. This should be more efficient if we add more binaries.

Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
---
 framework/environment.py | 57 +++++++++++++++++++++++++++++-------------------
 piglit-run.py            |  3 +--
 2 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/framework/environment.py b/framework/environment.py
index d795de6..c991622 100644
--- a/framework/environment.py
+++ b/framework/environment.py
@@ -20,10 +20,10 @@ import os
 import os.path as path
 import re
 import subprocess
-import platform
 
 __all__ = ['Environment',
-           'TEST_BIN_DIR']
+           'TEST_BIN_DIR',
+           'get_environment']
 
 if 'PIGLIT_BUILD_DIR' in os.environ:
     TEST_BIN_DIR = path.join(environ['PIGLIT_BUILD_DIR'], 'bin')
@@ -77,24 +77,35 @@ class Environment:
             else:
                 yield (key, values)
 
-    def run(self, command):
-        try:
-            p = subprocess.Popen(command,
-                                 stdout=subprocess.PIPE,
-                                 stderr=subprocess.PIPE,
-                                 universal_newlines=True)
-            (stdout, stderr) = p.communicate()
-        except:
-            return "Failed to run " + command
-        return stderr+stdout
-
-    def collectData(self):
-        result = {}
-        system = platform.system()
-        if (system == 'Windows' or system.find("CYGWIN_NT") == 0):
-            result['wglinfo'] = self.run('wglinfo')
-        else:
-            result['glxinfo'] = self.run('glxinfo')
-        if system == 'Linux':
-            result['lspci'] = self.run('lspci')
-        return result
+
+def get_environment():
+    """ Helper function that collects information about the test system
+
+    This uses popen to run a predefined set of binaries, returning their output
+    if they run successfully, or "Not Available" if they are not.
+
+    The commands are given as tuples in the form (<name to store>, <command to
+    run>); with <command to run> as a list-like object
+
+    Returns an iterator that generates the results of each of these commands
+    lazily.
+
+    """
+    commands = {"wglinfo": ['wglinfo'],
+                "glxinfo": ['glxinfo'],
+                "lspci": ['lspci']}
+
+    def iterate():
+        """ Return this generator with the environment """
+        for name, command in commands.iteritems():
+            try:
+                yield (name, subprocess.check_output(command))
+            except OSError as err:
+                # This is the error raised if the binary isn't available, and
+                # we return None in that case
+                if err.errno == 2:
+                    yield (name, None)
+                else:
+                    raise
+
+    return iterate()
diff --git a/piglit-run.py b/piglit-run.py
index aeb6a1f..653d40e 100755
--- a/piglit-run.py
+++ b/piglit-run.py
@@ -132,8 +132,7 @@ def main():
     json_writer.close_dict()
 
     json_writer.write_dict_item('name', results.name)
-
-    for (key, value) in env.collectData().items():
+    for (key, value) in environment.get_environment():
         json_writer.write_dict_item(key, value)
 
     profile = core.merge_test_profiles(args.test_profile)
-- 
2.8.5.2