[Piglit] [PATCH 2/3] compression.py: fix compression to be updated when piglit.conf is reloaded

Dylan Baker baker.dylan.c at gmail.com
Mon Jul 13 17:06:41 PDT 2015


There is currently a bug in piglit (demonstrated by the previous patch),
that shows that compression is hard set before piglit.conf is loaded in
run, and not affected by any subsequent re-reads of piglit.conf. This is
problematic, resulting in the compression mode being either
PIGLIT_COMPRESSION or the default value, bz2.

This patch corrects that by removing constant values and replacing them
with a getting function.

Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
---
 framework/backends/abstract.py               |  7 ++--
 framework/backends/compression.py            | 60 ++++++++++++++++------------
 framework/tests/compressed_backend_tests.py  | 51 ++++++-----------------
 framework/tests/json_backend_tests.py        | 14 ++++---
 framework/tests/json_results_update_tests.py | 14 ++++---
 framework/tests/json_tests.py                | 15 ++++---
 framework/tests/junit_backends_tests.py      | 14 ++++---
 7 files changed, 85 insertions(+), 90 deletions(-)

diff --git a/framework/backends/abstract.py b/framework/backends/abstract.py
index 6147e84..c892995 100644
--- a/framework/backends/abstract.py
+++ b/framework/backends/abstract.py
@@ -47,10 +47,11 @@ def write_compressed(filename):
     Currently it implements no compression
 
     """
-    if compression.MODE != 'none':
-        filename = '{}.{}'.format(filename, compression.MODE)
+    mode = compression.get_mode()
+    if mode != 'none':
+        filename = '{}.{}'.format(filename, mode)
 
-    with compression.COMPRESSOR(filename) as f:
+    with compression.COMPRESSORS[mode](filename) as f:
         yield f
 
 
diff --git a/framework/backends/compression.py b/framework/backends/compression.py
index 7bc488e..b26c4d7 100644
--- a/framework/backends/compression.py
+++ b/framework/backends/compression.py
@@ -22,23 +22,19 @@
 
 This includes both compression and decompression support.
 
-The primary way to interact with this module should be through the use of the
-COMPRESSORS and DECOMPRESSORS constants.
-
-These constants provide a dictionary
-mapping text representations of the compression methods to functions that
-should be called as context managers using the filename as the only argument.
-
-For example:
-with COMPRESSORS['none'] as f:
-    f.write('foobar')
-
-COMPRESSOR provides a convenience method for getting the default compressor,
-although COMPRESSORS is available, for more advanced uses.
+This provides a low level interface of dictionaries, COMPRESSORS and
+DECOMPRESSORS, which use compression modes ('bz2', 'gz', 'xz', 'none') to
+provide open-like functions with correct mode settings for writing or reading,
+respectively.
 
 They should always take unicode objects. It is up to the caller to ensure that
 they're passing unicode and not bytes.
 
+A helper, get_mode(), is provided to return the user selected mode (it will try
+the PIGLIT_COMPRESSION environment variable, then the piglit.conf
+[core]:compression key, and finally the value of compression.DEFAULT). This is
+the best way to get a compressor.
+
 """
 
 from __future__ import print_function, absolute_import, division
@@ -52,6 +48,22 @@ import subprocess
 from framework import exceptions
 from framework.core import PIGLIT_CONFIG
 
+__all__ = [
+    'UnsupportedCompressor',
+    'COMPRESSORS',
+    'DECOMPRESSORS',
+    'get_mode',
+]
+
+
+class UnsupportedCompressor(exceptions.PiglitInternalError):
+    def __init__(self, method, *args, **kwargs):
+        super(UnsupportedCompressor, self).__init__(*args, **kwargs)
+        self.__method = method
+
+    def __str__(self):
+        return 'unsupported compression method {}'.format(self.__method)
+
 
 # TODO: in python3 the bz2 module has an open function
 COMPRESSION_SUFFIXES = ['.gz', '.bz2']
@@ -159,28 +171,26 @@ except ImportError:
         pass
 
 
