clean up tests

* remove unnecessary parametrisation where we're not testing
type-specific things
* remove some unused fixtures
* replace len(x.all()) with x.count()
* don't deleted notificationhistory now that we know it's not written
* remove some pre-test function asserts
This commit is contained in:
Leo Hemsted
2019-06-04 10:53:57 +01:00
parent 2ac2cbbd37
commit 4fc7cc6f2e

View File

@@ -73,7 +73,6 @@ def test_should_delete_notifications_by_type_after_seven_days(
expected_letter_count
):
mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects")
assert len(Notification.query.all()) == 0
email_template, letter_template, sms_template = _create_templates(sample_service)
# create one notification a day between 1st and 10th from 11:00 to 19:00 of each type
for i in range(1, 11):
@@ -82,11 +81,12 @@ def test_should_delete_notifications_by_type_after_seven_days(
create_notification(template=email_template, created_at=datetime.utcnow(), status="permanent-failure")
create_notification(template=sms_template, created_at=datetime.utcnow(), status="delivered")
create_notification(template=letter_template, created_at=datetime.utcnow(), status="temporary-failure")
all_notifications = Notification.query.all()
assert len(all_notifications) == 30
assert Notification.query.count() == 30
# Records from before 3rd should be deleted
with freeze_time(delete_run_time):
delete_notifications_older_than_retention_by_type(notification_type)
remaining_sms_notifications = Notification.query.filter_by(notification_type='sms').all()
remaining_letter_notifications = Notification.query.filter_by(notification_type='letter').all()
remaining_email_notifications = Notification.query.filter_by(notification_type='email').all()
@@ -113,7 +113,6 @@ def test_should_not_delete_notification_history(sample_service, mocker):
create_notification(template=sms_template, status='permanent-failure')
create_notification(template=letter_template, status='permanent-failure')
assert Notification.query.count() == 3
assert NotificationHistory.query.count() == 0
delete_notifications_older_than_retention_by_type('sms')
assert Notification.query.count() == 2
assert NotificationHistory.query.count() == 1
@@ -123,10 +122,10 @@ def test_should_not_delete_notification_history(sample_service, mocker):
def test_delete_notifications_for_days_of_retention(sample_service, notification_type, mocker):
mock_get_s3 = mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects")
create_test_data(notification_type, sample_service)
assert len(Notification.query.all()) == 9
assert Notification.query.count() == 9
delete_notifications_older_than_retention_by_type(notification_type)
assert len(Notification.query.all()) == 7
assert len(Notification.query.filter_by(notification_type=notification_type).all()) == 1
assert Notification.query.count() == 7
assert Notification.query.filter_by(notification_type=notification_type).count() == 1
if notification_type == 'letter':
mock_get_s3.assert_called_with(bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'],
subfolder="{}/NOTIFY.LETTER_REF.D.2.C.C".format(str(datetime.utcnow().date()))
@@ -136,17 +135,13 @@ def test_delete_notifications_for_days_of_retention(sample_service, notification
mock_get_s3.assert_not_called()
@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter'])
def test_delete_notifications_inserts_notification_history(sample_service, notification_type, mocker):
mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects")
create_test_data(notification_type, sample_service)
NotificationHistory.query.delete()
assert len(Notification.query.all()) == 9
delete_notifications_older_than_retention_by_type(notification_type)
assert len(Notification.query.all()) == 7
def test_delete_notifications_inserts_notification_history(sample_service):
create_test_data('sms', sample_service)
assert Notification.query.count() == 9
delete_notifications_older_than_retention_by_type('sms')
assert Notification.query.count() == 7
history = NotificationHistory.query.all()
assert len(history) == 2
assert NotificationHistory.query.count() == 2
def test_delete_notifications_updates_notification_history(sample_email_template, mocker):
@@ -173,30 +168,23 @@ def test_delete_notifications_updates_notification_history(sample_email_template
assert history[0].sent_by == 'ses'
@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter'])
def test_delete_notifications_keep_data_for_days_of_retention_is_longer(sample_service, notification_type, mocker):
mock_get_s3 = mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects")
create_test_data(notification_type, sample_service, 15)
assert len(Notification.query.all()) == 9
delete_notifications_older_than_retention_by_type(notification_type)
assert len(Notification.query.filter_by().all()) == 8
assert len(Notification.query.filter_by(notification_type=notification_type).all()) == 2
if notification_type == 'letter':
assert mock_get_s3.called
else:
mock_get_s3.assert_not_called()
def test_delete_notifications_keep_data_for_days_of_retention_is_longer(sample_service):
create_test_data('sms', sample_service, 15)
assert Notification.query.count() == 9
delete_notifications_older_than_retention_by_type('sms')
assert Notification.query.count() == 8
assert Notification.query.filter(Notification.notification_type == 'sms').count() == 2
def test_delete_notifications_with_test_keys(sample_template, mocker):
mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects")
create_notification(template=sample_template, key_type='test', created_at=datetime.utcnow() - timedelta(days=8))
assert len(Notification.query.all()) == 1
delete_notifications_older_than_retention_by_type('sms')
assert len(Notification.query.filter_by().all()) == 0
assert Notification.query.count() == 0
def test_delete_notifications_delete_notification_type_for_default_time_if_no_days_of_retention_for_type(
sample_service, mocker
sample_service
):
create_service_data_retention(service=sample_service, notification_type='sms',
days_of_retention=15)
@@ -210,10 +198,10 @@ def test_delete_notifications_delete_notification_type_for_default_time_if_no_da
created_at=datetime.utcnow() - timedelta(days=14))
create_notification(template=letter_template, status='temporary-failure',
created_at=datetime.utcnow() - timedelta(days=14))
assert len(Notification.query.all()) == 6
assert Notification.query.count() == 6
delete_notifications_older_than_retention_by_type('email')
assert len(Notification.query.filter_by().all()) == 5
assert len(Notification.query.filter_by(notification_type='email').all()) == 1
assert Notification.query.count() == 5
assert Notification.query.filter_by(notification_type='email').count() == 1
def test_delete_notifications_does_try_to_delete_from_s3_when_letter_has_not_been_sent(sample_service, mocker):
@@ -236,7 +224,6 @@ def test_should_not_delete_notification_if_history_does_not_exist(sample_service
create_notification(template=sms_template, status='delivered')
create_notification(template=letter_template, status='temporary-failure')
assert Notification.query.count() == 3
assert NotificationHistory.query.count() == 0
delete_notifications_older_than_retention_by_type('sms')
assert Notification.query.count() == 3
assert NotificationHistory.query.count() == 0
@@ -252,7 +239,7 @@ def test_delete_notifications_calls_subquery_multiple_times(sample_template):
assert Notification.query.count() == 0
def test_delete_notifications_returns_sum_correctly(sample_template, notify_db_session):
def test_delete_notifications_returns_sum_correctly(sample_template):
create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8))
create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8))
@@ -265,25 +252,19 @@ def test_delete_notifications_returns_sum_correctly(sample_template, notify_db_s
assert ret == 4
@pytest.mark.parametrize('notification_type', ['sms'])
def test_insert_update_notification_history(sample_service, notification_type):
template = create_template(sample_service, template_type=notification_type)
def test_insert_update_notification_history(sample_service):
template = create_template(sample_service, template_type='sms')
notification_1 = create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=3))
notification_2 = create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=8))
notification_3 = create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=9))
other_types = ['sms', 'email', 'letter']
other_types.remove(notification_type)
other_types = ['email', 'letter']
for template_type in other_types:
t = create_template(service=sample_service, template_type=template_type)
create_notification(template=t, created_at=datetime.utcnow() - timedelta(days=3))
create_notification(template=t, created_at=datetime.utcnow() - timedelta(days=8))
NotificationHistory.query.delete()
history = NotificationHistory.query.all()
assert len(history) == 0
insert_update_notification_history(
notification_type=notification_type, date_to_delete_from=datetime.utcnow() - timedelta(days=7),
notification_type='sms', date_to_delete_from=datetime.utcnow() - timedelta(days=7),
service_id=sample_service.id)
history = NotificationHistory.query.all()
assert len(history) == 2
@@ -294,23 +275,16 @@ def test_insert_update_notification_history(sample_service, notification_type):
assert notification_3.id in history_ids
@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter'])
def test_insert_update_notification_history_only_insert_update_given_service(sample_service, notification_type):
def test_insert_update_notification_history_only_insert_update_given_service(sample_service):
other_service = create_service(service_name='another service')
other_template = create_template(service=other_service, template_type=notification_type)
template = create_template(service=sample_service, template_type=notification_type)
other_template = create_template(service=other_service)
template = create_template(service=sample_service)
notification_1 = create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=3))
notification_2 = create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=8))
notification_3 = create_notification(template=other_template, created_at=datetime.utcnow() - timedelta(days=3))
notification_4 = create_notification(template=other_template, created_at=datetime.utcnow() - timedelta(days=8))
NotificationHistory.query.delete()
history = NotificationHistory.query.all()
assert len(history) == 0
insert_update_notification_history(
notification_type, datetime.utcnow() - timedelta(days=7), sample_service.id)
insert_update_notification_history('sms', datetime.utcnow() - timedelta(days=7), sample_service.id)
history = NotificationHistory.query.all()
assert len(history) == 1
@@ -328,6 +302,5 @@ def test_insert_update_notification_history_updates_history_with_new_status(samp
insert_update_notification_history(
'sms', datetime.utcnow() - timedelta(days=7), sample_template.service_id)
history = NotificationHistory.query.get(notification_2.id)
assert history.id == notification_2.id
assert history.status == 'delivered'
assert not NotificationHistory.query.get(notification_1.id)