[Piglit] [PATCH 02/13] framework: Dont rely on os.environ so much

Dylan Baker baker.dylan.c at gmail.com
Sat Jun 21 05:06:03 PDT 2014


There are some problems with using os.environ, all of which are caused
shell child-variable relationships. Most of these problems are easy to
solve, since we don't actually need to have environment variables set
except during test execution, and that can be passed to Popen (or it's
helper wrappers). This gives us much better assurance that things are
happening in the way we expect.

Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
---
 framework/core.py         | 23 ++++++++---------------
 framework/exectest.py     | 15 +++++++++++----
 framework/programs/run.py | 21 +++++++++------------
 piglit                    | 15 ++++++---------
 4 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/framework/core.py b/framework/core.py
index cd72956..84832cf 100644
--- a/framework/core.py
+++ b/framework/core.py
@@ -206,21 +206,6 @@ def checkDir(dirname, failifexists):
         if e.errno != errno.EEXIST:
             raise
 
-if 'PIGLIT_SOURCE_DIR' not in os.environ:
-    p = os.path
-    os.environ['PIGLIT_SOURCE_DIR'] = p.abspath(p.join(p.dirname(__file__),
-                                                       '..'))
-
-# In debug builds, Mesa will by default log GL API errors to stderr.
-# This is useful for application developers or driver developers
-# trying to debug applications that should execute correctly.  But for
-# piglit we expect to generate errors regularly as part of testing,
-# and for exhaustive error-generation tests (particularly some in
-# khronos's conformance suite), it can end up ooming your system
-# trying to parse the strings.
-if 'MESA_DEBUG' not in os.environ:
-    os.environ['MESA_DEBUG'] = 'silent'
-
 
 class TestResult(dict):
     def __init__(self, *args):
@@ -353,6 +338,14 @@ class Environment:
         self.valgrind = valgrind
         self.dmesg = dmesg
         self.verbose = verbose
+        # env is used to set some base environment variables that are not going
+        # to change across runs, without sending them to os.environ which is
+        # fickle as easy to break
+        self.env = {
+            'PIGLIT_SOURCE_DIR': os.path.abspath(
+                os.path.join(os.path.dirname(__file__), '..')),
+            'MESA_DEBUG': 'silent',
+        }
 
         """
         The filter lists that are read in should be a list of string objects,
diff --git a/framework/exectest.py b/framework/exectest.py
index e55274e..ad6f2ae 100644
--- a/framework/exectest.py
+++ b/framework/exectest.py
@@ -136,7 +136,9 @@ class Test(object):
         """
         self.result['command'] = ' '.join(self.command)
         self.result['environment'] = " ".join(
-            '{0}="{1}"'.format(k, v) for k, v in  self.env.iteritems())
+            '{0}="{1}"'.format(k, v) for k, v in self.env.iteritems()) + \
+            " ".join(
+                '{0}="{1}"'.format(k, v) for k, v in self.ENV.env.iteritems())
 
         if self.check_for_skip_scenario():
             self.result['result'] = 'skip'
@@ -188,9 +190,14 @@ class Test(object):
         return False
 
     def get_command_result(self):
-        fullenv = os.environ.copy()
-        for key, value in self.env.iteritems():
-            fullenv[key] = str(value)
+        # Set the environment for the tests. Use the default settings created
+        # in the Environment constructor first, then use any user defined
+        # variables, finally, use any variables set for the test in the test
+        # profile
+        fullenv = self.ENV.env.copy()
+        for iterable in [os.environ, self.env]:
+            for key, value in iterable.iteritems():
+                fullenv[key] = str(value)
 
         try:
             proc = subprocess.Popen(self.command,
diff --git a/framework/programs/run.py b/framework/programs/run.py
index 16d4e1d..b99d884 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -116,10 +116,6 @@ def run(input_):
               |  SEM_NOOPENFILEERRORBOX
         ctypes.windll.kernel32.SetErrorMode(uMode)
 
-    # Set the platform to pass to waffle
-    if args.platform:
-        os.environ['PIGLIT_PLATFORM'] = args.platform
-
     # If dmesg is requested we must have serial run, this is becasue dmesg
     # isn't reliable with threaded run
     if args.dmesg:
@@ -130,8 +126,8 @@ def run(input_):
         core.PIGLIT_CONFIG.readfp(args.config_file)
         args.config_file.close()
     else:
-        core.PIGLIT_CONFIG.read(os.path.join(os.environ['PIGLIT_SOURCE_DIR'],
-                                             'piglit.conf'))
+        core.PIGLIT_CONFIG.read(os.path.abspath(
+            os.path.join(os.path.dirname(__file__), 'piglit.conf')))
 
     # Pass arguments into Environment
     env = core.Environment(concurrent=args.concurrency,
@@ -142,6 +138,11 @@ def run(input_):
                            dmesg=args.dmesg,
                            verbose=args.verbose)
 
+    # Set the platform to pass to waffle
+    if args.platform:
+        env.env['PIGLIT_PLATFORM'] = args.platform
+
+
     # Change working directory to the root of the piglit directory
     piglit_dir = path.dirname(path.realpath(sys.argv[0]))
     os.chdir(piglit_dir)
@@ -218,12 +219,8 @@ def resume(input_):
                            dmesg=results.options['dmesg'],
                            verbose=results.options['verbose'])
 
-    # attempt to restore a saved platform, if there is no saved platform just
-    # go on
-    try:
-        os.environ['PIGLIT_PLATFORM'] = results.options['platform']
-    except KeyError:
-        pass
+    if results.options.get('platform'):
+        env.env['PIGLIT_PLATFORM'] = results.options['platform']
 
     results_path = path.join(args.results_path, "main")
     json_writer = core.JSONWriter(open(results_path, 'w+'))
diff --git a/piglit b/piglit
index 616e408..4c9a24e 100755
--- a/piglit
+++ b/piglit
@@ -36,25 +36,24 @@ import os.path as path
 import sys
 import argparse
 
-# Setting PIGLIT_SOURCE_DIR (and by extension sys.path) is actually pretty
-# complicated, since there are three seperate uses we need to detect:
+# Setting sys.path is actually pretty complicated, since there are three
+# seperate uses we need to detect:
 # 1) piglit is being run in the source directory, built in tree
 # 2) piglit is being run from the source directory outside of it, built in tree
 # 3) piglit has been built out of tree and installed, and is being run in or
 #    out of the install directory
 
+# Case one is the implicit case. In this event nothing needs to be set, it
+# should "just work" (tm)
+
 # It is critical that this block be run before importing anything from
 # framework (as there is no gaurantee that framework will be in python's path
 # before this blck is run)
 
-# Case 1
-if path.exists('framework/exectest.py'):
-    os.environ['PIGLIT_SOURCE_DIR'] = path.abspath(path.curdir)
-else:
+if not path.exists('framework/exectest.py'):
     dirpath = path.dirname(path.abspath(__file__))
     # Case 2
     if path.exists(path.join(dirpath, 'framework/exectest.py')):
-        os.environ['PIGLIT_SOURCE_DIR'] = dirpath
         sys.path.append(dirpath)
     # Case 3
     else:
@@ -66,8 +65,6 @@ else:
         # piglits installed as piglit${the_date_of_install}, and we need to
         # detect that.
         install_path = path.abspath(path.join(dirpath, '..', 'lib', piglit))
-
-        os.environ['PIGLIT_SOURCE_DIR'] = install_path
         sys.path.append(install_path)
 
 import framework.programs.run as run
-- 
2.0.0



More information about the Piglit mailing list