-def _set_mode():
-    """Set the compression mode.
+def get_mode():
+    """Return the key value of the correct compressor to use.
 
     Try the environment variable PIGLIT_COMPRESSION; then check the
     PIGLIT_CONFIG section 'core', option 'compression'; finally fall back to
     DEFAULT.
 
+    This will raise an UnsupportedCompressionError if there isn't a compressor
+    for that mode. It is the job of the caller to handle this exceptions
+
     """
+    # This is provided as a function rather than a constant because as a
+    # function it can honor changes to the PIGLIT_CONFIG instance, or the
+    # PIGLIT_COMPRESSION environment variable.
+
     method = (os.environ.get('PIGLIT_COMPRESSION') or
               PIGLIT_CONFIG.safe_get('core', 'compression') or
               DEFAULT)
 
     if method not in COMPRESSORS:
-        raise exceptions.PiglitFatalError(
-            'unsupported compression method {}'.format(method))
-    if method not in DECOMPRESSORS:
-        raise exceptions.PiglitFatalError(
-            'unsupported decompression method {}'.format(method))
+        raise UnsupportedCompressor(method)
 
     return method
-
-
-MODE = _set_mode()
-
-COMPRESSOR = COMPRESSORS[MODE]
diff --git a/framework/tests/compressed_backend_tests.py b/framework/tests/compressed_backend_tests.py
index fdef94b..6e0f4ec 100644
--- a/framework/tests/compressed_backend_tests.py
+++ b/framework/tests/compressed_backend_tests.py
@@ -81,31 +81,6 @@ def _add_compression(value):
     return _wrapper
 
 
-def _set_compression_mode(mode):
-    """Change the compression mode for one test."""
-
-    def _wrapper(func):
-        """The actual decorator."""
-
-        @functools.wraps(func)
-        @utils.set_env(PIGLIT_COMPRESSION=mode)
-        def _inner(*args, **kwargs):
-            """The called function."""
-            restore = compression.MODE
-            compression.MODE = compression._set_mode()
-            compression.COMPRESSOR = compression.COMPRESSORS[compression.MODE]
-
-            try:
-                func(*args, **kwargs)
-            finally:
-                compression.MODE = restore
-                compression.COMPRESSOR = compression.COMPRESSORS[compression.MODE]
-
-        return _inner
-
-    return _wrapper
-
-
 def _test_compressor(mode):
     """Helper to simplify testing compressors."""
     func = compression.COMPRESSORS[mode]
@@ -166,24 +141,24 @@ def test_decompress_none():
 
 @_add_compression('foobar')
 @utils.set_env(PIGLIT_COMPRESSION='foobar')
-def test_set_mode_env():
-    """framework.backends.compression._set_mode: uses PIGlIT_COMPRESSION environment variable"""
-    nt.eq_(compression._set_mode(), 'foobar')
+def testget_mode_env():
+    """framework.backends.compression.get_mode: uses PIGlIT_COMPRESSION environment variable"""
+    nt.eq_(compression.get_mode(), 'foobar')
 
 
 @_add_compression('foobar')
 @utils.set_env(PIGLIT_COMPRESSION=None)
 @utils.set_piglit_conf(('core', 'compression', 'foobar'))
-def test_set_mode_piglit_conf():
-    """framework.backends.compression._set_mode: uses piglit.conf [core]:compression value if env is unset"""
-    nt.eq_(compression._set_mode(), 'foobar')
+def testget_mode_piglit_conf():
+    """framework.backends.compression.get_mode: uses piglit.conf [core]:compression value if env is unset"""
+    nt.eq_(compression.get_mode(), 'foobar')
 
 
 @utils.set_env(PIGLIT_COMPRESSION=None)
 @utils.set_piglit_conf(('core', 'compression', None))
-def test_set_mode_default():
-    """framework.backends.compression._set_mode: uses DEFAULT if env and piglit.conf are unset"""
-    nt.eq_(compression._set_mode(), compression.DEFAULT)
+def testget_mode_default():
+    """framework.backends.compression.get_mode: uses DEFAULT if env and piglit.conf are unset"""
+    nt.eq_(compression.get_mode(), compression.DEFAULT)
 
 
 @utils.no_error
@@ -197,7 +172,7 @@ def test_decompress_gz():
     _test_decompressor('gz')
 
 
- at _set_compression_mode('gz')
+ at utils.set_env(PIGLIT_COMPRESSION='gz')
 def test_gz_output():
     """framework.backends: when using gz compression a gz file is created"""
     nt.eq_(_test_extension(), '.gz')
@@ -214,7 +189,7 @@ def test_decompress_bz2():
     _test_decompressor('bz2')
 
 
- at _set_compression_mode('bz2')
+ at utils.set_env(PIGLIT_COMPRESSION='bz2')
 def test_bz2_output():
     """framework.backends: when using bz2 compression a bz2 file is created"""
     nt.eq_(_test_extension(), '.bz2')
@@ -231,7 +206,7 @@ def test_decompress_xz():
     _test_decompressor('xz')
 
 
- at _set_compression_mode('xz')
+ at utils.set_env(PIGLIT_COMPRESSION='xz')
 def test_xz_output():
     """framework.backends: when using xz compression a xz file is created"""
     nt.eq_(_test_extension(), '.xz')
@@ -247,4 +222,4 @@ def test_update_piglit_conf():
     compression mode needs to be changed with them.
 
     """
