Merge lp:~james-page/charms/precise/glance/unit-tests into lp:~charmers/charms/precise/glance/trunk

[James Page]

Improve unit testing coverage + a couple of minor tweaks.

[lp:~james-page/charms/precise/glance/unit-tests]

40. By James Page on 2013-10-20

Fixup use of assert_called()

=== modified file 'Makefile'
--- Makefile	2013-09-26 09:47:58 +0000
+++ Makefile	2013-10-20 18:36:29 +0000
@@ -2,6 +2,7 @@
 
 lint:
 	@flake8 --exclude hooks/charmhelpers hooks
+	@flake8 unit_tests
 	@charm proof
 
 sync:

=== modified file 'hooks/glance_contexts.py'
--- hooks/glance_contexts.py	2013-10-14 11:24:25 +0000
+++ hooks/glance_contexts.py	2013-10-20 18:36:29 +0000
@@ -16,6 +16,8 @@
 )
 
 
+# TODO: switch to charmhelpers once landed
+# NOTE: zero units tests - done in charmhelpers
 def is_relation_made(relation, key='private-address'):
     for r_id in relation_ids(relation):
         for unit in related_units(r_id):
@@ -83,7 +85,4 @@
     service_namespace = 'glance'
 
     def __call__(self):
-        #from glance_utils import service_enabled
-        #if not service_enabled('glance-api'):
-        #    return {}
         return super(ApacheSSLContext, self).__call__()

=== modified file 'hooks/glance_relations.py'
--- hooks/glance_relations.py	2013-09-27 16:15:43 +0000
+++ hooks/glance_relations.py	2013-10-20 18:36:29 +0000
@@ -1,5 +1,4 @@
 #!/usr/bin/python
-import os
 import sys
 
 from glance_utils import (
@@ -31,12 +30,15 @@
     unit_get,
     UnregisteredHookError, )
 
-from charmhelpers.core.host import restart_on_change, service_stop
+from charmhelpers.core.host import (
+    restart_on_change,
+    service_stop,
+    mkdir, )
 
 from charmhelpers.fetch import apt_install, apt_update
 
 from charmhelpers.contrib.hahelpers.cluster import (
-    canonical_url, eligible_leader, is_leader)
+    canonical_url, eligible_leader)
 
 from charmhelpers.contrib.openstack.utils import (
     configure_installation_source,
@@ -48,9 +50,8 @@
 from charmhelpers.payload.execd import execd_preinstall
 
 from subprocess import (
-    check_call, )
-
-from commands import getstatusoutput
+    check_call,
+    call, )
 
 hooks = Hooks()
 
@@ -97,7 +98,7 @@
 
     if eligible_leader(CLUSTER_RES):
         if rel == "essex":
-            (status, output) = getstatusoutput('glance-manage db_version')
+            status = call(['glance-manage', 'db_version'])
             if status != 0:
                 juju_log('Setting version_control to 0')
                 check_call(["glance-manage", "version_control", "0"])
@@ -126,7 +127,7 @@
 def object_store_joined():
 
     if 'identity-service' not in CONFIGS.complete_contexts():
