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

Proposed by Paul Collins
Status: Merged
Approved by: Haw Loeung
Approved revision: 3b3c8f072780da5e36d949e8678cda1f969c9a9b
Merged at revision: cc09bd3e25a29290590b254758b70a4c1fec33bd
Proposed branch: ~pjdc/ubuntu-mirror-charm/+git/ubuntu-mirror-charm:testability
Merge into: ubuntu-mirror-charm:master
Diff against target: 198 lines (+88/-11)
5 files modified
.gitignore (+5/-0)
Makefile (+14/-0)
hooks/Config.py (+3/-3)
hooks/hooks.py (+19/-8)
tox.ini (+47/-0)
Reviewer Review Type Date Requested Status
Haw Loeung +1 Approve
Canonical IS Reviewers Pending
Review via email: mp+388433@code.launchpad.net

Commit message

add lint and unittest, fix lint errors

To post a comment you must log in.
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

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

Revision history for this message
Haw Loeung (hloeung) wrote :

LGTM

review: Approve (+1)
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision cc09bd3e25a29290590b254758b70a4c1fec33bd

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/.gitignore b/.gitignore
2index b25c15b..490cc43 100644
3--- a/.gitignore
4+++ b/.gitignore
5@@ -1 +1,6 @@
6 *~
7+*.charm
8+.tox
9+.coverage
10+__pycache__
11+build
12diff --git a/Makefile b/Makefile
13new file mode 100644
14index 0000000..4f10863
15--- /dev/null
16+++ b/Makefile
17@@ -0,0 +1,14 @@
18+lint:
19+ @echo "Running flake8"
20+ @tox -e lint
21+
22+unittest:
23+ @tox -e unit
24+
25+test: lint unittest
26+
27+clean:
28+ @echo "Cleaning files"
29+ @git clean -fXd
30+
31+.PHONY: lint test unittest clean
32diff --git a/hooks/Config.py b/hooks/Config.py
33index d6e8ba3..a729ba6 100755
34--- a/hooks/Config.py
35+++ b/hooks/Config.py
36@@ -59,7 +59,7 @@ class Config:
37 def archive_logs_for(self, service):
38 try:
39 ret = config("archive_logs_" + service)
40- except:
41+ except Exception:
42 ret = False
43 return ret
44
45@@ -104,11 +104,11 @@ class Config:
46 def logdir(self, service):
47 try:
48 logdir = str(config(service + "_logdir"))
49- except:
50+ except Exception:
51 logdir = None
52 return logdir
53
54- def mirror_map(self, role):
55+ def mirror_map(self, role): # noqa: C901
56 if role in self._mirror_map:
57 return self._mirror_map[role]
58
59diff --git a/hooks/hooks.py b/hooks/hooks.py
60index 7dfa42a..fb28d13 100755
61--- a/hooks/hooks.py
62+++ b/hooks/hooks.py
63@@ -28,7 +28,17 @@ from charmhelpers.core.host import (
64 user_exists,
65 write_file,
66 )
67-from charmhelpers.core.hookenv import Hooks, log, charm_dir, relation_set, local_unit, is_relation_made, relation_id, relation_ids, status_set
68+from charmhelpers.core.hookenv import (
69+ Hooks,
70+ charm_dir,
71+ is_relation_made,
72+ local_unit,
73+ log,
74+ relation_id,
75+ relation_ids,
76+ relation_set,
77+ status_set,
78+)
79 from charmhelpers.core import sysctl
80 from charmhelpers.fetch import apt_install
81 from charmhelpers.payload.execd import execd_run
82@@ -42,7 +52,7 @@ execd_dir = os.path.join(charm_dir(), 'exec.d')
83 conf = Config()
84
85 apache_tls_settings = {
86- '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',
87+ '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
88 'ssl_honor_cipher_order': 'on',
89 # per IS policy, TLS 1.2 is the minimum acceptable version
90 'ssl_protocol': 'ALL -SSLv2 -SSLv3 -TLSv1 -TLSv1.1',
91@@ -185,7 +195,7 @@ def get_ssh_keyfile(username):
92 # Creation of any necessary destination directories is the responsibility
93 # of the sync script specified in ${mirror}_command
94 #
95-def configure_rsync_client():
96+def configure_rsync_client(): # noqa: C901
97 log("CHARM: Configuring rsync client")
98 mirror_userinfo = conf.mirror_userinfo()
99 mirror_confdir = os.path.join(mirror_userinfo.pw_dir, "mirror-config")
100@@ -315,7 +325,7 @@ def get_role_config(roles, hostname, role):
101 #
102 # Set up apache config files - adding or removing vhosts as needed
103 #
104-def configure_apache():
105+def configure_apache(): # noqa: C901
106 log("CHARM: Configuring apache")
107 ensure_package_status(service_affecting_packages, conf.package_status())
108 apache_dir = "/etc/apache2"
109@@ -452,7 +462,7 @@ def configure_apache():
110 # Set up the mpm config
111 # 2.4 defaults to event
112 # Returns True is something changed, False otherwise
113-def configure_apache_mpm():
114+def configure_apache_mpm(): # noqa: C901
115 log("CHARM: Configuring mpm")
116 restart_required = False
117 apache_version = get_pkg_version("apache2")[0:3]
118@@ -592,7 +602,7 @@ def configure_triggers():
119 write_file(keyfile, contents)
120
121
122-def configure_nrpe():
123+def configure_nrpe(): # noqa: C901
124 if not conf.nagios_relation_ok():
125 log("CHARM: Nagios relation not established - not configuring nrpe")
126 return
127@@ -684,7 +694,8 @@ def configure_nrpe():
128
129 # Clear out any old role checks
130 # Making (fairly) sure we only touch files we manage
131- valid_beginnings = role_checks.keys() + ["service__{}_".format(conf.nagios_hostname()) + s for s in role_checks.keys()]
132+ valid_beginnings = role_checks.keys() + ["service__{}_".format(conf.nagios_hostname()) + s
133+ for s in role_checks.keys()]
134 for d in [nagios_etcdir, nagios_exportdir]:
135 enabled_checks = []
136 for f in os.listdir(d):
137@@ -727,7 +738,7 @@ def configure_nrpe():
138
139 # RT#79518: Add prompt-critical check for all disks; taken from puppet.
140 check_all_disks = juju_header()
141- 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"
142+ 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
143 write_file(os.path.join(nagios_etcdir, 'check_all_disks_promptcrit.cfg'),
144 check_all_disks)
145
146diff --git a/tox.ini b/tox.ini
147new file mode 100644
148index 0000000..7898f03
149--- /dev/null
150+++ b/tox.ini
151@@ -0,0 +1,47 @@
152+[tox]
153+skipsdist=True
154+envlist = unit, functional
155+skip_missing_interpreters = True
156+
157+[testenv]
158+basepython = python2
159+setenv =
160+ PYTHONPATH = {toxinidir}/hooks
161+
162+[testenv:unit]
163+commands =
164+ pytest --ignore mod --ignore {toxinidir}/tests/functional \
165+ {posargs:-v --cov=src --cov-report=term-missing --cov-branch}
166+deps = -r{toxinidir}/tests/unit/requirements.txt
167+ -r{toxinidir}/requirements.txt
168+setenv =
169+ PYTHONPATH={toxinidir}/src:{toxinidir}/build/lib:{toxinidir}/build/venv
170+ TZ=UTC
171+
172+[testenv:functional]
173+passenv =
174+ HOME
175+ JUJU_REPOSITORY
176+ PATH
177+commands =
178+ pytest -v --ignore mod --ignore {toxinidir}/tests/unit {posargs}
179+deps = -r{toxinidir}/tests/functional/requirements.txt
180+ -r{toxinidir}/requirements.txt
181+
182+[testenv:black]
183+commands = black --skip-string-normalization --line-length=120 src/ tests/
184+deps = black
185+
186+[testenv:lint]
187+commands = flake8 --exclude hooks/charmhelpers hooks/ tests/
188+# Pin flake8 to 3.7.9 to match focal
189+deps =
190+ flake8==3.7.9
191+
192+[flake8]
193+exclude =
194+ .git,
195+ __pycache__,
196+ .tox,
197+max-line-length = 120
198+max-complexity = 10

Subscribers

People subscribed via source and target branches