-    nt.eq_(compression.MODE, 'foobar')
+    nt.eq_(compression.get_mode(), 'foobar')
diff --git a/framework/tests/json_backend_tests.py b/framework/tests/json_backend_tests.py
index 9eff61c..6a58534 100644
--- a/framework/tests/json_backend_tests.py
+++ b/framework/tests/json_backend_tests.py
@@ -32,11 +32,10 @@ except ImportError:
 import nose.tools as nt
 
 from framework import results, backends, exceptions, grouptools
-from framework.backends import compression
 import framework.tests.utils as utils
 from .backends_tests import BACKEND_INITIAL_META
 
-_SAVED_COMPRESSION = compression.MODE
+_SAVED_COMPRESSION = os.environ.get('PIGLIT_COMPRESSON')
 
 
 def setup_module():
@@ -44,13 +43,16 @@ def setup_module():
     # ensure that we're not getting unexpected file extensions. This means that
     # the default can be changed, or environment variables set without
     # affecting unit tests
-    compression.MODE = 'none'
-    compression.COMPRESSOR = compression.COMPRESSORS['none']
+    # We set PIGLIT_COMPRESSION because it is the first value to checked when
+    # setting a compressor
+    os.environ['PIGLIT_COMPRESSION'] = 'none'
 
 
 def teardown_module():
-    compression.MODE = _SAVED_COMPRESSION
-    compression.COMPRESSOR = compression.COMPRESSORS[_SAVED_COMPRESSION]
+    if _SAVED_COMPRESSION is not None:
+        os.environ['PIGLIT_COMPRESSION'] = _SAVED_COMPRESSION
+    else:
+        del os.environ['PIGLIT_COMPRESSION']
 
 
 def test_initialize_jsonbackend():
diff --git a/framework/tests/json_results_update_tests.py b/framework/tests/json_results_update_tests.py
index 7774a53..b492501 100644
--- a/framework/tests/json_results_update_tests.py
+++ b/framework/tests/json_results_update_tests.py
@@ -33,13 +33,12 @@ import nose.tools as nt
 
 import framework.tests.utils as utils
 from framework import backends, results
-from framework.backends import compression
 
 # Disable some errors that cannot be fixed either because tests need to probe
 # protected members, or because of nose requirements, like long lines
 # pylint: disable=protected-access,invalid-name,line-too-long
 
-_SAVED_COMPRESSION = compression.MODE
+_SAVED_COMPRESSION = os.environ.get('PIGLIT_COMPRESSON')
 
 
 def setup_module():
@@ -47,13 +46,16 @@ def setup_module():
     # ensure that we're not getting unexpected file extensions. This means that
     # the default can be changed, or environment variables set without
     # affecting unit tests