-        juju_log('Deferring swift stora configuration until '
+        juju_log('Deferring swift storage configuration until '
                  'an identity-service relation exists')
         return
 
@@ -139,8 +140,7 @@
 
 @hooks.hook('ceph-relation-joined')
 def ceph_joined():
-    if not os.path.isdir('/etc/ceph'):
-        os.mkdir('/etc/ceph')
+    mkdir('/etc/ceph')
     apt_install(['ceph-common', 'python-ceph'])
 
 
@@ -274,7 +274,7 @@
     if not clustered or clustered in [None, 'None', '']:
         juju_log('ha_changed: hacluster subordinate is not fully clustered.')
         return
-    if not is_leader(CLUSTER_RES):
+    if not eligible_leader(CLUSTER_RES):
         juju_log('ha_changed: hacluster complete but we are not leader.')
         return
 

=== modified file 'hooks/glance_utils.py'
--- hooks/glance_utils.py	2013-10-14 11:24:25 +0000
+++ hooks/glance_utils.py	2013-10-20 18:36:29 +0000
@@ -13,9 +13,11 @@
 
 from charmhelpers.core.hookenv import (
     config,
-    log as juju_log,
+    log,
     relation_ids)
 
+from charmhelpers.core.host import mkdir
+
 from charmhelpers.contrib.openstack import (
     templating,
     context, )
@@ -82,7 +84,7 @@
     }),
     (CEPH_CONF, {
         'hook_contexts': [context.CephContext()],
-        'services': []
+        'services': ['glance-api', 'glance-registry']
     }),
     (HAPROXY_CONF, {
         'hook_contexts': [context.HAProxyContext(),
@@ -115,8 +117,7 @@
              HAPROXY_CONF]
 
     if relation_ids('ceph'):
-        if not os.path.isdir('/etc/ceph'):
-            os.mkdir('/etc/ceph')
+        mkdir('/etc/ceph')
         confs.append(CEPH_CONF)
 
     for conf in confs:
@@ -157,7 +158,7 @@
     new_src = config('openstack-origin')
     new_os_rel = get_os_codename_install_source(new_src)
 
-    juju_log('Performing OpenStack upgrade to %s.' % (new_os_rel))
+    log('Performing OpenStack upgrade to %s.' % (new_os_rel))
 
     configure_installation_source(new_src)
     dpkg_opts = [

=== modified file 'unit_tests/test_glance_relations.py'
--- unit_tests/test_glance_relations.py	2013-09-27 16:15:43 +0000
+++ unit_tests/test_glance_relations.py	2013-10-20 18:36:29 +0000
@@ -38,7 +38,6 @@
     'openstack_upgrade_available',
     # charmhelpers.contrib.hahelpers.cluster_utils
     'eligible_leader',
-    'is_leader',
     # glance_utils
     'restart_map',
     'register_configs',
@@ -47,9 +46,11 @@
     'ensure_ceph_keyring',
     'ensure_ceph_pool',
     # other
-    'getstatusoutput',
+    'call',
     'check_call',
-    'execd_preinstall'
+    'execd_preinstall',
+    'mkdir',
+    'lsb_release'
 ]
 
 
@@ -65,15 +66,27 @@
         relations.install_hook()
         self.configure_installation_source.assert_called_with(repo)
         self.assertTrue(self.apt_update.called)
-        self.apt_install.assert_called_with(['apache2', 'glance', 'python-mysqldb',
-                                             'python-swift', 'python-keystone',
+        self.apt_install.assert_called_with(['apache2', 'glance',
+                                             'python-mysqldb',
+                                             'python-swift',
+                                             'python-keystone',
                                              'uuid', 'haproxy'])
-        self.execd_preinstall.assert_called()
+        self.assertTrue(self.execd_preinstall.called)
+
+    def test_install_hook_precise_distro(self):
+        self.test_config.set('openstack-origin', 'distro')
+        self.lsb_release.return_value = {'DISTRIB_CODENAME': 'precise'}
+        self.service_stop.return_value = True
+        relations.install_hook()
+        self.configure_installation_source.assert_called_with(
+            "cloud:precise-folsom"
+        )
 
     def test_db_joined(self):
         self.unit_get.return_value = 'glance.foohost.com'
         relations.db_joined()
-        self.relation_set.assert_called_with(database='glance', username='glance',
+        self.relation_set.assert_called_with(database='glance',
+                                             username='glance',
                                              hostname='glance.foohost.com')
         self.unit_get.assert_called_with('private-address')
 
@@ -97,7 +110,7 @@
         self._shared_db_test(configs)
         self.assertEquals([call('/etc/glance/glance-registry.conf'),
                            call('/etc/glance/glance-api.conf')],
-                           configs.write.call_args_list)
+                          configs.write.call_args_list)
         self.juju_log.assert_called_with(
             'Cluster leader, performing db sync'
         )
@@ -106,10 +119,10 @@
     @patch.object(relations, 'CONFIGS')
     def test_db_changed_with_essex_not_setting_version_control(self, configs):
         self.get_os_codename_package.return_value = "essex"
-        self.getstatusoutput.return_value = (0, "version")
+        self.call.return_value = 0
         self._shared_db_test(configs)
         self.assertEquals([call('/etc/glance/glance-registry.conf')],
-                           configs.write.call_args_list)
+                          configs.write.call_args_list)
         self.juju_log.assert_called_with(
             'Cluster leader, performing db sync'
         )
@@ -118,10 +131,10 @@
     @patch.object(relations, 'CONFIGS')
     def test_db_changed_with_essex_setting_version_control(self, configs):
         self.get_os_codename_package.return_value = "essex"
-        self.getstatusoutput.return_value = (1, "version")
+        self.call.return_value = 1
         self._shared_db_test(configs)
         self.assertEquals([call('/etc/glance/glance-registry.conf')],
-                           configs.write.call_args_list)
+                          configs.write.call_args_list)
         self.check_call.assert_called_with(
             ["glance-manage", "version_control", "0"]
         )
@@ -162,12 +175,13 @@
         configs.write = MagicMock()
         relations.object_store_joined()
         self.juju_log.assert_called_with(
-            'Deferring swift stora configuration until '
+            'Deferring swift storage configuration until '
             'an identity-service relation exists'
         )
 
     @patch.object(relations, 'CONFIGS')
-    def test_object_store_joined_with_identity_service_without_object_store(self, configs):
+    def test_object_store_joined_with_identity_service_without_object_store(
+            self, configs):
         configs.complete_contexts = MagicMock()
         configs.complete_contexts.return_value = ['identity-service']
         configs.write = MagicMock()
@@ -177,20 +191,19 @@
         )
 
     @patch.object(relations, 'CONFIGS')
-    def test_object_store_joined_with_identity_service_with_object_store(self, configs):
+    def test_object_store_joined_with_identity_service_with_object_store(
+            self, configs):
         configs.complete_contexts = MagicMock()
-        configs.complete_contexts.return_value = ['identity-service', 'object-store']
+        configs.complete_contexts.return_value = ['identity-service',
+                                                  'object-store']
         configs.write = MagicMock()
         relations.object_store_joined()
         self.assertEquals([call('/etc/glance/glance-api.conf')],
-                           configs.write.call_args_list)
+                          configs.write.call_args_list)
 
