Make testing with different configuration values easier and less error-prone by using mock.patch to change the configuration.

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)
+            setx = lambda self, val: getattr(self, setifok_func)(val)
-            return
+        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
+        # Set the attribute to the correct value
-        else:
+        setattr(self, key, value)
-            setattr(self, key, value)
-
-        return
 
     #
     # 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

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)
+        with patch("module_build_service.config.Config.yaml_submit_allowed",
-        data = submit()
+                new_callable=PropertyMock, return_value = True):
-        self.assertEqual(data['id'], 1)
+            conf.set_item("yaml_submit_allowed", True)
+            data = submit()
+            self.assertEqual(data['id'], 1)
 
-        conf.set_item("yaml_submit_allowed", False)
+        with patch("module_build_service.config.Config.yaml_submit_allowed",
-        data = submit()
+                new_callable=PropertyMock, return_value = False):
-        self.assertEqual(data['status'], 401)
+            data = submit()
-        self.assertEqual(data['message'], 'YAML submission is not enabled')
+            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'}))

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)

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):
+    @patch("module_build_service.config.Config.modules_allow_repository", 
-        _prev_modules_allow_repository = conf.modules_allow_repository
+           new_callable=PropertyMock, return_value = True)
-        conf.set_item("modules_allow_repository", 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(
-            rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps(
+            {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/'
-                {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/'
+                'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'}))
-                    'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'}))
-        except e:
-            raise
-        finally:
-            conf.set_item("modules_allow_repository", _prev_modules_allow_repository)
         data = json.loads(rv.data)
 
         assert 'component_builds' in data, data