openstack/barbican
Code Issues Proposed changes

Add missing unit test for clean_command and fix error handling

Browse Source 
The clean_command in barbican/model/clean.py is missing
a unit test for coverage. Unit tests have been added.

There was a redundant log exception that was removed.
A check for a successfull exit was removed because there are no
apparent exceptions that raise successfull exits during clean up.

Closes-Bug: #1555182
Change-Id: Ia1e1fe07dc9081b20c5fb0bad1a979809db0be4a
This commit is contained in:
Elvin Tubillara 2016-03-07 15:24:59 -06:00
parent c5526e2b69
commit 01e6df5f47
2 changed files with 81 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
barbican/model/clean.py
View File

@ -29,11 +29,6 @@ log.setup(CONF, 'barbican')
LOG = log.getLogger(__name__)
def _exception_is_successful_exit(thrown_exception):
return (isinstance(thrown_exception, SystemExit) and
(thrown_exception.code is None or thrown_exception.code == 0))
def cleanup_unassociated_projects():
"""Clean up unassociated projects.
@ -311,8 +306,6 @@ def clean_command(sql_url, min_num_days, do_clean_unassociated_projects,
:param verbose: If True, log and print more information
:param log_file: If set, override the log_file configured
"""
# TODO(edtubill) Make unit test for this method
if verbose:
# The verbose flag prints out log events to the screen, otherwise
# the log events will only go to the log file
@ -350,12 +343,10 @@ def clean_command(sql_url, min_num_days, do_clean_unassociated_projects,
repo.commit()
except Exception as ex:
if not _exception_is_successful_exit(ex):
LOG.exception('Failed to clean up soft deletions in database.')
LOG.exception(ex.message)
repo.rollback()
cleanup_total = 0 # rollback happened, no entries affected
raise ex
LOG.exception('Failed to clean up soft deletions in database.')
repo.rollback()
cleanup_total = 0 # rollback happened, no entries affected
raise ex
finally:
stop_watch.stop()
elapsed_time = stop_watch.elapsed()

77
barbican/tests/cmd/test_db_cleanup.py
View File

@ -20,6 +20,7 @@ from barbican.tests import database_utils as utils
from sqlalchemy.exc import IntegrityError
import datetime
import mock
def _create_project(project_name):
@ -340,6 +341,82 @@ class WhenTestingDBCleanUpCommand(utils.RepositoryTestCase):
clean.cleanup_unassociated_projects()
self.assertFalse(_entry_exists(project_with_children))
@mock.patch('barbican.model.clean.cleanup_all')
@mock.patch('barbican.model.clean.soft_delete_expired_secrets')
@mock.patch('barbican.model.clean.cleanup_unassociated_projects')
@mock.patch('barbican.model.clean.repo')
@mock.patch('barbican.model.clean.log')
@mock.patch('barbican.model.clean.CONF')
def test_clean_up_command(self, mock_conf, mock_log, mock_repo,
mock_clean_unc_projects,
mock_soft_del_expire_secrets, mock_clean_all):
"""Tests the clean command"""
test_sql_url = "mysql+pymysql://notrealuser:datab@127.0.0.1/barbican't"
min_num_days = 91
do_clean_unassociated_projects = True
do_soft_delete_expired_secrets = True
verbose = True
test_log_file = "/tmp/sometempfile"
clean.clean_command(test_sql_url, min_num_days,
do_clean_unassociated_projects,
do_soft_delete_expired_secrets, verbose,
test_log_file)
set_calls = [mock.call('debug', True),
mock.call('log_file', test_log_file),
mock.call('sql_connection', test_sql_url)]
mock_conf.set_override.assert_has_calls(set_calls)
clear_calls = [mock.call('debug'), mock.call('log_file'),
mock.call('sql_connection')]
mock_conf.clear_override.assert_has_calls(clear_calls)
self.assertTrue(mock_repo.setup_database_engine_and_factory.called)
self.assertTrue(mock_repo.commit.called)
self.assertTrue(mock_repo.clear.called)
self.assertTrue(mock_clean_unc_projects.called)
self.assertTrue(mock_soft_del_expire_secrets)
self.assertTrue(mock_clean_all)
@mock.patch('barbican.model.clean.cleanup_all')
@mock.patch('barbican.model.clean.soft_delete_expired_secrets')
@mock.patch('barbican.model.clean.cleanup_unassociated_projects')
@mock.patch('barbican.model.clean.repo')
@mock.patch('barbican.model.clean.log')
@mock.patch('barbican.model.clean.CONF')
def test_clean_up_command_with_false_args(
self, mock_conf, mock_log, mock_repo, mock_clean_unc_projects,
mock_soft_del_expire_secrets, mock_clean_all):
"""Tests the clean command with false args"""
test_sql_url = None
min_num_days = -1
do_clean_unassociated_projects = False
do_soft_delete_expired_secrets = False
verbose = None
test_log_file = None
clean.clean_command(test_sql_url, min_num_days,
do_clean_unassociated_projects,
do_soft_delete_expired_secrets, verbose,
test_log_file)
mock_conf.set_override.assert_not_called()
mock_conf.clear_override.assert_not_called()
self.assertTrue(mock_repo.setup_database_engine_and_factory.called)
self.assertTrue(mock_repo.commit.called)
self.assertTrue(mock_repo.clear.called)
self.assertTrue(mock_clean_all)
self.assertFalse(mock_clean_unc_projects.called)
self.assertFalse(mock_soft_del_expire_secrets.called)
@mock.patch('barbican.model.clean.cleanup_all',
side_effect=IntegrityError("", "", "", ""))
@mock.patch('barbican.model.clean.repo')
@mock.patch('barbican.model.clean.log')
@mock.patch('barbican.model.clean.CONF')
def test_clean_up_command_with_exception(
self, mock_conf, mock_log, mock_repo, mock_clean_all):
"""Tests that the clean command throws exceptions"""
args = ("sql", 2, False, False, False, "/tmp/nope")
self.assertRaises(IntegrityError, clean.clean_command, *args)
self.assertTrue(mock_repo.rollback.called)
@_create_project("my integrity error keystone id")
def test_db_cleanup_raise_integrity_error(self, project):
"""Test that an integrity error is thrown