-    @patch('os.mkdir')
-    @patch('os.path.isdir')
-    def test_ceph_joined(self, isdir, mkdir):
-        isdir.return_value = False
+    def test_ceph_joined(self):
         relations.ceph_joined()
-        mkdir.assert_called_with('/etc/ceph')
+        self.mkdir.assert_called_with('/etc/ceph')
         self.apt_install.assert_called_with(['ceph-common', 'python-ceph'])
 
     @patch.object(relations, 'CONFIGS')
@@ -223,7 +236,7 @@
         relations.ceph_changed()
         self.assertEquals([call('/etc/glance/glance-api.conf'),
                            call('/etc/ceph/ceph.conf')],
-                           configs.write.call_args_list)
+                          configs.write.call_args_list)
         self.ensure_ceph_pool.assert_called_with(service=self.service_name(),
                                                  replicas=2)
 
@@ -260,35 +273,44 @@
         }
         self.relation_set.assert_called_with(**ex)
 
+    @patch.object(relations, 'CONFIGS')
+    def test_keystone_changes_incomplete(self, configs):
+        configs.complete_contexts.return_value = []
+        relations.keystone_changed()
+        self.assertTrue(self.juju_log.called)
+        self.assertFalse(configs.write.called)
+
     @patch.object(relations, 'configure_https')
     @patch.object(relations, 'CONFIGS')
-    def test_keystone_changed_no_object_store_relation(self, configs, configure_https):
+    def test_keystone_changed_no_object_store_relation(self, configs,
+                                                       configure_https):
         configs.complete_contexts = MagicMock()
         configs.complete_contexts.return_value = ['identity-service']
         configs.write = MagicMock()
-        self.relation_ids.return_value = False
+        self.relation_ids.return_value = []
         relations.keystone_changed()
         self.assertEquals([call('/etc/glance/glance-api.conf'),
                            call('/etc/glance/glance-registry.conf'),
                            call('/etc/glance/glance-api-paste.ini'),
                            call('/etc/glance/glance-registry-paste.ini')],
-                           configs.write.call_args_list)
+                          configs.write.call_args_list)
         self.assertTrue(configure_https.called)
 
     @patch.object(relations, 'configure_https')
     @patch.object(relations, 'object_store_joined')
     @patch.object(relations, 'CONFIGS')
-    def test_keystone_changed_with_object_store_relation(self, configs, object_store_joined, configure_https):
+    def test_keystone_changed_with_object_store_relation(
+            self, configs, object_store_joined, configure_https):
         configs.complete_contexts = MagicMock()
         configs.complete_contexts.return_value = ['identity-service']
         configs.write = MagicMock()
