Merge ~hloeung/jenkins-agent-charm:cleanup into jenkins-agent-charm:master

[Haw Loeung]

Add and normalise python layout with black

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

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

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

Unable to determine commit message from repository - please click "Set commit message" and enter the commit message manually.

[Stuart Bishop (stub)]

Yup, all good.

review: Approve

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

Change successfully merged at revision 63a4a4bf7be28b85f2d972b06339c498cc703082

diff --git a/Makefile b/Makefile
index 43470ed..c0b3852 100644
--- a/Makefile
+++ b/Makefile
@@ -10,6 +10,8 @@ help:
 	@echo ""
 
 lint:
+	@echo "Normalising python layout with black."
+	@tox -e black
 	@echo "Running flake8"
 	@tox -e lint
 
@@ -25,6 +27,8 @@ clean:
 	@echo "Cleaning files"
 	@rm -rf ./.tox
 	@rm -rf ./.pytest_cache
+	@rm -rf ./tests/unit/__pycache__ ./reactive/__pycache__ ./lib/__pycache__
+	@rm -rf ./.coverage ./.unit-state.db
 
 # The targets below don't depend on a file
 .PHONY: lint test unittest functionaltest clean help
diff --git a/reactive/jenkins_slave.py b/reactive/jenkins_slave.py
index 29bf58f..9c2089b 100755
--- a/reactive/jenkins_slave.py
+++ b/reactive/jenkins_slave.py
@@ -215,20 +215,18 @@ def file_to_units(local_path, unit_path, perms=None, owner='root', group='root')
             file_perms = 0o644
 
     with open(local_path, 'r') as fh:
-        host.write_file(
-            path=unit_path,
-            content=fh.read().encode(),
-            owner=owner,
-            group=group,
-            perms=file_perms,
-        )
+        host.write_file(path=unit_path, content=fh.read().encode(), owner=owner, group=group, perms=file_perms)
 
 
-def write_default_conf(master_url=None, owner='root', group='root',
-                       conf_path='/etc/default/jenkins-slave'):
+def write_default_conf(master_url=None, owner='root', group='root', conf_path='/etc/default/jenkins-slave'):
     templates_dir = os.path.join(hookenv.charm_dir(), 'templates')
     slave_host = hookenv.local_unit().replace('/', '-')
-    templating.render('jenkins-slave-default', conf_path,
-                      {'master_url': master_url, 'slave_host': slave_host},
-                      owner=owner, group=group, perms=0o444,
-                      templates_dir=templates_dir)
+    templating.render(
+        'jenkins-slave-default',
+        conf_path,
+        {'master_url': master_url, 'slave_host': slave_host},
+        owner=owner,
+        group=group,
+        perms=0o444,
+        templates_dir=templates_dir,
+    )
diff --git a/tests/unit/test_jenkins_slave.py b/tests/unit/test_jenkins_slave.py
index 535bc95..b33f1ed 100644
--- a/tests/unit/test_jenkins_slave.py
+++ b/tests/unit/test_jenkins_slave.py
@@ -71,8 +71,13 @@ class TestSetDefaultConf(unittest.TestCase):
         patcher = mock.patch('os.uname')
         self.mock_log = patcher.start()
         self.addCleanup(patcher.stop)
-        self.mock_log.return_value = ('Linux', 'mock-jenkins-slave', '4.15.0-46-generic',
-                                      '#49-Ubuntu SMP Wed Feb 6 09:33:07 UTC 2019', 'x86_64')
+        self.mock_log.return_value = (
+            'Linux',
+            'mock-jenkins-slave',
+            '4.15.0-46-generic',
+            '#49-Ubuntu SMP Wed Feb 6 09:33:07 UTC 2019',
+            'x86_64',
+        )
 
         patcher = mock.patch('os.cpu_count')
         self.mock_log = patcher.start()
@@ -89,9 +94,11 @@ class TestSetDefaultConf(unittest.TestCase):
         '''Test correct flags set via upgrade-charm hook'''
         jenkins_slave.upgrade_charm()
         self.assertFalse(status.maintenance.assert_called())
-        expected = [mock.call('jenkins-slave.active'),
-                    mock.call('jenkins-slave.installed'),
-                    mock.call('nagios-nrpe.configured')]
+        expected = [
+            mock.call('jenkins-slave.active'),
+            mock.call('jenkins-slave.installed'),
+            mock.call('nagios-nrpe.configured'),
+        ]
         self.assertFalse(clear_flag.assert_has_calls(expected, any_order=True))
 
     @mock.patch('charms.reactive.clear_flag')
@@ -99,8 +106,7 @@ class TestSetDefaultConf(unittest.TestCase):
         '''Test correct flags are set via config-changed charm hook'''
         jenkins_slave.config_changed()
         self.assertFalse(status.maintenance.assert_called())
-        expected = [mock.call('jenkins-slave.configured'),
-                    mock.call('nagios-nrpe.configured')]
+        expected = [mock.call('jenkins-slave.configured'), mock.call('nagios-nrpe.configured')]
         self.assertFalse(clear_flag.assert_has_calls(expected, any_order=True))
 
     @mock.patch('charmhelpers.core.hookenv.config')
