From 769a6910876d3259d4fd53781191ce4dc5d40d6c Mon Sep 17 00:00:00 2001 From: Mark McClain Date: Wed, 13 Mar 2013 00:08:04 -0400 Subject: [PATCH 1/2] os.environ test clean-up and reorder overwrite fixes issue 197 --- pecan/configuration.py | 15 +++++++-------- pecan/tests/test_conf.py | 37 ++++++++++++++++++------------------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/pecan/configuration.py b/pecan/configuration.py index 9925ba9..c842f20 100644 --- a/pecan/configuration.py +++ b/pecan/configuration.py @@ -156,23 +156,22 @@ def conf_from_file(filepath): return conf_from_dict(conf_dict) -def conf_from_env(): +def get_conf_path_from_env(): ''' If the ``PECAN_CONFIG`` environment variable exists and it points to a valid path it will return that, otherwise it will raise a ``RuntimeError``. ''' config_path = os.environ.get('PECAN_CONFIG') - error = None if not config_path: error = "PECAN_CONFIG is not set and " \ "no config file was passed as an argument." elif not os.path.isfile(config_path): error = "PECAN_CONFIG was set to an invalid path: %s" % config_path + else: + return config_path - if error: - raise RuntimeError(error) - return config_path + raise RuntimeError(error) def conf_from_dict(conf_dict): @@ -209,12 +208,12 @@ def set_config(config, overwrite=False): which represents a (relative) configuration filename. ''' + if config is None: + config = get_conf_path_from_env() + if overwrite is True: _runtime_conf.empty() - if config is None: - config = conf_from_env() - if isinstance(config, basestring): config = conf_from_file(config) _runtime_conf.update(config) diff --git a/pecan/tests/test_conf.py b/pecan/tests/test_conf.py index 36ce380..e62da2c 100644 --- a/pecan/tests/test_conf.py +++ b/pecan/tests/test_conf.py @@ -3,7 +3,6 @@ import sys from pecan.tests import PecanTestCase -from mock import patch __here__ = os.path.dirname(__file__) @@ -295,33 +294,33 @@ class TestConfFromEnv(PecanTestCase): def setUp(self): super(TestConfFromEnv, self).setUp() - self.conf_from_env = self.get_conf_from_env() + self.addCleanup(self._remove_config_key) - def tearDown(self): - os.environ['PECAN_CONFIG'] = '' - - def get_conf_from_env(self): from pecan import configuration - return configuration.conf_from_env + self.get_conf_path_from_env = configuration.get_conf_path_from_env - def assertRaisesMessage(self, msg, exc, func, *args, **kwargs): - try: - func(*args, **kwargs) - self.assertFail() - except Exception as error: - assert issubclass(exc, error.__class__) - assert error.message == msg + def _remove_config_key(self): + os.environ.pop('PECAN_CONFIG', None) - @patch.dict('os.environ', {'PECAN_CONFIG': '/'}) def test_invalid_path(self): + os.environ['PECAN_CONFIG'] = '/' msg = "PECAN_CONFIG was set to an invalid path: /" - self.assertRaisesMessage(msg, RuntimeError, self.conf_from_env) + self.assertRaisesRegexp( + RuntimeError, + msg, + self.get_conf_path_from_env + ) def test_is_not_set(self): msg = "PECAN_CONFIG is not set and " \ "no config file was passed as an argument." - self.assertRaisesMessage(msg, RuntimeError, self.conf_from_env) + self.assertRaisesRegexp( + RuntimeError, + msg, + self.get_conf_path_from_env + ) - @patch.dict('os.environ', {'PECAN_CONFIG': os.path.abspath(__file__)}) def test_return_valid_path(self): - assert self.conf_from_env() == os.path.abspath(__file__) + __here__ = os.path.abspath(__file__) + os.environ['PECAN_CONFIG'] = __here__ + assert self.get_conf_path_from_env() == __here__ From c049ca43de6e75f56cb187c42d1b428064427c80 Mon Sep 17 00:00:00 2001 From: Mark McClain Date: Wed, 13 Mar 2013 01:26:05 -0400 Subject: [PATCH 2/2] add comment explaining why the code was reordered --- pecan/configuration.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pecan/configuration.py b/pecan/configuration.py index c842f20..939f079 100644 --- a/pecan/configuration.py +++ b/pecan/configuration.py @@ -211,6 +211,7 @@ def set_config(config, overwrite=False): if config is None: config = get_conf_path_from_env() + # must be after the fallback other a bad fallback will incorrectly clear if overwrite is True: _runtime_conf.empty()