Merge lp:~adam-collard/charms/precise/reviewboard/trunk into lp:charms/reviewboard

[Adam Collard]

This branch adds unit test coverage to all of the implemented hooks and fixes a couple of bugs:

1. Lack of apt update before apt install meant (with out of date image files) units failed to install python-setuptools and therefore easy_install failed.
2. Ordering of relating and config setting (relating first and then setting config) meant a theoretical bug with failing to setup apache site.

[lp:~adam-collard/charms/precise/reviewboard/trunk]

31. By Adam Collard on 2014-06-28

Add test dependencies and setup target

[Charles Butler (lazypower)]

Adam,

Thanks for the updated test suite and the make targets.

+1 LGTM

review: Approve

=== added file 'Makefile'
--- Makefile	1970-01-01 00:00:00 +0000
+++ Makefile	2014-06-29 13:52:25 +0000
@@ -0,0 +1,12 @@
+.PHONY: check test lint setup
+
+test: setup
+	cd hooks; trial tests
+
+lint:
+	flake8 --exclude=charmhelpers hooks/
+
+check: test lint
+
+setup:
+	@sudo apt-get install python-twisted-core python-django python-mock
 
=== modified file 'README.md'
--- README.md	2014-06-10 18:53:50 +0000
+++ README.md	2014-06-29 13:52:25 +0000
@@ -35,11 +35,10 @@
 
 ## Known Limitations and Issues
 
-* Add test coverage
-* Add configuration for setting up initial repositories?
 * Add support for MySQL as a database backend.
  * Switch package installation between `python-psycopg2` and `python-mysqldb`.
  * Change configuration of rb-site command.
+* Add configuration for setting up initial repositories?
 * Optionally allow relation to separate
  [memcached](https://jujucharms.com/sidebar/search/precise/memcached/)
  charm instead of using local memcached.
 
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-06-11 18:52:18 +0000
+++ hooks/hooks.py	2014-06-29 13:52:25 +0000
@@ -7,7 +7,7 @@
 
 import charmhelpers.core.hookenv as hookenv
 from charmhelpers.core.host import service_restart, service_stop
-from charmhelpers.fetch import apt_install
+from charmhelpers.fetch import apt_install, apt_update
 
 
 hooks = hookenv.Hooks()
@@ -42,7 +42,8 @@
 
 
 @contextmanager
-def django_environ():
+def _django_environ():
+    """Context manager for setting up DJANGO_SETTINGS_MODULE."""
     cwd = os.getcwd()
     config_dir = "/var/www/reviewboard/conf"
     os.chdir(config_dir)
@@ -119,8 +120,9 @@
 @hooks.hook('db-relation-changed')
 def db_relation_changed():
     if not is_rb_site_installed():
-        if install_rb_site():
-            configure_apache2()
+        install_rb_site()
+    if is_rb_site_installed():
+        configure_apache2()
 
 
 @hooks.hook("website-relation-joined")
@@ -134,23 +136,24 @@
 def config_changed():
     config = hookenv.config()
     if not is_rb_site_installed():
-        if install_rb_site():
-            configure_apache2()
-        else:
-            # Failed to install rb site for some reason, try again later.
-            return
-    if config["admin-password"]:
-        with django_environ():
+        install_rb_site()
+    if is_rb_site_installed():
+        configure_apache2()
+    else:
+        # Failed to install rb site for some reason, try again later.
+        return
+    if config.get("admin-password"):
+        with _django_environ():
             from django.contrib.auth.management.commands import changepassword
             command = changepassword.Command()
             command._get_pass = lambda *args: config["admin-password"]
             command.execute("admin")
-    if config["host"]:
+    if config.get("host"):
         if config["host"] == "localhost":
             host = hookenv.unit_get("public-address")
         else:
             host = config["host"]
-        with django_environ():
+        with _django_environ():
             from django.contrib.sites.models import Site
             cur_site = Site.objects.get_current()
             cur_site.domain = host
@@ -159,6 +162,7 @@
 
 @hooks.hook("install")
 def install():
+    apt_update(fatal=True)
     apt_install(["apache2-mpm-prefork",
                  "libapache2-mod-wsgi",
                  "python-setuptools",
@@ -166,7 +170,7 @@
                  "memcached",
                  "python-memcache",
                  "patch",
-                 "python-psycopg2"])
+                 "python-psycopg2"], fatal=True)
     hookenv.log("Installing ReviewBoard")
     subprocess.check_call(["easy_install", "ReviewBoard"])
 
