refactor tests

re-order notification dao delete notifications test to move "fixtures"
to the top of the file

changed create_service_data_retention to take an ORM object, not an id.
brings it in line with other db.py test functions
This commit is contained in:
Leo Hemsted
2019-06-03 17:27:08 +01:00
parent 396149ddde
commit 67f171f2f8
7 changed files with 65 additions and 71 deletions

View File

@@ -105,8 +105,8 @@ def test_will_remove_csv_files_for_jobs_older_than_retention_period(
mocker.patch('app.celery.nightly_tasks.s3.remove_job_from_s3')
service_1 = create_service(service_name='service 1')
service_2 = create_service(service_name='service 2')
create_service_data_retention(service_id=service_1.id, notification_type=SMS_TYPE, days_of_retention=3)
create_service_data_retention(service_id=service_2.id, notification_type=EMAIL_TYPE, days_of_retention=30)
create_service_data_retention(service=service_1, notification_type=SMS_TYPE, days_of_retention=3)
create_service_data_retention(service=service_2, notification_type=EMAIL_TYPE, days_of_retention=30)
sms_template_service_1 = create_template(service=service_1)
email_template_service_1 = create_template(service=service_1, template_type='email')

View File

@@ -20,6 +20,40 @@ from tests.app.db import (
)
def create_test_data(notification_type, sample_service, days_of_retention=3):
service_with_default_data_retention = create_service(service_name='default data retention')
email_template, letter_template, sms_template = _create_templates(sample_service)
default_email_template, default_letter_template, default_sms_template = _create_templates(
service_with_default_data_retention)
create_notification(template=email_template, status='delivered')
create_notification(template=sms_template, status='permanent-failure')
create_notification(template=letter_template, status='temporary-failure',
reference='LETTER_REF', sent_at=datetime.utcnow())
create_notification(template=email_template, status='delivered',
created_at=datetime.utcnow() - timedelta(days=4))
create_notification(template=sms_template, status='permanent-failure',
created_at=datetime.utcnow() - timedelta(days=4))
create_notification(template=letter_template, status='temporary-failure',
reference='LETTER_REF', sent_at=datetime.utcnow(),
created_at=datetime.utcnow() - timedelta(days=4))
create_notification(template=default_email_template, status='delivered',
created_at=datetime.utcnow() - timedelta(days=8))
create_notification(template=default_sms_template, status='permanent-failure',
created_at=datetime.utcnow() - timedelta(days=8))
create_notification(template=default_letter_template, status='temporary-failure',
reference='LETTER_REF', sent_at=datetime.utcnow(),
created_at=datetime.utcnow() - timedelta(days=8))
create_service_data_retention(service=sample_service, notification_type=notification_type,
days_of_retention=days_of_retention)
def _create_templates(sample_service):
email_template = create_template(service=sample_service, template_type='email')
sms_template = create_template(service=sample_service)
letter_template = create_template(service=sample_service, template_type='letter')
return email_template, letter_template, sms_template
@pytest.mark.parametrize('month, delete_run_time',
[(4, '2016-04-10 23:40'), (1, '2016-01-11 00:40')])
@pytest.mark.parametrize(
@@ -70,9 +104,8 @@ def test_should_delete_notifications_by_type_after_seven_days(
assert notification.created_at.date() >= date(2016, month, 3)
@pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter'])
@freeze_time("2016-01-10 12:00:00.000000")
def test_should_not_delete_notification_history(sample_service, notification_type, mocker):
def test_should_not_delete_notification_history(sample_service, mocker):
mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects")
with freeze_time('2016-01-01 12:00'):
email_template, letter_template, sms_template = _create_templates(sample_service)
@@ -81,7 +114,7 @@ def test_should_not_delete_notification_history(sample_service, notification_typ
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(notification_type)
delete_notifications_older_than_retention_by_type('sms')
assert Notification.query.count() == 2
assert NotificationHistory.query.count() == 1
@@ -140,33 +173,6 @@ def test_delete_notifications_updates_notification_history(sample_email_template
assert history[0].sent_by == 'ses'
def create_test_data(notification_type, sample_service, days_of_retention=3):
service_with_default_data_retention = create_service(service_name='default data retention')
email_template, letter_template, sms_template = _create_templates(sample_service)
default_email_template, default_letter_template, default_sms_template = _create_templates(
service_with_default_data_retention)
create_notification(template=email_template, status='delivered')
create_notification(template=sms_template, status='permanent-failure')
create_notification(template=letter_template, status='temporary-failure',
reference='LETTER_REF', sent_at=datetime.utcnow())
create_notification(template=email_template, status='delivered',
created_at=datetime.utcnow() - timedelta(days=4))
create_notification(template=sms_template, status='permanent-failure',
created_at=datetime.utcnow() - timedelta(days=4))
create_notification(template=letter_template, status='temporary-failure',
reference='LETTER_REF', sent_at=datetime.utcnow(),
created_at=datetime.utcnow() - timedelta(days=4))
create_notification(template=default_email_template, status='delivered',
created_at=datetime.utcnow() - timedelta(days=8))
create_notification(template=default_sms_template, status='permanent-failure',
created_at=datetime.utcnow() - timedelta(days=8))
create_notification(template=default_letter_template, status='temporary-failure',
reference='LETTER_REF', sent_at=datetime.utcnow(),
created_at=datetime.utcnow() - timedelta(days=8))
create_service_data_retention(service_id=sample_service.id, notification_type=notification_type,
days_of_retention=days_of_retention)
@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")
@@ -192,7 +198,7 @@ def test_delete_notifications_with_test_keys(sample_template, mocker):
def test_delete_notifications_delete_notification_type_for_default_time_if_no_days_of_retention_for_type(
sample_service, mocker
):
create_service_data_retention(service_id=sample_service.id, notification_type='sms',
create_service_data_retention(service=sample_service, notification_type='sms',
days_of_retention=15)
email_template, letter_template, sms_template = _create_templates(sample_service)
create_notification(template=email_template, status='delivered')
@@ -220,9 +226,8 @@ def test_delete_notifications_does_try_to_delete_from_s3_when_letter_has_not_bee
mock_get_s3.assert_not_called()
@pytest.mark.parametrize('notification_type', ['sms'])
@freeze_time("2016-01-10 12:00:00.000000")
def test_should_not_delete_notification_if_history_does_not_exist(sample_service, notification_type, mocker):
def test_should_not_delete_notification_if_history_does_not_exist(sample_service, mocker):
mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects")
mocker.patch("app.dao.notifications_dao.insert_update_notification_history")
with freeze_time('2016-01-01 12:00'):
@@ -232,19 +237,15 @@ def test_should_not_delete_notification_if_history_does_not_exist(sample_service
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(notification_type)
delete_notifications_older_than_retention_by_type('sms')
assert Notification.query.count() == 3
assert NotificationHistory.query.count() == 0
def test_delete_notifications_calls_subquery(
notify_db_session, mocker
):
service = create_service()
sms_template = create_template(service=service)
create_notification(template=sms_template, created_at=datetime.now() - timedelta(days=8))
create_notification(template=sms_template, created_at=datetime.now() - timedelta(days=8))
create_notification(template=sms_template, created_at=datetime.now() - timedelta(days=8))
def test_delete_notifications_calls_subquery_multiple_times(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))
create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8))
assert Notification.query.count() == 3
delete_notifications_older_than_retention_by_type('sms', qry_limit=1)
@@ -317,10 +318,3 @@ def test_insert_update_notification_history_updates_history_with_new_status(samp
assert history.id == notification_2.id
assert history.status == 'delivered'
assert not NotificationHistory.query.get(notification_1.id)
def _create_templates(sample_service):
email_template = create_template(service=sample_service, template_type='email')
sms_template = create_template(service=sample_service)
letter_template = create_template(service=sample_service, template_type='letter')
return email_template, letter_template, sms_template

View File

@@ -97,10 +97,10 @@ def test_should_delete_inbound_sms_according_to_data_retention(notify_db_session
services = [short_retention_service, no_retention_service, long_retention_service]
create_service_data_retention(long_retention_service.id, notification_type='sms', days_of_retention=30)
create_service_data_retention(short_retention_service.id, notification_type='sms', days_of_retention=3)
create_service_data_retention(long_retention_service, notification_type='sms', days_of_retention=30)
create_service_data_retention(short_retention_service, notification_type='sms', days_of_retention=3)
# email retention doesn't affect anything
create_service_data_retention(short_retention_service.id, notification_type='email', days_of_retention=4)
create_service_data_retention(short_retention_service, notification_type='email', days_of_retention=4)
dates = [
datetime(2017, 6, 4, 23, 00), # just before three days

View File

@@ -141,8 +141,8 @@ def test_update_service_data_retention_does_not_update_row_if_data_retention_is_
('letter', 'sms')]
)
def test_fetch_service_data_retention_by_notification_type(sample_service, notification_type, alternate):
data_retention = create_service_data_retention(service_id=sample_service.id, notification_type=notification_type)
create_service_data_retention(service_id=sample_service.id, notification_type=alternate)
data_retention = create_service_data_retention(service=sample_service, notification_type=notification_type)
create_service_data_retention(service=sample_service, notification_type=alternate)
result = fetch_service_data_retention_by_notification_type(sample_service.id, notification_type)
assert result == data_retention

View File

@@ -806,12 +806,12 @@ def ses_notification_callback():
def create_service_data_retention(
service_id,
service,
notification_type='sms',
days_of_retention=3
):
data_retention = insert_service_data_retention(
service_id=service_id,
service_id=service.id,
notification_type=notification_type,
days_of_retention=days_of_retention
)

View File

@@ -113,7 +113,7 @@ def test_post_to_get_inbound_sms_for_service_respects_data_retention(
too_old_date,
returned_date
):
create_service_data_retention(sample_service.id, 'sms', days_of_retention)
create_service_data_retention(sample_service, 'sms', days_of_retention)
create_inbound_sms(sample_service, created_at=too_old_date)
returned_inbound = create_inbound_sms(sample_service, created_at=returned_date)
@@ -217,7 +217,7 @@ def test_get_most_recent_inbound_sms_for_service_respects_data_retention(
admin_request,
sample_service
):
create_service_data_retention(sample_service.id, 'sms', 5)
create_service_data_retention(sample_service, 'sms', 5)
for i in range(10):
created = datetime.utcnow() - timedelta(days=i)
create_inbound_sms(sample_service, user_number='44770090000{}'.format(i), created_at=created)
@@ -240,7 +240,7 @@ def test_get_most_recent_inbound_sms_for_service_respects_data_retention_if_olde
admin_request,
sample_service
):
create_service_data_retention(sample_service.id, 'sms', 14)
create_service_data_retention(sample_service, 'sms', 14)
create_inbound_sms(sample_service, created_at=datetime(2017, 4, 1, 12, 0))
response = admin_request.get('inbound_sms.get_most_recent_inbound_sms_for_service', service_id=sample_service.id)
@@ -254,7 +254,7 @@ def test_get_inbound_sms_for_service_respects_data_retention(
admin_request,
sample_service
):
create_service_data_retention(sample_service.id, 'sms', 5)
create_service_data_retention(sample_service, 'sms', 5)
for i in range(10):
created = datetime.utcnow() - timedelta(days=i)
create_inbound_sms(sample_service, user_number='44770090000{}'.format(i), created_at=created)

View File

@@ -7,10 +7,10 @@ from tests.app.db import create_service_data_retention
def test_get_service_data_retention(client, sample_service):
sms_data_retention = create_service_data_retention(service_id=sample_service.id)
email_data_retention = create_service_data_retention(service_id=sample_service.id, notification_type='email',
sms_data_retention = create_service_data_retention(service=sample_service)
email_data_retention = create_service_data_retention(service=sample_service, notification_type='email',
days_of_retention=10)
letter_data_retention = create_service_data_retention(service_id=sample_service.id, notification_type='letter',
letter_data_retention = create_service_data_retention(service=sample_service, notification_type='letter',
days_of_retention=30)
response = client.get(
@@ -36,7 +36,7 @@ def test_get_service_data_retention_returns_empty_list(client, sample_service):
def test_get_data_retention_for_service_notification_type(client, sample_service):
data_retention = create_service_data_retention(service_id=sample_service.id)
data_retention = create_service_data_retention(service=sample_service)
response = client.get('/service/{}/data-retention/notification-type/{}'.format(sample_service.id, 'sms'),
headers=[('Content-Type', 'application/json'), create_authorization_header()],
)
@@ -45,10 +45,10 @@ def test_get_data_retention_for_service_notification_type(client, sample_service
def test_get_service_data_retention_by_id(client, sample_service):
sms_data_retention = create_service_data_retention(service_id=sample_service.id)
create_service_data_retention(service_id=sample_service.id, notification_type='email',
sms_data_retention = create_service_data_retention(service=sample_service)
create_service_data_retention(service=sample_service, notification_type='email',
days_of_retention=10)
create_service_data_retention(service_id=sample_service.id, notification_type='letter',
create_service_data_retention(service=sample_service, notification_type='letter',
days_of_retention=30)
response = client.get(
'/service/{}/data-retention/{}'.format(str(sample_service.id), sms_data_retention.id),
@@ -105,7 +105,7 @@ def test_create_service_data_retention_returns_400_when_notification_type_is_inv
def test_create_service_data_retention_returns_400_when_data_retention_for_notification_type_already_exists(
client, sample_service
):
create_service_data_retention(service_id=sample_service.id)
create_service_data_retention(service=sample_service)
data = {
"notification_type": "sms",
"days_of_retention": 3
@@ -123,7 +123,7 @@ def test_create_service_data_retention_returns_400_when_data_retention_for_notif
def test_modify_service_data_retention(client, sample_service):
data_retention = create_service_data_retention(service_id=sample_service.id)
data_retention = create_service_data_retention(service=sample_service)
data = {
"days_of_retention": 3
}