Code review comment for lp:~mwhudson/charms/trusty/openstack-dashboard/no-node

Revision history for this message
Liam Young (gnuoy) wrote :

Looks good except the test_install_hook unit test is failing because os_release needs mocking out. I'd recommend also adding two additional tests to check that the packages are being adjusted correctly, one for pre-icehouse and one for icehouse. The icehouse could look something like:

=== modified file 'unit_tests/test_horizon_hooks.py'
--- unit_tests/test_horizon_hooks.py 2014-04-10 16:19:57 +0000
+++ unit_tests/test_horizon_hooks.py 2014-07-25 09:26:32 +0000
@@ -28,6 +28,7 @@
     'unit_get',
     'log',
     'execd_preinstall',
+ 'os_release',
     'b64decode']

@@ -53,6 +54,15 @@
         self.apt_update.assert_called_with(fatal=True)
         self.apt_install.assert_called_with(['foo', 'bar'], fatal=True)

+ def test_install_hook_icehouse_pkgs(self):
+ self.os_release.return_value = 'icehouse'
+ self._call_hook('install')
+ self.configure_installation_source.assert_called_with('distro')
+ self.apt_update.assert_called_with(fatal=True)
+ for pkg in ['nodejs', 'node-less']:
+ self.assertFalse(pkg in self.filter_installed_packages.call_args)
+ self.apt_install.assert_called()
+
     @patch('charmhelpers.core.host.file_hash')
     @patch('charmhelpers.core.host.service')
     def test_upgrade_charm_hook(self, _service, _hash):

review: Needs Fixing

« Back to merge proposal