Skip to content

Commit

Permalink
Fix unit tests.
Browse files Browse the repository at this point in the history
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: openstack-charmers/release-tools#301
Change-Id: Id6767e70bfea9fab6a975d8e453d9334f6c31eaa
  • Loading branch information
freyes committed Aug 14, 2024
1 parent b572c75 commit 930f5b3
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 27 deletions.
4 changes: 2 additions & 2 deletions hooks/keystone_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ def _decode_password_security_compliance_string(cls, maybe_yaml):
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)),
Expand All @@ -332,7 +332,7 @@ def _decode_password_security_compliance_string(cls, maybe_yaml):
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 "
Expand Down
6 changes: 3 additions & 3 deletions hooks/keystone_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions unit_tests/test_keystone_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,10 @@ def fake_relation_ids(relation):
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))
Expand Down
42 changes: 25 additions & 17 deletions unit_tests/test_keystone_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -796,9 +796,9 @@ def test_protect_user_account_from_pci_dss_v3(

# 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
Expand Down Expand Up @@ -1779,19 +1779,19 @@ def test_key_setup(self, mock_path_exists, mock_is_leader):
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()

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')
Expand All @@ -1805,7 +1805,7 @@ def test_key_leader_set(self, listdir, leader_set, leader_get, is_leader):
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(
Expand Down Expand Up @@ -1833,7 +1833,7 @@ def test_key_leader_set_no_change(
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()
Expand All @@ -1859,12 +1859,20 @@ def test_key_write(
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
Expand Down Expand Up @@ -2007,7 +2015,7 @@ def test_get_charm_credentials(self, mock_leader_get):
'_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]
Expand Down

0 comments on commit 930f5b3

Please sign in to comment.