-        self.relation_ids.return_value = True
+        self.relation_ids.return_value = ['object-store:0']
         relations.keystone_changed()
         self.assertEquals([call('/etc/glance/glance-api.conf'),
                            call('/etc/glance/glance-registry.conf'),
                            call('/etc/glance/glance-api-paste.ini'),
                            call('/etc/glance/glance-registry-paste.ini')],
-                           configs.write.call_args_list)
+                          configs.write.call_args_list)
         object_store_joined.assert_called_with()
         self.assertTrue(configure_https.called)
 
@@ -317,7 +339,7 @@
         relations.cluster_changed()
         self.assertEquals([call('/etc/glance/glance-api.conf'),
                            call('/etc/haproxy/haproxy.cfg')],
-                           configs.write.call_args_list)
+                          configs.write.call_args_list)
 
     @patch.object(relations, 'cluster_changed')
     def test_upgrade_charm(self, cluster_changed):
@@ -337,8 +359,10 @@
             'init_services': {'res_glance_haproxy': 'haproxy'},
             'resources': {'res_glance_vip': 'ocf:heartbeat:IPaddr2',
                           'res_glance_haproxy': 'lsb:haproxy'},
-            'resource_params': {'res_glance_vip': 'params ip="10.10.10.10" cidr_netmask="24" nic="em1"',
-                                'res_glance_haproxy': 'op monitor interval="5s"'},
+            'resource_params': {
+                'res_glance_vip': 'params ip="10.10.10.10"'
+                                  ' cidr_netmask="24" nic="em1"',
+                'res_glance_haproxy': 'op monitor interval="5s"'},
             'clones': {'cl_glance_haproxy': 'res_glance_haproxy'}
         }
         self.relation_set.assert_called_with(**args)
@@ -346,56 +370,79 @@
     def test_ha_relation_changed_not_clustered(self):
         self.relation_get.return_value = False
         relations.ha_relation_changed()
