diff --git a/module_build_service/config.py b/module_build_service/config.py index 18e08b31..af37c24d 100644 --- a/module_build_service/config.py +++ b/module_build_service/config.py @@ -324,15 +324,26 @@ class Config(object): self.set_item(key.lower(), getattr(conf_section_obj, key)) def set_item(self, key, value): - """Set value for configuration item as self.key = value""" + """ + Set value for configuration item. Creates the self._key = value + attribute and self.key property to set/get/del the attribute. + """ if key == 'set_item' or key.startswith('_'): raise Exception("Configuration item's name is not allowed: %s" % key) - # customized check & set if there's a corresponding handler + # Create the empty self._key attribute, so we can assign to it. + setattr(self, "_" + key, None) + + # Create self.key property to access the self._key attribute. + # Use the setifok_func if available for the attribute. setifok_func = '_setifok_{}'.format(key) if hasattr(self, setifok_func): - getattr(self, setifok_func)(value) - return + setx = lambda self, val: getattr(self, setifok_func)(val) + else: + setx = lambda self, val: setattr(self, "_" + key, val) + getx = lambda self: getattr(self, "_" + key) + delx = lambda self: delattr(self, "_" + key) + setattr(Config, key, property(getx, setx, delx)) # managed/registered configuration items if key in self._defaults: @@ -340,21 +351,17 @@ class Config(object): convert = self._defaults[key]['type'] if convert in [bool, int, list, str, set]: try: - setattr(self, key, convert(value)) + # Do no try to convert None... + if value is not None: + value = convert(value) except: raise TypeError("Configuration value conversion failed for name: %s" % key) - # if type is None, do not perform any conversion - elif convert is None: - setattr(self, key, value) # unknown type/unsupported conversion - else: + elif convert is not None: raise TypeError("Unsupported type %s for configuration item name: %s" % (convert, key)) - # passthrough for unmanaged configuration items - else: - setattr(self, key, value) - - return + # Set the attribute to the correct value + setattr(self, key, value) # # Register your _setifok_* handlers here @@ -364,62 +371,62 @@ class Config(object): s = str(s) if s not in ("koji", "copr", "mock"): raise ValueError("Unsupported buildsystem: %s." % s) - self.system = s + self._system = s def _setifok_polling_interval(self, i): if not isinstance(i, int): raise TypeError("polling_interval needs to be an int") if i < 0: raise ValueError("polling_interval must be >= 0") - self.polling_interval = i + self._polling_interval = i def _setifok_rpms_default_repository(self, s): rpm_repo = str(s) if rpm_repo[-1] != '/': rpm_repo = rpm_repo + '/' - self.rpms_default_repository = rpm_repo + self._rpms_default_repository = rpm_repo def _setifok_rpms_default_cache(self, s): rpm_cache = str(s) if rpm_cache[-1] != '/': rpm_cache = rpm_cache + '/' - self.rpms_default_cache = rpm_cache + self._rpms_default_cache = rpm_cache def _setifok_log_backend(self, s): if s is None: - self.log_backend = "console" + self._log_backend = "console" elif s not in logger.supported_log_backends(): raise ValueError("Unsupported log backend") - self.log_backend = str(s) + self._log_backend = str(s) def _setifok_log_file(self, s): if s is None: - self.log_file = "" + self._log_file = "" else: - self.log_file = str(s) + self._log_file = str(s) def _setifok_log_level(self, s): level = str(s).lower() - self.log_level = logger.str_to_log_level(level) + self._log_level = logger.str_to_log_level(level) def _setifok_messaging(self, s): s = str(s) if s not in ("fedmsg", "amq", "in_memory"): raise ValueError("Unsupported messaging system.") - self.messaging = s + self._messaging = s def _setifok_amq_recv_addresses(self, l): assert isinstance(l, list) or isinstance(l, tuple) - self.amq_recv_addresses = list(l) + self._amq_recv_addresses = list(l) def _setifok_scmurls(self, l): if not isinstance(l, list): raise TypeError("scmurls needs to be a list.") - self.scmurls = [str(x) for x in l] + self._scmurls = [str(x) for x in l] def _setifok_num_consecutive_builds(self, i): if not isinstance(i, int): raise TypeError('NUM_CONSECUTIVE_BUILDS needs to be an int') if i < 0: raise ValueError('NUM_CONSECUTIVE_BUILDS must be >= 0') - self.num_consecutive_builds = i + self._num_consecutive_builds = i diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index b8d0b841..464796aa 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -34,7 +34,7 @@ import module_build_service.scheduler.handlers.repos import module_build_service.utils from module_build_service import db, models, conf -from mock import patch +from mock import patch, PropertyMock from tests import app, init_data import os @@ -203,15 +203,13 @@ class TestModuleBuilder(GenericBuilder): pass +@patch("module_build_service.config.Config.system", + new_callable=PropertyMock, return_value = "mock") class TestBuild(unittest.TestCase): def setUp(self): GenericBuilder.register_backend_class(TestModuleBuilder) self.client = app.test_client() - self._prev_system = conf.system - self._prev_num_consecutive_builds = conf.num_consecutive_builds - - conf.set_item("system", "mock") init_data() models.ModuleBuild.query.delete() @@ -222,8 +220,6 @@ class TestBuild(unittest.TestCase): self.vcr.__enter__() def tearDown(self): - conf.set_item("system", self._prev_system) - conf.set_item("num_consecutive_builds", self._prev_num_consecutive_builds) TestModuleBuilder.reset() # Necessary to restart the twisted reactor for the next test. @@ -237,7 +233,7 @@ class TestBuild(unittest.TestCase): @timed(30) @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') - def test_submit_build(self, mocked_scm, mocked_get_user): + def test_submit_build(self, mocked_scm, mocked_get_user, conf_system): """ Tests the build of testmodule.yaml using TestModuleBuilder which succeeds everytime. @@ -292,7 +288,7 @@ class TestBuild(unittest.TestCase): @timed(30) @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') - def test_submit_build_from_yaml(self, mocked_scm, mocked_get_user): + def test_submit_build_from_yaml(self, mocked_scm, mocked_get_user, conf_system): MockedSCM(mocked_scm, "testmodule", "testmodule.yaml") testmodule = os.path.join(base_dir, 'staged_data', 'testmodule.yaml') @@ -305,18 +301,21 @@ class TestBuild(unittest.TestCase): data={'yaml': (testmodule, yaml)}) return json.loads(rv.data) - conf.set_item("yaml_submit_allowed", True) - data = submit() - self.assertEqual(data['id'], 1) + with patch("module_build_service.config.Config.yaml_submit_allowed", + new_callable=PropertyMock, return_value = True): + conf.set_item("yaml_submit_allowed", True) + data = submit() + self.assertEqual(data['id'], 1) - conf.set_item("yaml_submit_allowed", False) - data = submit() - self.assertEqual(data['status'], 401) - self.assertEqual(data['message'], 'YAML submission is not enabled') + with patch("module_build_service.config.Config.yaml_submit_allowed", + new_callable=PropertyMock, return_value = False): + data = submit() + self.assertEqual(data['status'], 401) + self.assertEqual(data['message'], 'YAML submission is not enabled') @timed(30) @patch('module_build_service.auth.get_user', return_value=user) - def test_submit_build_with_optional_params(self, mocked_get_user): + def test_submit_build_with_optional_params(self, mocked_get_user, conf_system): params = {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68932c90de214d9d13feefbd35246a81b6cb8d49'} @@ -335,7 +334,7 @@ class TestBuild(unittest.TestCase): @timed(30) @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') - def test_submit_build_cancel(self, mocked_scm, mocked_get_user): + def test_submit_build_cancel(self, mocked_scm, mocked_get_user, conf_system): """ Submit all builds for a module and cancel the module build later. """ @@ -387,7 +386,7 @@ class TestBuild(unittest.TestCase): @timed(30) @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') - def test_submit_build_instant_complete(self, mocked_scm, mocked_get_user): + def test_submit_build_instant_complete(self, mocked_scm, mocked_get_user, conf_system): """ Tests the build of testmodule.yaml using TestModuleBuilder which succeeds everytime. @@ -418,7 +417,11 @@ class TestBuild(unittest.TestCase): @timed(30) @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') - def test_submit_build_concurrent_threshold(self, mocked_scm, mocked_get_user): + @patch("module_build_service.config.Config.num_consecutive_builds", + new_callable=PropertyMock, return_value = 1) + def test_submit_build_concurrent_threshold(self, conf_num_consecutive_builds, + mocked_scm, mocked_get_user, + conf_system): """ Tests the build of testmodule.yaml using TestModuleBuilder with num_consecutive_builds set to 1. @@ -426,8 +429,6 @@ class TestBuild(unittest.TestCase): MockedSCM(mocked_scm, 'testmodule', 'testmodule.yaml', '620ec77321b2ea7b0d67d82992dda3e1d67055b4') - conf.set_item("num_consecutive_builds", 1) - rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68932c90de214d9d13feefbd35246a81b6cb8d49'})) diff --git a/tests/test_messaging.py b/tests/test_messaging.py index b6e810b5..97043f55 100644 --- a/tests/test_messaging.py +++ b/tests/test_messaging.py @@ -23,6 +23,7 @@ import unittest from module_build_service import messaging, conf +from mock import patch, PropertyMock class TestFedmsgMessaging(unittest.TestCase): @@ -53,7 +54,9 @@ class TestFedmsgMessaging(unittest.TestCase): self.assertEqual(msg.build_id, 614503) self.assertEqual(msg.build_new_state, 1) - def test_copr_build_end(self): + @patch("module_build_service.config.Config.system", + new_callable=PropertyMock, return_value = "copr") + def test_copr_build_end(self, conf_system): # http://fedora-fedmsg.readthedocs.io/en/latest/topics.html#copr-build-end copr_build_end_msg = { 'msg': { @@ -75,7 +78,6 @@ class TestFedmsgMessaging(unittest.TestCase): 'username': 'copr' } - conf.set_item("system", "copr") topic = 'org.fedoraproject.prod.copr.build.end' msg = messaging.BaseMessage.from_fedmsg(topic, copr_build_end_msg) self.assertIsInstance(msg, messaging.KojiBuildChange) diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 195a2d71..7d911005 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -24,7 +24,7 @@ import unittest import json import time import vcr -from mock import patch, Mock +from mock import patch, Mock, PropertyMock from shutil import copyfile from os import path, mkdir from os.path import dirname @@ -475,19 +475,14 @@ class TestViews(unittest.TestCase): @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') - def test_submit_build_includedmodule(self, mocked_scm, mocked_get_user): - _prev_modules_allow_repository = conf.modules_allow_repository - conf.set_item("modules_allow_repository", True) + @patch("module_build_service.config.Config.modules_allow_repository", + new_callable=PropertyMock, return_value = True) + def test_submit_build_includedmodule(self, conf, mocked_scm, mocked_get_user): mocked_scm_obj = MockedSCM(mocked_scm, "includedmodules", ["includedmodules.yaml", "testmodule.yaml"]) - try: - rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' - 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) - except e: - raise - finally: - conf.set_item("modules_allow_repository", _prev_modules_allow_repository) + rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) assert 'component_builds' in data, data