Merge ~pjdc/ubuntu-mirror-charm/+git/ubuntu-mirror-charm:testability into ubuntu-mirror-charm:master

[Paul Collins]

add lint and unittest, fix lint errors

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Haw Loeung (hloeung)]

LGTM

review: Approve (+1)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision cc09bd3e25a29290590b254758b70a4c1fec33bd

diff --git a/.gitignore b/.gitignore
index b25c15b..490cc43 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1 +1,6 @@
 *~
+*.charm
+.tox
+.coverage
+__pycache__
+build
diff --git a/Makefile b/Makefile
 new file mode 100644
index 0000000..4f10863
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,14 @@
+lint:
+	@echo "Running flake8"
+	@tox -e lint
+
+unittest:
+	@tox -e unit
+
+test: lint unittest
+
+clean:
+	@echo "Cleaning files"
+	@git clean -fXd
+
+.PHONY: lint test unittest clean
diff --git a/hooks/Config.py b/hooks/Config.py
index d6e8ba3..a729ba6 100755
--- a/hooks/Config.py
+++ b/hooks/Config.py
@@ -59,7 +59,7 @@ class Config:
     def archive_logs_for(self, service):
         try:
             ret = config("archive_logs_" + service)
-        except:
+        except Exception:
             ret = False
         return ret
 
@@ -104,11 +104,11 @@ class Config:
     def logdir(self, service):
         try:
             logdir = str(config(service + "_logdir"))
-        except:
+        except Exception:
             logdir = None
         return logdir
 
-    def mirror_map(self, role):
+    def mirror_map(self, role):  # noqa: C901
         if role in self._mirror_map:
             return self._mirror_map[role]
 
diff --git a/hooks/hooks.py b/hooks/hooks.py
index 7dfa42a..fb28d13 100755
--- a/hooks/hooks.py
+++ b/hooks/hooks.py
@@ -28,7 +28,17 @@ from charmhelpers.core.host import (
     user_exists,
     write_file,
 )
-from charmhelpers.core.hookenv import Hooks, log, charm_dir, relation_set, local_unit, is_relation_made, relation_id, relation_ids, status_set
+from charmhelpers.core.hookenv import (
+    Hooks,
+    charm_dir,
+    is_relation_made,
+    local_unit,
+    log,
+    relation_id,
+    relation_ids,
+    relation_set,
+    status_set,
+)
 from charmhelpers.core import sysctl
 from charmhelpers.fetch import apt_install
 from charmhelpers.payload.execd import execd_run