-    compression.MODE = 'none'
-    compression.COMPRESSOR = compression.COMPRESSORS['none']
+    # We set PIGLIT_COMPRESSION because it is the first value to checked when
+    # setting a compressor
+    os.environ['PIGLIT_COMPRESSION'] = 'none'
 
 
 def teardown_module():
-    compression.MODE = _SAVED_COMPRESSION
-    compression.COMPRESSOR = compression.COMPRESSORS[_SAVED_COMPRESSION]
+    if _SAVED_COMPRESSION is not None:
+        os.environ['PIGLIT_COMPRESSION'] = _SAVED_COMPRESSION
+    else:
+        del os.environ['PIGLIT_COMPRESSION']
 
 
 class TestV0toV1(object):
diff --git a/framework/tests/json_tests.py b/framework/tests/json_tests.py
index 53701ee..9c985ab 100644
--- a/framework/tests/json_tests.py
+++ b/framework/tests/json_tests.py
@@ -37,12 +37,12 @@ except ImportError:
 
 import framework.core as core
 import framework.tests.utils as utils
-from framework.backends import compression
 from framework.backends.json import JSONBackend
 from framework.programs.run import _create_metadata
 
 # pylint: disable=invalid-name
-_SAVED_COMPRESSION = compression.MODE
+
+_SAVED_COMPRESSION = os.environ.get('PIGLIT_COMPRESSON')
 
 
 def setup_module():
@@ -50,13 +50,16 @@ def setup_module():
     # ensure that we're not getting unexpected file extensions. This means that
     # the default can be changed, or environment variables set without
     # affecting unit tests
-    compression.MODE = 'none'
-    compression.COMPRESSOR = compression.COMPRESSORS['none']
+    # We set PIGLIT_COMPRESSION because it is the first value to checked when
+    # setting a compressor
+    os.environ['PIGLIT_COMPRESSION'] = 'none'
 
 
 def teardown_module():
-    compression.MODE = _SAVED_COMPRESSION
-    compression.COMPRESSOR = compression.COMPRESSORS[_SAVED_COMPRESSION]
+    if _SAVED_COMPRESSION is not None:
+        os.environ['PIGLIT_COMPRESSION'] = _SAVED_COMPRESSION
+    else:
+        del os.environ['PIGLIT_COMPRESSION']
 
 
 # Helpers
diff --git a/framework/tests/junit_backends_tests.py b/framework/tests/junit_backends_tests.py
index e2b1dd7..df70ada 100644
--- a/framework/tests/junit_backends_tests.py
+++ b/framework/tests/junit_backends_tests.py
@@ -35,7 +35,6 @@ from nose.plugins.skip import SkipTest
 from framework import results, backends, grouptools, status
 import framework.tests.utils as utils
 from .backends_tests import BACKEND_INITIAL_META
-from framework.backends import compression
 
 
 JUNIT_SCHEMA = 'framework/tests/schema/junit-7.xsd'
@@ -54,7 +53,7 @@ _XML = """\
   </testsuites>
 """
 
-_SAVED_COMPRESSION = compression.MODE
+_SAVED_COMPRESSION = os.environ.get('PIGLIT_COMPRESSON')
 
 
 def setup_module():
@@ -62,13 +61,16 @@ def setup_module():
     # ensure that we're not getting unexpected file extensions. This means that
     # the default can be changed, or environment variables set without
     # affecting unit tests
-    compression.MODE = 'none'
-    compression.COMPRESSOR = compression.COMPRESSORS['none']
+    # We set PIGLIT_COMPRESSION because it is the first value to checked when
+    # setting a compressor
+    os.environ['PIGLIT_COMPRESSION'] = 'none'
 
 
 def teardown_module():
-    compression.MODE = _SAVED_COMPRESSION
-    compression.COMPRESSOR = compression.COMPRESSORS[_SAVED_COMPRESSION]
+    if _SAVED_COMPRESSION is not None:
+        os.environ['PIGLIT_COMPRESSION'] = _SAVED_COMPRESSION
+    else:
+        del os.environ['PIGLIT_COMPRESSION']
 
 
 class TestJunitNoTests(utils.StaticDirectory):
-- 
2.4.5



More information about the Piglit mailing list