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 <[email protected]> --- 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') -@_set_compression_mode('gz') [email protected]_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') -@_set_compression_mode('bz2') [email protected]_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') -@_set_compression_mode('xz') [email protected]_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 _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