@@ -42,7 +52,7 @@ execd_dir = os.path.join(charm_dir(), 'exec.d')
 conf = Config()
 
 apache_tls_settings = {
-    'ssl_cipher_suite': 'EECDH+AESGCM+AES128:EDH+AESGCM+AES128:EECDH+AES128:EDH+AES128:ECDH+AESGCM+AES128:aRSA+AESGCM+AES128:ECDH+AES128:DH+AES128:aRSA+AES128:EECDH+AESGCM:EDH+AESGCM:EECDH:EDH:ECDH+AESGCM:aRSA+AESGCM:ECDH:DH:aRSA:HIGH:!MEDIUM:!aNULL:!NULL:!LOW:!3DES:!DSS:!EXP:!PSK:!SRP',
+    'ssl_cipher_suite': 'EECDH+AESGCM+AES128:EDH+AESGCM+AES128:EECDH+AES128:EDH+AES128:ECDH+AESGCM+AES128:aRSA+AESGCM+AES128:ECDH+AES128:DH+AES128:aRSA+AES128:EECDH+AESGCM:EDH+AESGCM:EECDH:EDH:ECDH+AESGCM:aRSA+AESGCM:ECDH:DH:aRSA:HIGH:!MEDIUM:!aNULL:!NULL:!LOW:!3DES:!DSS:!EXP:!PSK:!SRP',  # noqa: E501
     'ssl_honor_cipher_order': 'on',
     # per IS policy, TLS 1.2 is the minimum acceptable version
     'ssl_protocol': 'ALL -SSLv2 -SSLv3 -TLSv1 -TLSv1.1',
@@ -185,7 +195,7 @@ def get_ssh_keyfile(username):
 # Creation of any necessary destination directories is the responsibility
 # of the sync script specified in ${mirror}_command
 #
-def configure_rsync_client():
+def configure_rsync_client():  # noqa: C901
     log("CHARM: Configuring rsync client")
     mirror_userinfo = conf.mirror_userinfo()
     mirror_confdir = os.path.join(mirror_userinfo.pw_dir, "mirror-config")
@@ -315,7 +325,7 @@ def get_role_config(roles, hostname, role):
 #
 # Set up apache config files - adding or removing vhosts as needed
 #
-def configure_apache():
+def configure_apache():  # noqa: C901
     log("CHARM: Configuring apache")
     ensure_package_status(service_affecting_packages, conf.package_status())
     apache_dir = "/etc/apache2"
@@ -452,7 +462,7 @@ def configure_apache():
 # Set up the mpm config
 # 2.4 defaults to event
 # Returns True is something changed, False otherwise
-def configure_apache_mpm():
+def configure_apache_mpm():  # noqa: C901
     log("CHARM: Configuring mpm")
     restart_required = False
     apache_version = get_pkg_version("apache2")[0:3]
@@ -592,7 +602,7 @@ def configure_triggers():
     write_file(keyfile, contents)
 
 
-def configure_nrpe():
+def configure_nrpe():  # noqa: C901
     if not conf.nagios_relation_ok():
         log("CHARM: Nagios relation not established - not configuring nrpe")
         return
@@ -684,7 +694,8 @@ def configure_nrpe():
 
     # Clear out any old role checks
     # Making (fairly) sure we only touch files we manage
-    valid_beginnings = role_checks.keys() + ["service__{}_".format(conf.nagios_hostname()) + s for s in role_checks.keys()]
+    valid_beginnings = role_checks.keys() + ["service__{}_".format(conf.nagios_hostname()) + s
+                                             for s in role_checks.keys()]
     for d in [nagios_etcdir, nagios_exportdir]:
         enabled_checks = []
         for f in os.listdir(d):
@@ -727,7 +738,7 @@ def configure_nrpe():
 
     # RT#79518: Add prompt-critical check for all disks; taken from puppet.
     check_all_disks = juju_header()
-    check_all_disks += "command[check_all_disks_promptcrit]=/usr/lib/nagios/plugins/check_disk -L -w 2% -c 1% -K 4% -A -i '^/run/lxcfs' -i '^/sys' -i '^/snap'\n"
+    check_all_disks += "command[check_all_disks_promptcrit]=/usr/lib/nagios/plugins/check_disk -L -w 2% -c 1% -K 4% -A -i '^/run/lxcfs' -i '^/sys' -i '^/snap'\n"  # noqa: E501
     write_file(os.path.join(nagios_etcdir, 'check_all_disks_promptcrit.cfg'),
                check_all_disks)
 
diff --git a/tox.ini b/tox.ini
 new file mode 100644
index 0000000..7898f03
--- /dev/null
+++ b/tox.ini
@@ -0,0 +1,47 @@
+[tox]
+skipsdist=True
+envlist = unit, functional
+skip_missing_interpreters = True
+
+[testenv]
+basepython = python2
+setenv =
+  PYTHONPATH = {toxinidir}/hooks
+
+[testenv:unit]
+commands =
+    pytest --ignore mod --ignore {toxinidir}/tests/functional \
+      {posargs:-v  --cov=src --cov-report=term-missing --cov-branch}
+deps = -r{toxinidir}/tests/unit/requirements.txt
+       -r{toxinidir}/requirements.txt
+setenv =
+  PYTHONPATH={toxinidir}/src:{toxinidir}/build/lib:{toxinidir}/build/venv
+  TZ=UTC
+
+[testenv:functional]
+passenv =
+  HOME
+  JUJU_REPOSITORY
+  PATH
+commands =
+	pytest -v --ignore mod --ignore {toxinidir}/tests/unit {posargs}
+deps = -r{toxinidir}/tests/functional/requirements.txt
+       -r{toxinidir}/requirements.txt
+
+[testenv:black]
+commands = black --skip-string-normalization --line-length=120 src/ tests/
+deps = black
+
+[testenv:lint]
+commands = flake8 --exclude hooks/charmhelpers hooks/ tests/
+# Pin flake8 to 3.7.9 to match focal
+deps =
+    flake8==3.7.9
+
+[flake8]
+exclude =
+    .git,
+    __pycache__,
+    .tox,
+max-line-length = 120
+max-complexity = 10