From be47dcc45f25515d087d250ac6da32261405b009 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Thu, 3 Mar 2016 12:36:15 -0300 Subject: [PATCH] avocado/core/data_dir.py: make data_dir settings dynamic The configurations related to the "data_dir" are currently read at module load time. This means that changes in the configuration between module load time and changes to the settings (such as parsing an additional config file) is never seen. Since the avocado command line application parses extra configuration files (given with `--config`) after the module is loaded, these extra configuration files are never applied to the data_dir configuration. There's one more issue with regards to the the settings usage: because once references to the `settings` instance singleton are grabbed, they will always point to the same settings objects. The data_dir unittests exercise changes to the settings objects by replacing that instance. So, let's refer to the settings using the full location (module.attribute), which will always give back the "current" (be it original or replaced) settings instance. Signed-off-by: Cleber Rosa --- avocado/core/data_dir.py | 29 ++++++++++++++----------- selftests/functional/test_basic.py | 34 ++++++++++++++++++++++++++++++ selftests/unit/test_datadir.py | 34 ++++++++++++++++++++++++++---- 3 files changed, 81 insertions(+), 16 deletions(-) diff --git a/avocado/core/data_dir.py b/avocado/core/data_dir.py index efd53451..ab825492 100755 --- a/avocado/core/data_dir.py +++ b/avocado/core/data_dir.py @@ -34,7 +34,7 @@ import time import tempfile from . import job_id -from .settings import settings +from . import settings from ..utils import path as utils_path from ..utils.data_structures import Borg @@ -42,11 +42,6 @@ _BASE_DIR = os.path.join(sys.modules[__name__].__file__, "..", "..", "..") _BASE_DIR = os.path.abspath(_BASE_DIR) _IN_TREE_TESTS_DIR = os.path.join(_BASE_DIR, 'examples', 'tests') -SETTINGS_BASE_DIR = os.path.expanduser(settings.get_value('datadir.paths', 'base_dir')) -SETTINGS_TEST_DIR = os.path.expanduser(settings.get_value('datadir.paths', 'test_dir')) -SETTINGS_DATA_DIR = os.path.expanduser(settings.get_value('datadir.paths', 'data_dir')) -SETTINGS_LOG_DIR = os.path.expanduser(settings.get_value('datadir.paths', 'logs_dir')) - SYSTEM_BASE_DIR = '/var/lib/avocado' if 'VIRTUAL_ENV' in os.environ: SYSTEM_BASE_DIR = os.environ['VIRTUAL_ENV'] @@ -60,6 +55,13 @@ USER_DATA_DIR = os.path.join(USER_BASE_DIR, 'data') USER_LOG_DIR = os.path.join(USER_BASE_DIR, 'job-results') +def _get_settings_dir(dir_name): + """ + Returns a given "datadir" directory as set by the configuration system + """ + return os.path.expanduser(settings.settings.get_value('datadir.paths', dir_name)) + + def _get_rw_dir(settings_location, system_location, user_location): if utils_path.usable_rw_dir(settings_location): return settings_location @@ -73,7 +75,7 @@ def _get_rw_dir(settings_location, system_location, user_location): def _get_ro_dir(settings_location, system_location, user_location): - if not settings.intree: + if not settings.settings.intree: if utils_path.usable_ro_dir(settings_location): return settings_location @@ -97,7 +99,8 @@ def get_base_dir(): * Data directory * Tests directory """ - return _get_rw_dir(SETTINGS_BASE_DIR, SYSTEM_BASE_DIR, USER_BASE_DIR) + return _get_rw_dir(_get_settings_dir('base_dir'), + SYSTEM_BASE_DIR, USER_BASE_DIR) def get_test_dir(): @@ -106,9 +109,9 @@ def get_test_dir(): The test location is where we store tests written with the avocado API. """ - if settings.intree: + if settings.settings.intree: return _IN_TREE_TESTS_DIR - return _get_ro_dir(SETTINGS_TEST_DIR, SYSTEM_TEST_DIR, USER_TEST_DIR) + return _get_ro_dir(_get_settings_dir('test_dir'), SYSTEM_TEST_DIR, USER_TEST_DIR) def get_data_dir(): @@ -124,7 +127,8 @@ def get_data_dir(): * VM images * Reference bitmaps """ - return _get_rw_dir(SETTINGS_DATA_DIR, SYSTEM_DATA_DIR, USER_DATA_DIR) + return _get_rw_dir(_get_settings_dir('data_dir'), + SYSTEM_DATA_DIR, USER_DATA_DIR) def get_datafile_path(*args): @@ -143,7 +147,8 @@ def get_logs_dir(): The log dir is where we store job/test logs in general. """ - return _get_rw_dir(SETTINGS_LOG_DIR, SYSTEM_LOG_DIR, USER_LOG_DIR) + return _get_rw_dir(_get_settings_dir('logs_dir'), + SYSTEM_LOG_DIR, USER_LOG_DIR) def create_job_logs_dir(logdir=None, unique_id=None): diff --git a/selftests/functional/test_basic.py b/selftests/functional/test_basic.py index 6c2c991c..4b5fbbcb 100644 --- a/selftests/functional/test_basic.py +++ b/selftests/functional/test_basic.py @@ -63,6 +63,40 @@ class RunnerOperationTest(unittest.TestCase): "Version string does not match 'Avocado \\d\\.\\d\\.\\" "d':\n%r" % (result.stderr)) + def test_alternate_config_datadir(self): + """ + Uses the "--config" flag to check custom configuration is applied + + Even on the more complex data_dir module, which adds extra checks + to what is set on the plain settings module. + """ + base_dir = os.path.join(self.tmpdir, 'datadir_base') + os.mkdir(base_dir) + mapping = {'base_dir': base_dir, + 'test_dir': os.path.join(base_dir, 'test'), + 'data_dir': os.path.join(base_dir, 'data'), + 'logs_dir': os.path.join(base_dir, 'logs')} + config = '[datadir.paths]' + for key, value in mapping.iteritems(): + if not os.path.isdir(value): + os.mkdir(value) + config += "%s = %s\n" % (key, value) + fd, config_file = tempfile.mkstemp(dir=self.tmpdir) + os.write(fd, config) + os.close(fd) + + os.chdir(basedir) + cmd = './scripts/avocado --config %s config --datadir' % config_file + result = process.run(cmd) + output = result.stdout + expected_rc = exit_codes.AVOCADO_ALL_OK + self.assertEqual(result.exit_status, expected_rc, + "Avocado did not return rc %d:\n%s" % + (expected_rc, result)) + self.assertIn(' base ' + mapping['base_dir'], result.stdout) + self.assertIn(' data ' + mapping['data_dir'], result.stdout) + self.assertIn(' logs ' + mapping['logs_dir'], result.stdout) + def test_runner_all_ok(self): os.chdir(basedir) cmd_line = './scripts/avocado run --sysinfo=off --job-results-dir %s passtest passtest' % self.tmpdir diff --git a/selftests/unit/test_datadir.py b/selftests/unit/test_datadir.py index 86748968..8da0ea8c 100644 --- a/selftests/unit/test_datadir.py +++ b/selftests/unit/test_datadir.py @@ -48,15 +48,13 @@ class DataDirTest(unittest.TestCase): stg.intree = False flexmock(settings, settings=stg) from avocado.core import data_dir - flexmock(data_dir, settings=stg) - self.assertFalse(data_dir.settings.intree) - reload(data_dir) + flexmock(data_dir.settings, settings=stg) + self.assertFalse(data_dir.settings.settings.intree) for key in self.mapping.keys(): data_dir_func = getattr(data_dir, 'get_%s' % key) self.assertEqual(data_dir_func(), stg.get_value('datadir.paths', key)) finally: flexmock(settings, settings=stg_orig) - reload(data_dir) del data_dir def testUniqueLogDir(self): @@ -80,9 +78,37 @@ class DataDirTest(unittest.TestCase): self.assertEqual(path, path_prefix + uid + ".1") self.assertTrue(os.path.exists(path)) + def testSettingsDirAlternateDynamic(self): + """ + Tests that changes to the data_dir settings are applied dynamically + + To guarantee that, first the data_dir module is loaded. Then a new, + alternate set of data directories are created and set in the + "canonical" settings location, that is, avocado.core.settings.settings. + + No data_dir module reload should be necessary to get the new locations + from data_dir APIs. + """ + stg_orig = settings.settings + from avocado.core import data_dir + (self.alt_mapping, + self.alt_config_file_path) = self._get_temporary_dirs_mapping_and_config() + stg = settings.Settings(self.alt_config_file_path) + flexmock(settings, settings=stg) + for key in self.alt_mapping.keys(): + data_dir_func = getattr(data_dir, 'get_%s' % key) + self.assertEqual(data_dir_func(), self.alt_mapping[key]) + flexmock(settings, settings=stg_orig) + del data_dir + def tearDown(self): os.unlink(self.config_file_path) shutil.rmtree(self.mapping['base_dir']) + # clean up alternate configuration file if set by the test + if hasattr(self, 'alt_config_file_path'): + os.unlink(self.alt_config_file_path) + if hasattr(self, 'alt_mapping'): + shutil.rmtree(self.alt_mapping['base_dir']) if __name__ == '__main__': unittest.main() -- GitLab