@@ -183,8 +187,12 @@
     service_stop("apache2")
 
 
-if __name__ == '__main__':
+def main(argv):
     try:
-        hooks.execute(sys.argv)
+        hooks.execute(argv)
     except hookenv.UnregisteredHookError as e:
         hookenv.log('Unknown hook {} - skipping.'.format(e))
+
+
+if __name__ == '__main__':
+    main(sys.argv)
 
=== added directory 'hooks/tests'
=== added file 'hooks/tests/__init__.py'
=== added file 'hooks/tests/test_hooks.py'
--- hooks/tests/test_hooks.py	1970-01-01 00:00:00 +0000
+++ hooks/tests/test_hooks.py	2014-06-29 13:52:25 +0000
@@ -0,0 +1,514 @@
+from contextlib import contextmanager
+import unittest
+
+from django.conf import settings
+settings.configure()
+
+from mock import patch
+
+import hooks
+
+
+class IsPostgresAvailableTest(unittest.TestCase):
+
+    def setUp(self):
+        super(IsPostgresAvailableTest, self).setUp()
+        log_patcher = patch("charmhelpers.core.hookenv.log")
+        self.mock_log = log_patcher.start()
+        self.addCleanup(log_patcher.stop)
+
+    @patch('charmhelpers.core.hookenv.is_relation_made')
+    def test_requires_relation_to_db(self, mock_is_relation_made):
+        """is_postgres_available depends on relation to PostgreSQL."""
+        mock_is_relation_made.return_value = False
+        self.assertFalse(hooks.is_postgres_available())
+        mock_is_relation_made.assert_called_with("db")
+        self.mock_log.assert_called_with("Missing relation to PostgreSQL")
+
+    @patch('charmhelpers.core.hookenv.is_relation_made')
+    @patch('charmhelpers.core.hookenv.local_unit')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    def test_requires_unit_in_allowed_units(
+            self, mock_relation_get, mock_local_unit, mock_is_relation_made):
+        """is_postgres_available checks if unit is in allowed-units."""
+        mock_is_relation_made.return_value = True
+        mock_local_unit.return_value = "test-unit/0"
+        mock_relation_get.return_value = "another-unit/0"
+        self.assertFalse(hooks.is_postgres_available())
+        self.mock_log.assert_called_with(
+            "Unit is not yet allowed (another-unit/0)")
+
+    @patch('charmhelpers.core.hookenv.is_relation_made')
+    @patch('charmhelpers.core.hookenv.local_unit')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    def test_checks_database_name(
+            self, mock_relation_get, mock_local_unit, mock_is_relation_made):
+        """If we are an allowed_unit then check the database name"""
+
+        def relation_get_side_effect(relation_key):
+            if relation_key == "allowed-units":
+                return "test-unit/0"
+            elif relation_key == "database":
+                return "not-reviewboard"
+            else:
+                assert False, "Unexpected relation key %s" % relation_key
+
+        mock_is_relation_made.return_value = True
+        mock_local_unit.return_value = "test-unit/0"
+        mock_relation_get.side_effect = relation_get_side_effect
+        self.assertFalse(hooks.is_postgres_available())
+        self.mock_log.assert_called_with(
+            "Database provided by PostgreSQL is "
+            "not 'reviewboard': 'not-reviewboard'")
+
+    @patch('charmhelpers.core.hookenv.is_relation_made')
+    @patch('charmhelpers.core.hookenv.local_unit')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    def test_everything_green(
+            self, mock_relation_get, mock_local_unit, mock_is_relation_made):
+        """When everything is OK, is_postgres_available returns True"""
+        def relation_get_side_effect(relation_key):
+            if relation_key == "allowed-units":
+                return "test-unit/0"
+            elif relation_key == "database":
+                return "reviewboard"
+            else:
+                assert False, "Unexpected relation key %s" % relation_key
+
+        mock_is_relation_made.return_value = True
+        mock_local_unit.return_value = "test-unit/0"
+        mock_relation_get.side_effect = relation_get_side_effect
+        self.assertTrue(hooks.is_postgres_available())
+        self.assertFalse(self.mock_log.called)
+
+
+class IsRbSiteInstalledTest(unittest.TestCase):
+
+    @patch("os.path.exists")
+    def test_looks_for_site_directory(self, mock_exists):
+        mock_exists.return_value = False
+        self.assertFalse(hooks.is_rb_site_installed())
+        mock_exists.assert_called_with("/var/www/reviewboard")
+        mock_exists.return_value = True
+        self.assertTrue(hooks.is_rb_site_installed())
+
+
+class InstallRbSiteTest(unittest.TestCase):
+
+    def setUp(self):
+        super(InstallRbSiteTest, self).setUp()
+        config_patcher = patch("charmhelpers.core.hookenv.config")
+        self.mock_config = config_patcher.start()
+        self.addCleanup(config_patcher.stop)
+
+        log_patcher = patch("charmhelpers.core.hookenv.log")
+        self.mock_log = log_patcher.start()
+        self.addCleanup(log_patcher.stop)
+
+        available_patcher = patch("hooks.is_postgres_available")
+        self.mock_available = available_patcher.start()
+        self.mock_available.return_value = True
+        self.addCleanup(available_patcher.stop)
+
+    def test_guarded_on_is_postgres_available(self):
+        self.mock_available.return_value = False
+        self.assertFalse(hooks.install_rb_site())
+        self.mock_available.assert_called_with()
+
+    def test_guarded_on_admin_password(self):
+        self.mock_config.return_value = {}
+        self.assertFalse(hooks.install_rb_site())
+        self.mock_log.assert_called_with(
+            "admin-password is not set, refusing to configure site")
+
+    @patch('charmhelpers.core.hookenv.unit_get')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    @patch('subprocess.check_call')
+    def test_host_info_from_config_defaults_to_public_address(
+            self, mock_call, mock_relation_get, mock_unit_get):
+        """
+        The host information is a config value which defaults to
+        "localhost". When it is "localhost" that means use the public
+        address of the unit.
+        """
+        self.mock_config.return_value = {"admin-password": "a-password"}
+        mock_unit_get.return_value = "8.8.8.8"
+        mock_relation_get.return_value = {"host": "psql.example.com",
+                                          "port": "5432",
+                                          "user": "rb-db-user",
+                                          "password": "hunter2"}
+        self.assertTrue(hooks.install_rb_site())
+        args_list, kwargs_list = mock_call.call_args
+        self.assertIn("--domain-name=8.8.8.8", args_list[0])
+        self.mock_log.assert_any_call(
+            "Installing rb-site for 8.8.8.8 in /var/www/reviewboard")
+
+    @patch('charmhelpers.core.hookenv.unit_get')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    @patch('subprocess.check_call')
+    def test_db_host_and_port_used_from_relation(
+            self, mock_call, mock_relation_get, mock_unit_get):
+        self.mock_config.return_value = {"admin-password": "a-password",
+                                         "host": "rb.example.com"}
+        mock_relation_get.return_value = {"host": "psql.example.com",
+                                          "port": "5432",
+                                          "user": "rb-db-user",
+                                          "password": "hunter2"}
+        self.assertTrue(hooks.install_rb_site())
+        args_list, kwargs_list = mock_call.call_args
+        self.assertIn("--db-host=psql.example.com:5432", args_list[0])
+
+    @patch('charmhelpers.core.hookenv.unit_get')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    @patch('subprocess.check_call')
+    def test_db_user_used_from_relation(
+            self, mock_call, mock_relation_get, mock_unit_get):
+        self.mock_config.return_value = {"admin-password": "a-password",
+                                         "host": "rb.example.com"}
+        mock_relation_get.return_value = {"host": "psql.example.com",
+                                          "port": "5432",
+                                          "user": "rb-db-user",
+                                          "password": "hunter2"}
+        self.assertTrue(hooks.install_rb_site())
+        args_list, kwargs_list = mock_call.call_args
+        self.assertIn("--db-user=rb-db-user", args_list[0])
+
+    @patch('charmhelpers.core.hookenv.unit_get')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    @patch('subprocess.check_call')
+    def test_db_password_used_from_relation(
+            self, mock_call, mock_relation_get, mock_unit_get):
+        self.mock_config.return_value = {"admin-password": "a-password",
+                                         "host": "rb.example.com"}
+        mock_relation_get.return_value = {"host": "psql.example.com",
+                                          "port": "5432",
+                                          "user": "rb-db-user",
+                                          "password": "hunter2"}
+        self.assertTrue(hooks.install_rb_site())
+        args_list, kwargs_list = mock_call.call_args
+        self.assertIn("--db-pass=hunter2", args_list[0])
+
+    @patch('charmhelpers.core.hookenv.unit_get')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    @patch('subprocess.check_call')
+    def test_admin_password_used_from_config(
+            self, mock_call, mock_relation_get, mock_unit_get):
+        self.mock_config.return_value = {"admin-password": "a-password",
+                                         "host": "rb.example.com"}
+        mock_relation_get.return_value = {"host": "psql.example.com",
+                                          "port": "5432",
+                                          "user": "rb-db-user",
+                                          "password": "hunter2"}
+        self.assertTrue(hooks.install_rb_site())
+        args_list, kwargs_list = mock_call.call_args
+        self.assertIn("--admin-password=a-password", args_list[0])
+
+    @patch('charmhelpers.core.hookenv.unit_get')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    @patch('subprocess.check_call')
+    def test_log_confirmation(
+            self, mock_call, mock_relation_get, mock_unit_get):
+        self.mock_config.return_value = {"admin-password": "a-password",
+                                         "host": "rb.example.com"}
+        mock_relation_get.return_value = {"host": "psql.example.com",
+                                          "port": "5432",
+                                          "user": "rb-db-user",
+                                          "password": "hunter2"}
+        self.assertTrue(hooks.install_rb_site())
+        self.mock_log.assert_called_with("Installed rb-site!")
+
+    @patch('charmhelpers.core.hookenv.unit_get')
+    @patch('charmhelpers.core.hookenv.relation_get')
+    @patch('subprocess.check_call')
+    def test_full_rb_site_install_command(
+            self, mock_call, mock_relation_get, mock_unit_get):
+        self.mock_config.return_value = {"admin-password": "a-password",
+                                         "host": "rb.example.com"}
+        mock_relation_get.return_value = {"host": "psql.example.com",
+                                          "port": "5432",
+                                          "user": "rb-db-user",
+                                          "password": "hunter2"}
+        self.assertTrue(hooks.install_rb_site())
+        args_list, kwargs_list = mock_call.call_args
+        full_command_args = [
+            "rb-site",
+            "install",
+            "--noinput",
+            "--domain-name=rb.example.com",
+            "--db-type=postgresql",
+            "--db-name=reviewboard",
+            "--db-host=psql.example.com:5432",
+            "--db-user=rb-db-user",
+            "--db-pass=hunter2",
+            "--cache-type=memcached",
+            "--web-server-type=apache",
+            "--web-server-port=80",
+            "--python-loader=wsgi",
+            "--admin-user=admin",
+            "--admin-password=a-password",
+            "/var/www/reviewboard"]
+
+        self.assertEqual(full_command_args, args_list[0])
+
+
+class ConfigureApache2Test(unittest.TestCase):
+
+    def setUp(self):
+        super(ConfigureApache2Test, self).setUp()
+        log_patcher = patch("charmhelpers.core.hookenv.log")
+        self.mock_log = log_patcher.start()
+        self.addCleanup(log_patcher.stop)
+
+        call_patcher = patch("subprocess.check_call")
+        self.mock_call = call_patcher.start()
+        self.addCleanup(call_patcher.stop)
+
+        restart_patcher = patch("hooks.service_restart")
+        self.mock_restart = restart_patcher.start()
+        self.addCleanup(restart_patcher.stop)
+
+    def test_copies_wsgi_conf(self):
+        hooks.configure_apache2()
+        self.mock_call.assert_any_call(
+            ["cp", "/var/www/reviewboard/conf/apache-wsgi.conf",
+             "/etc/apache2/sites-available/reviewboard.conf"])
+
+    def test_disables_default_site(self):
+        hooks.configure_apache2()
+        self.mock_call.assert_any_call(["a2dissite", "000-default"])
+
+    def test_enables_reviewboard_site(self):
+        hooks.configure_apache2()
+        self.mock_call.assert_any_call(["a2ensite", "reviewboard.conf"])
+
+    def test_chowns_dirs(self):
+        hooks.configure_apache2()
+        for leaf in ["htdocs/media/uploaded", "htdocs/media/ext",
+                     "htdocs/static/ext", "data"]:
+            self.mock_call.assert_any_call(
+                ["chown", "-R", "www-data", "/var/www/reviewboard/%s" % leaf])
+
+    def test_restarts_apache2(self):
+        hooks.configure_apache2()
+        self.mock_restart.assert_called_with("apache2")
+
+    def test_log_confirmation(self):
+        hooks.configure_apache2()
+        self.mock_log.assert_called_with("Apache configured!")
+        self.mock_log.assert_any_call("Configuring Apache...")
+
+
+class DbRelationJoinedTest(unittest.TestCase):
+
+    @patch('charmhelpers.core.hookenv.relation_set')
+    def test_requests_database(self, mock_relation_set):
+        hooks.db_relation_joined()
+        mock_relation_set.assert_called_with(
+            relation_settings={"database": "reviewboard"})
+
+
+class DbRelationChangedTest(unittest.TestCase):
+
+    @patch('hooks.is_rb_site_installed')
+    @patch('hooks.install_rb_site')
+    @patch('hooks.configure_apache2')
+    def test_configures_apache2_if_rb_needs_installing(
+            self, mock_configure, mock_install, mock_installed):
+
+        def mock_installed_side_effect():
+            mock_installed.side_effect = lambda: True
+            return False
+
+        mock_installed.side_effect = mock_installed_side_effect
+        mock_install.return_value = True
+        hooks.db_relation_changed()
+        mock_install.assert_called_with()
+        mock_configure.assert_called_with()
+
+    @patch('hooks.is_rb_site_installed')
+    @patch('hooks.configure_apache2')
+    def test_configures_apache2_if_rb_already_installed(
+            self, mock_configure, mock_installed):
+        mock_installed.return_value = True
+        hooks.db_relation_changed()
+        mock_configure.assert_called_with()
+
+
+class WebsiteRelationJoinedTest(unittest.TestCase):
+
+    @patch('charmhelpers.core.hookenv.unit_get')
+    @patch('charmhelpers.core.hookenv.relation_set')
+    def test_requests_database(self, mock_relation_set, mock_unit_get):
+        mock_unit_get.return_value = "127.0.0.1"
+        hooks.website_relation_joined()
+        mock_relation_set.assert_called_with(
+            relation_settings={"hostname": "127.0.0.1", "port": 80})
+
+
+class ConfigChangedTest(unittest.TestCase):
+
+    def setUp(self):
+        super(ConfigChangedTest, self).setUp()
+        config_patcher = patch("charmhelpers.core.hookenv.config")
+        self.mock_config = config_patcher.start()
+        self.mock_config.return_value = {}
+        self.addCleanup(config_patcher.stop)
+
+    @contextmanager
+    def _fake_django_environ():
+        yield
+
+    @patch('hooks.is_rb_site_installed')
+    @patch('hooks.install_rb_site')
+    @patch('hooks.configure_apache2')
+    def test_configures_apache2_if_rb_needs_installing(
+            self, mock_configure, mock_install, mock_installed):
+
+        def mock_installed_side_effect():
+            mock_installed.side_effect = lambda: True
+            return False
+
+        mock_installed.side_effect = mock_installed_side_effect
+        mock_install.return_value = True
+        hooks.config_changed()
+        mock_install.assert_called_with()
+        mock_configure.assert_called_with()
+
+    @patch('hooks.is_rb_site_installed')
+    @patch('hooks.configure_apache2')
+    def test_configures_apache2_if_rb_already_installed(
+            self, mock_configure, mock_installed):
+        mock_installed.return_value = True
+        hooks.config_changed()
+        mock_configure.assert_called_with()
+
+    @patch('hooks.is_rb_site_installed')
+    @patch('hooks.configure_apache2')
+    @patch('hooks._django_environ', _fake_django_environ)
+    @patch("django.contrib.auth.management.commands.changepassword.Command")
+    def test_changes_password_for_admin_user(
+            self, mock_command, mock_configure, mock_installed):
+        mock_installed.return_value = True
+        self.mock_config.return_value = {"admin-password": "a-password"}
+        instance = mock_command.return_value
+        hooks.config_changed()
+        instance.execute.assert_called_with("admin")
+        self.assertEqual(instance._get_pass(), "a-password")
+
+    @patch('hooks.is_rb_site_installed')
+    @patch('hooks.configure_apache2')
+    @patch('hooks._django_environ', _fake_django_environ)
+    @patch("django.contrib.sites.models.Site")
+    def test_changes_host(self, mock_site, mock_configure, mock_installed):
+        mock_installed.return_value = True
+        self.mock_config.return_value = {"host": "rb.example.com"}
+        instance = mock_site.objects.get_current.return_value
+        hooks.config_changed()
+        instance.save.assert_called_with()
+        self.assertEqual(instance.domain, "rb.example.com")
+
+    @patch('hooks.is_rb_site_installed')
+    @patch('hooks.configure_apache2')
+    @patch('charmhelpers.core.hookenv.unit_get')
+    @patch('hooks._django_environ', _fake_django_environ)
+    @patch("django.contrib.sites.models.Site")
+    def test_localhost_means_public_address(
+            self, mock_site, mock_unit_get, mock_configure, mock_installed):
+        mock_installed.return_value = True
+        self.mock_config.return_value = {"host": "localhost"}
+        mock_unit_get.return_value = "8.8.8.8"
+        instance = mock_site.objects.get_current.return_value
+        hooks.config_changed()
+        instance.save.assert_called_with()
+        self.assertEqual(instance.domain, "8.8.8.8")
+
+
+class InstallTest(unittest.TestCase):
+
+    def setUp(self):
+        super(InstallTest, self).setUp()
+        log_patcher = patch("charmhelpers.core.hookenv.log")
+        self.mock_log = log_patcher.start()
+        self.addCleanup(log_patcher.stop)
+
+        update_patcher = patch("hooks.apt_update")
+        self.mock_update = update_patcher.start()
+        self.addCleanup(update_patcher.stop)
+
+        install_patcher = patch("hooks.apt_install")
+        self.mock_install = install_patcher.start()
+        self.addCleanup(install_patcher.stop)
+
+        call_patcher = patch("subprocess.check_call")
+        self.mock_call = call_patcher.start()
+        self.addCleanup(call_patcher.stop)
+
+    def test_apt_packages(self):
+        hooks.install()
+        self.mock_install.assert_called_with(
+            ["apache2-mpm-prefork",
+             "libapache2-mod-wsgi",
+             "python-setuptools",
+             "python-dev",
+             "memcached",
+             "python-memcache",
+             "patch",
+             "python-psycopg2"], fatal=True)
+
+    def test_apt_update(self):
+        hooks.install()
+        self.mock_update.assert_called_with(fatal=True)
+
+    def test_easy_install(self):
+        hooks.install()
+        self.mock_call.assert_called_with(["easy_install", "ReviewBoard"])
+
+    def test_log(self):
+        hooks.install()
+        self.mock_log.assert_called_with("Installing ReviewBoard")
+
+
+class StartTest(unittest.TestCase):
+
+    @patch("hooks.service_restart")
+    @patch('charmhelpers.core.hookenv.open_port')
+    def test_restarts_apache(self, mock_open_port, mock_restart):
+        hooks.start()
+        mock_restart.assert_called_with("apache2")
+
+    @patch("hooks.service_restart")
+    @patch('charmhelpers.core.hookenv.open_port')
+    def test_opens_port_80(self, mock_open_port, mock_restart):
+        hooks.start()
+        mock_open_port.assert_called_with(80)
+
+
+class StopTest(unittest.TestCase):
+
+    @patch("hooks.service_stop")
+    @patch('charmhelpers.core.hookenv.close_port')
+    def test_stops_apache(self, mock_close_port, mock_stop):
+        hooks.stop()
+        mock_stop.assert_called_with("apache2")
+
+    @patch("hooks.service_stop")
+    @patch('charmhelpers.core.hookenv.close_port')
+    def test_closes_port_80(self, mock_close_port, mock_stop):
+        hooks.stop()
+        mock_close_port.assert_called_with(80)
+
+
+class MainTest(unittest.TestCase):
+
+    @patch("hooks.hooks")
+    def test_executes_sys_argv(self, mock_hooks):
+        hooks.main(["arg", "v"])
+        mock_hooks.execute.assert_called_with(["arg", "v"])
+
+    @patch("hooks.hooks")
+    @patch("charmhelpers.core.hookenv.log")
+    def test_logs_about_unknown_hooks(self, mock_log, mock_hooks):
+        mock_hooks.execute.side_effect =\
+            hooks.hookenv.UnregisteredHookError("missing-hook")
+        hooks.main(["arg", "v"])
+        mock_log.assert_called_with("Unknown hook missing-hook - skipping.")