From 930f5b31f333f750b6fb7c9ca9f559d60379491e Mon Sep 17 00:00:00 2001 From: Felipe Reyes Date: Wed, 14 Aug 2024 00:52:20 -0400 Subject: [PATCH] Fix unit tests. Summary of changes: - Replace self.assertTrue(foo.called) with self.assert_called() - Replace foo.has_calls(...) with foo.assert_has_calls(...) - Fix unit test failures - Bump up flake8 to make it compatible with py312 - Fix flake8/pep8 errors Related-Pr: https://github.com/openstack-charmers/release-tools/pull/301 Change-Id: Id6767e70bfea9fab6a975d8e453d9334f6c31eaa --- hooks/keystone_context.py | 4 +-- hooks/keystone_utils.py | 6 ++--- tox.ini | 2 +- unit_tests/test_keystone_hooks.py | 8 +++--- unit_tests/test_keystone_utils.py | 42 ++++++++++++++++++------------- 5 files changed, 35 insertions(+), 27 deletions(-) diff --git a/hooks/keystone_context.py b/hooks/keystone_context.py index f0fb6be9..209cbbfc 100644 --- a/hooks/keystone_context.py +++ b/hooks/keystone_context.py @@ -314,7 +314,7 @@ class KeystoneContext(context.OSContextGenerator): level='ERROR') return None # ensure that the top level is a dictionary. - if type(config_items) != dict: + if not isinstance(config_items, dict): log("Couldn't decode config value for " "'password-security-compliance'. It doesn't appear to be a " "dictionary: {}".format(str(config_items)), @@ -332,7 +332,7 @@ class KeystoneContext(context.OSContextGenerator): return None # check that the types are valid valid_types = cls.ALLOWED_SECURITY_COMPLIANCE_SCHEMA - invalid_types = {k: (type(v) != valid_types[k]) + invalid_types = {k: (not isinstance(v, valid_types[k])) for k, v in config_items.items()} if any(invalid_types.values()): log("Invalid config value type(s) found in config " diff --git a/hooks/keystone_utils.py b/hooks/keystone_utils.py index 177b67b6..18d0f615 100644 --- a/hooks/keystone_utils.py +++ b/hooks/keystone_utils.py @@ -569,8 +569,8 @@ def restart_pid_check(service_name, ptable_string=None): @retry_on_exception(5, base_delay=3, exc_type=AssertionError) def check_pids_gone(svc_string): log("Checking no pids for {} exist".format(svc_string), level=INFO) - assert(subprocess.call(["pgrep", svc_string, "--nslist", "pid", - "--ns", str(os.getpid())]) == 1) + assert subprocess.call(["pgrep", svc_string, "--nslist", "pid", + "--ns", str(os.getpid())]) == 1 if not ptable_string: ptable_string = service_name @@ -2652,7 +2652,7 @@ def check_extra_for_assess_status(configs): # Check if any of the vips are invalid invalid_vips = get_invalid_vips() if invalid_vips: - return('blocked', f'Invalid vips: {invalid_vips}') + return ('blocked', f'Invalid vips: {invalid_vips}') # verify that the config item, if set, is actually usable and valid conf = config('password-security-compliance') diff --git a/tox.ini b/tox.ini index 57c2f4ec..0a594c83 100644 --- a/tox.ini +++ b/tox.ini @@ -65,7 +65,7 @@ deps = [testenv:pep8] basepython = python3 -deps = flake8==3.9.2 +deps = flake8==7.1.1 git+https://github.com/juju/charm-tools.git commands = flake8 {posargs} hooks unit_tests tests actions lib files charm-proof diff --git a/unit_tests/test_keystone_hooks.py b/unit_tests/test_keystone_hooks.py index 4934fbd3..e1fcfe73 100644 --- a/unit_tests/test_keystone_hooks.py +++ b/unit_tests/test_keystone_hooks.py @@ -282,10 +282,10 @@ class KeystoneRelationTests(CharmTestCase): self.assertTrue(configs.write_all.called) self.open_port.assert_called_with(5000) - self.assertTrue(mock_cluster_joined.called) - self.assertTrue(update.called) - self.assertTrue(mock_update_domains.called) - self.assertTrue(mock_notify_middleware.called_once) + mock_cluster_joined.assert_called() + update.assert_called() + mock_update_domains.assert_called() + mock_notify_middleware.assert_called_once() (mock_maybe_do_policyd_overrides_on_config_changed .assert_called_once_with(ANY, "keystone", restart_handler=ANY)) diff --git a/unit_tests/test_keystone_utils.py b/unit_tests/test_keystone_utils.py index c8c82962..a299c384 100644 --- a/unit_tests/test_keystone_utils.py +++ b/unit_tests/test_keystone_utils.py @@ -796,9 +796,9 @@ class TestKeystoneUtils(CharmTestCase): # test user exists and option exists and is true utils.protect_user_account_from_pci_dss_force_change_password('u1') - calls = [call('u1', utils.DEFAULT_DOMAIN), - call('u1', utils.SERVICE_DOMAIN)] - mock_get_user_dict.has_calls(calls) + calls = [call('u1', domain=utils.DEFAULT_DOMAIN), + call('u1', domain=utils.SERVICE_DOMAIN)] + mock_get_user_dict.assert_has_calls(calls) mock_update_user.assert_not_called() # test user exists and option exists and is False @@ -1779,11 +1779,11 @@ class TestKeystoneUtils(CharmTestCase): with patch.object(builtins, 'open', mock_open()) as m: utils.key_setup() m.assert_called_once_with(utils.KEY_SETUP_FILE, "w") - self.subprocess.check_output.has_calls( + self.subprocess.check_call.assert_has_calls( [ - base_cmd + ['fernet_setup'], - base_cmd + ['credential_setup'], - base_cmd + ['credential_migrate'], + call(base_cmd + ['fernet_setup']), + call(base_cmd + ['credential_setup']), + call(base_cmd + ['credential_migrate']), ]) mock_path_exists.assert_called_once_with(utils.KEY_SETUP_FILE) mock_is_leader.assert_called_once_with() @@ -1791,7 +1791,7 @@ class TestKeystoneUtils(CharmTestCase): def test_fernet_rotate(self): cmd = ['sudo', '-u', 'keystone', 'keystone-manage', 'fernet_rotate'] utils.fernet_rotate() - self.subprocess.check_output.called_with(cmd) + self.subprocess.check_call.assert_called_with(cmd) @patch.object(utils, 'is_leader') @patch.object(utils, 'leader_get') @@ -1805,7 +1805,7 @@ class TestKeystoneUtils(CharmTestCase): with patch.object(builtins, 'open', mock_open( read_data="some_data")): utils.key_leader_set() - listdir.has_calls([ + listdir.assert_has_calls([ call(utils.FERNET_KEY_REPOSITORY), call(utils.CREDENTIAL_KEY_REPOSITORY)]) leader_set.assert_called_with( @@ -1833,7 +1833,7 @@ class TestKeystoneUtils(CharmTestCase): with patch.object(builtins, 'open', mock_open( read_data="some_data")): utils.key_leader_set() - listdir.has_calls([ + listdir.assert_has_calls([ call(utils.FERNET_KEY_REPOSITORY), call(utils.CREDENTIAL_KEY_REPOSITORY)]) leader_set.assert_not_called() @@ -1859,12 +1859,20 @@ class TestKeystoneUtils(CharmTestCase): with patch.object(builtins, 'open', mock_open()) as m: utils.key_write() m.assert_called_with(utils.KEY_SETUP_FILE, "w") - self.mkdir.has_calls([call(utils.CREDENTIAL_KEY_REPOSITORY, - owner='keystone', group='keystone', - perms=0o700), - call(utils.FERNET_KEY_REPOSITORY, - owner='keystone', group='keystone', - perms=0o700)]) + self.mkdir.assert_has_calls( + [call(utils.FERNET_KEY_REPOSITORY, + owner='keystone', group='keystone', + perms=0o700), + call(utils.FERNET_KEY_REPOSITORY + '.tmp', + owner='keystone', group='keystone', + perms=0o700), + call(utils.CREDENTIAL_KEY_REPOSITORY, + owner='keystone', group='keystone', + perms=0o700), + call(utils.CREDENTIAL_KEY_REPOSITORY + '.tmp', + owner='keystone', group='keystone', + perms=0o700)] + ) tmp_fernet_dir = utils.FERNET_KEY_REPOSITORY + ".tmp" tmp_creds_dir = utils.CREDENTIAL_KEY_REPOSITORY + ".tmp" # note 'any_order=True' as we are dealing with dictionaries in Py27 @@ -2007,7 +2015,7 @@ class TestKeystoneUtils(CharmTestCase): '_charm-keystone-admin', 'fakepassword', 'all', 'admin', 'default', 'default') mock_leader_get.return_value = 'fakepassword' - self.assertEquals(utils.get_charm_credentials(), expect) + self.assertEqual(utils.get_charm_credentials(), expect) mock_leader_get.assert_called_once_with(expect.username + '_passwd') mock_leader_get.retrun_value = None mock_leader_get.side_effect = [None]