-
-        self.juju_log.assert_called_with(
-            'ha_changed: hacluster subordinate is not fully clustered.'
-        )
-
-    @patch.object(relations, 'keystone_joined')
-    @patch.object(relations, 'CONFIGS')
-    def test_configure_https_enable_with_identity_service(self, configs, keystone_joined):
-        configs.complete_contexts = MagicMock()
-        configs.complete_contexts.return_value = ['https']
-        configs.write = MagicMock()
-        self.relation_ids.return_value = ['identity-service:0']
-        relations.configure_https()
-        cmd = ['a2ensite', 'openstack_https_frontend']
-        self.check_call.assert_called_with(cmd)
-        keystone_joined.assert_called_with(relation_id='identity-service:0')
-
-    @patch.object(relations, 'keystone_joined')
-    @patch.object(relations, 'CONFIGS')
-    def test_configure_https_disable_with_keystone_joined(self, configs, keystone_joined):
-        configs.complete_contexts = MagicMock()
-        configs.complete_contexts.return_value = ['']
-        configs.write = MagicMock()
-        self.relation_ids.return_value = ['identity-service:0']
-        relations.configure_https()
-        cmd = ['a2dissite', 'openstack_https_frontend']
-        self.check_call.assert_called_with(cmd)
-        keystone_joined.assert_called_with(relation_id='identity-service:0')
-
-    @patch.object(relations, 'image_service_joined')
-    @patch.object(relations, 'CONFIGS')
-    def test_configure_https_enable_with_image_service(self, configs, image_service_joined):
-        configs.complete_contexts = MagicMock()
-        configs.complete_contexts.return_value = ['https']
-        configs.write = MagicMock()
-        self.relation_ids.return_value = ['image-service:0']
-        relations.configure_https()
-        cmd = ['a2ensite', 'openstack_https_frontend']
-        self.check_call.assert_called_with(cmd)
-        image_service_joined.assert_called_with(relation_id='image-service:0')
-
-    @patch.object(relations, 'image_service_joined')
-    @patch.object(relations, 'CONFIGS')
-    def test_configure_https_disable_with_image_service(self, configs, image_service_joined):
-        configs.complete_contexts = MagicMock()
-        configs.complete_contexts.return_value = ['']
-        configs.write = MagicMock()
-        self.relation_ids.return_value = ['image-service:0']
-        relations.configure_https()
-        cmd = ['a2dissite', 'openstack_https_frontend']
-        self.check_call.assert_called_with(cmd)
-        image_service_joined.assert_called_with(relation_id='image-service:0')
-
+        self.assertTrue(self.juju_log.called)
+
+    @patch.object(relations, 'keystone_joined')
+    def test_ha_relation_changed_not_leader(self, joined):
+        self.relation_get.return_value = True
+        self.eligible_leader.return_value = False
+        relations.ha_relation_changed()
+        self.assertTrue(self.juju_log.called)
+        self.assertFalse(joined.called)
+
+    @patch.object(relations, 'image_service_joined')
+    @patch.object(relations, 'keystone_joined')
+    def test_ha_relation_changed_leader(self, ks_joined, image_joined):
+        self.relation_get.return_value = True
+        self.eligible_leader.return_value = True
+        self.relation_ids.side_effect = [['identity:0'], ['image:1']]
+        relations.ha_relation_changed()
+        ks_joined.assert_called_with('identity:0')
+        image_joined.assert_called_with('image:1')
+
+    @patch.object(relations, 'keystone_joined')
+    @patch.object(relations, 'CONFIGS')
+    def test_configure_https_enable_with_identity_service(
+            self, configs, keystone_joined):
+        configs.complete_contexts = MagicMock()
+        configs.complete_contexts.return_value = ['https']
+        configs.write = MagicMock()
+        self.relation_ids.return_value = ['identity-service:0']
+        relations.configure_https()
+        cmd = ['a2ensite', 'openstack_https_frontend']
+        self.check_call.assert_called_with(cmd)
+        keystone_joined.assert_called_with(relation_id='identity-service:0')
+
+    @patch.object(relations, 'keystone_joined')
+    @patch.object(relations, 'CONFIGS')
+    def test_configure_https_disable_with_keystone_joined(
+            self, configs, keystone_joined):
+        configs.complete_contexts = MagicMock()
+        configs.complete_contexts.return_value = ['']
+        configs.write = MagicMock()
+        self.relation_ids.return_value = ['identity-service:0']
+        relations.configure_https()
+        cmd = ['a2dissite', 'openstack_https_frontend']
+        self.check_call.assert_called_with(cmd)
+        keystone_joined.assert_called_with(relation_id='identity-service:0')
+
+    @patch.object(relations, 'image_service_joined')
+    @patch.object(relations, 'CONFIGS')
+    def test_configure_https_enable_with_image_service(
+            self, configs, image_service_joined):
+        configs.complete_contexts = MagicMock()
+        configs.complete_contexts.return_value = ['https']
+        configs.write = MagicMock()
+        self.relation_ids.return_value = ['image-service:0']
+        relations.configure_https()
+        cmd = ['a2ensite', 'openstack_https_frontend']
+        self.check_call.assert_called_with(cmd)
+        image_service_joined.assert_called_with(relation_id='image-service:0')
+
+    @patch.object(relations, 'image_service_joined')
+    @patch.object(relations, 'CONFIGS')
+    def test_configure_https_disable_with_image_service(
+            self, configs, image_service_joined):
+        configs.complete_contexts = MagicMock()
+        configs.complete_contexts.return_value = ['']
+        configs.write = MagicMock()
+        self.relation_ids.return_value = ['image-service:0']
+        relations.configure_https()
+        cmd = ['a2dissite', 'openstack_https_frontend']
+        self.check_call.assert_called_with(cmd)
+        image_service_joined.assert_called_with(relation_id='image-service:0')
+
+    @patch.object(relations, 'CONFIGS')
+    def test_relation_broken(self, configs):
+        relations.relation_broken()
+        self.assertTrue(configs.write_all.called)

=== modified file 'unit_tests/test_utils.py'
--- unit_tests/test_utils.py	2013-08-09 18:45:02 +0000
+++ unit_tests/test_utils.py	2013-10-20 18:36:29 +0000
@@ -93,7 +93,7 @@
         self.relation_data = relation_data
 
     def get(self, attr=None, unit=None, rid=None):
-        if attr == None:
+        if attr is None:
             return self.relation_data
         elif attr in self.relation_data:
             return self.relation_data[attr]