@@ -112,8 +118,8 @@ class TestSetDefaultConf(unittest.TestCase):
     @mock.patch('reactive.jenkins_slave.file_to_units')
     @mock.patch('reactive.jenkins_slave.write_default_conf')
     def test_hook_install(
-            self, write_default_conf, file_to_units, apt_purge, user_exists, service,
-            mkdir, adduser, config):
+        self, write_default_conf, file_to_units, apt_purge, user_exists, service, mkdir, adduser, config
+    ):
         config.return_value = {'tools': 'some-tools-package'}
         apt.install_queued.return_value = True
         user_exists.return_value = False
@@ -122,18 +128,24 @@ class TestSetDefaultConf(unittest.TestCase):
 
         expected = [mock.call(home_dir='/var/lib/jenkins', system_user=True, username='jenkins')]
         self.assertFalse(adduser.assert_has_calls(expected, any_order=True))
-        expected = [mock.call('/var/lib/jenkins', group='jenkins', owner='jenkins'),
-                    mock.call('/var/log/jenkins', group='jenkins', owner='jenkins')]
+        expected = [
+            mock.call('/var/lib/jenkins', group='jenkins', owner='jenkins'),
+            mock.call('/var/log/jenkins', group='jenkins', owner='jenkins'),
+        ]
         self.assertFalse(mkdir.assert_has_calls(expected, any_order=True))
         self.assertFalse(service.assert_has_calls([mock.call('enable', 'jenkins-slave')], any_order=True))
         self.assertFalse(apt_purge.assert_has_calls([mock.call(['jenkins-slave'])], any_order=True))
-        expected = [mock.call('files/download-slave.sh', '/usr/local/sbin/download-slave.sh'),
-                    mock.call('files/jenkins-slave-logrotate-config', '/etc/logrotate.d/jenkins-slave'),
-                    mock.call('files/jenkins-slave-systemd-config', '/lib/systemd/system/jenkins-slave.service')]
+        expected = [
+            mock.call('files/download-slave.sh', '/usr/local/sbin/download-slave.sh'),
+            mock.call('files/jenkins-slave-logrotate-config', '/etc/logrotate.d/jenkins-slave'),
+            mock.call('files/jenkins-slave-systemd-config', '/lib/systemd/system/jenkins-slave.service'),
+        ]
         self.assertFalse(file_to_units.assert_has_calls(expected, any_order=True))
         self.assertFalse(write_default_conf.assert_called())
-        expected = [mock.call.queue_install(['wget', 'default-jre-headless', 'some-tools-package']),
-                    mock.call.install_queued()]
+        expected = [
+            mock.call.queue_install(['wget', 'default-jre-headless', 'some-tools-package']),
+            mock.call.install_queued(),
+        ]
         self.assertEqual(apt.method_calls, expected)
 
         apt.install_queued.return_value = False
@@ -168,8 +180,9 @@ class TestSetDefaultConf(unittest.TestCase):
     @mock.patch('charmhelpers.core.unitdata.kv')
     @mock.patch('reactive.jenkins_slave.write_default_conf')
     @mock.patch('reactive.jenkins_slave.file_to_units')
-    def test_configure_jenkins_slave_no_url(self, file_to_units, write_default_conf, unitdata_kv, config,
-                                            set_flag, clear_flag):
+    def test_configure_jenkins_slave_no_url(
+        self, file_to_units, write_default_conf, unitdata_kv, config, set_flag, clear_flag
+    ):
         config.return_value = {}
         unitdata_kv.return_value = {}
         jenkins_slave.configure_jenkins_slave()
@@ -183,8 +196,9 @@ class TestSetDefaultConf(unittest.TestCase):
     @mock.patch('charmhelpers.core.unitdata.kv')
     @mock.patch('reactive.jenkins_slave.write_default_conf')
     @mock.patch('reactive.jenkins_slave.file_to_units')
-    def test_configure_jenkins_slave_master_url(self, file_to_units, write_default_conf, unitdata_kv, config,
-                                                set_flag, clear_flag):
+    def test_configure_jenkins_slave_master_url(
+        self, file_to_units, write_default_conf, unitdata_kv, config, set_flag, clear_flag
+    ):
         config.return_value = {'master_url': 'http://10.1.1.1:8080'}
         unitdata_kv.return_value = {}
         jenkins_slave.configure_jenkins_slave()
@@ -199,8 +213,9 @@ class TestSetDefaultConf(unittest.TestCase):
     @mock.patch('charmhelpers.core.unitdata.kv')
     @mock.patch('reactive.jenkins_slave.write_default_conf')
     @mock.patch('reactive.jenkins_slave.file_to_units')
-    def test_configure_jenkins_slave_relation_url(self, file_to_units, write_default_conf, unitdata_kv, config,
-                                                  set_flag, clear_flag):
+    def test_configure_jenkins_slave_relation_url(
+        self, file_to_units, write_default_conf, unitdata_kv, config, set_flag, clear_flag
+    ):
         config.return_value = {}
         unitdata_kv.return_value = {'url': 'http://10.22.22.22:8080'}
         jenkins_slave.configure_jenkins_slave()
@@ -253,10 +268,7 @@ class TestSetDefaultConf(unittest.TestCase):
     @mock.patch('charms.reactive.set_flag')
     def test_hook_slave_relation_changed_flags(self, set_flag, clear_flag):
         jenkins_slave.slave_relation_changed()
-        expected = [
-            mock.call('jenkins-slave.configured'),
-            mock.call('slave-relation.configured'),
-        ]
+        expected = [mock.call('jenkins-slave.configured'), mock.call('slave-relation.configured')]
         self.assertFalse(clear_flag.assert_has_calls(expected, any_order=True))
         self.assertFalse(set_flag.assert_has_calls([mock.call('slave-relation.available')], any_order=True))
 
@@ -273,28 +285,32 @@ class TestSetDefaultConf(unittest.TestCase):
     @mock.patch('charmhelpers.core.unitdata.kv')
     @mock.patch('reactive.jenkins_slave.write_default_conf')
     def test_hook_slave_relation(
-            self, write_default_conf, unitdata_kv, relation_set, relation_get,
-            config, set_flag, clear_flag):
+        self, write_default_conf, unitdata_kv, relation_set, relation_get, config, set_flag, clear_flag
+    ):
         config.return_value = {'master_url': ''}
         relation_get.return_value = 'http://10.1.1.1:8080'
         jenkins_slave.slave_relation()
         self.assertFalse(relation_get.assert_called_once_with('url'))
         self.assertFalse(clear_flag.assert_called_once_with('jenkins-slave.active'))
         self.assertFalse(set_flag.assert_called_once_with('slave-relation.configured'))
-        expected = [mock.call(executors=3),
-                    mock.call(labels='x86_64'),
-                    mock.call(slavehost='mock-jenkins-slave-0'),
-                    mock.call(slaveaddress='10.1.2.3')]
+        expected = [
+            mock.call(executors=3),
+            mock.call(labels='x86_64'),
+            mock.call(slavehost='mock-jenkins-slave-0'),
+            mock.call(slaveaddress='10.1.2.3'),
+        ]
         self.assertFalse(relation_set.assert_has_calls(expected, any_order=True))
 
         config.return_value = {'master_url': '', 'labels': 'label1 label2'}
         relation_get.return_value = 'http://10.1.1.1:8080'
         relation_set.reset_mock()
         jenkins_slave.slave_relation()
-        expected = [mock.call(executors=3),
-                    mock.call(labels='label1 label2'),
-                    mock.call(slavehost='mock-jenkins-slave-0'),
-                    mock.call(slaveaddress='10.1.2.3')]
+        expected = [
+            mock.call(executors=3),
+            mock.call(labels='label1 label2'),
+            mock.call(slavehost='mock-jenkins-slave-0'),
+            mock.call(slaveaddress='10.1.2.3'),
+        ]
         self.assertFalse(relation_set.assert_has_calls(expected, any_order=True))
 
     @mock.patch('charmhelpers.core.hookenv.config')
@@ -311,8 +327,7 @@ class TestSetDefaultConf(unittest.TestCase):
     @mock.patch('charms.reactive.set_flag')
     @mock.patch('charmhelpers.core.hookenv.config')
     @mock.patch('charmhelpers.core.hookenv.relation_get')
-    def test_hook_slave_relation_relation_url_not_set(
-            self, relation_get, config, set_flag, clear_flag):
+    def test_hook_slave_relation_relation_url_not_set(self, relation_get, config, set_flag, clear_flag):
         config.return_value = {'master_url': ''}
         relation_get.return_value = ''
         jenkins_slave.slave_relation()
@@ -326,8 +341,11 @@ class TestSetDefaultConf(unittest.TestCase):
         nrpe_instance_mock = nrpe(get_nagios_hostname(), primary=True)
         jenkins_slave.configure_nagios(None)
         status.maintenance.assert_called()
-        want = [mock.call('jenkins_slave_ps', 'Jenkins Slave Process',
-                          '/usr/lib/nagios/plugins/check_procs -c 1:1 -a slave.jar')]
+        want = [
+            mock.call(
+                'jenkins_slave_ps', 'Jenkins Slave Process', '/usr/lib/nagios/plugins/check_procs -c 1:1 -a slave.jar'
+            )
+        ]
         nrpe_instance_mock.add_check.assert_has_calls(want, any_order=True)
 
         nrpe_instance_mock.write.assert_called()
diff --git a/tox.ini b/tox.ini
index 99078b2..220ff5d 100644
--- a/tox.ini
+++ b/tox.ini
@@ -20,6 +20,10 @@ basepython = python3
 commands = functest-run-suite --keep-model
 deps = -r{toxinidir}/tests/functional/requirements.txt
 
+[testenv:black]
+commands = black --skip-string-normalization --line-length=120 .
+deps = black
+
 [testenv:lint]
 commands = flake8
 deps = flake8