Merge pull request #2614 from alphagov/test-updates

Make a few fixtures Pytest 4 compliant and fix tests with errors

tests/app/celery/test_ftp_update_tasks.py

@@ -182,7 +182,7 @@ def test_update_letter_notifications_to_error_updates_based_on_notification_refe
     with freeze_time(dt):
         with pytest.raises(NotificationTechnicalFailureException) as e:
             update_letter_notifications_to_error([first.reference])
-            assert first.reference in e.value
+    assert first.reference in str(e.value)
 
     assert first.status == NOTIFICATION_TECHNICAL_FAILURE
     assert first.sent_by is None

tests/app/celery/test_letters_pdf_tasks.py

@@ -684,7 +684,7 @@ def test_process_letter_task_check_virus_scan_error(sample_letter_notification,
     with pytest.raises(VirusScanError) as e:
         process_virus_scan_error(filename)
 
-    assert "Virus scan error:" in str(e)
+    assert "Virus scan error:" in str(e.value)
     mock_move_failed_pdf.assert_called_once_with(filename, ScanErrorType.ERROR)
     assert sample_letter_notification.status == NOTIFICATION_TECHNICAL_FAILURE
 

tests/app/celery/test_nightly_tasks.py

@@ -194,7 +194,7 @@ def test_update_status_of_notifications_after_timeout(notify_api, sample_templat
                 seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10))
         with pytest.raises(NotificationTechnicalFailureException) as e:
             timeout_notifications()
-            assert str(not2.id) in e.value
+        assert str(not2.id) in str(e.value)
         assert not1.status == 'temporary-failure'
         assert not2.status == 'technical-failure'
         assert not3.status == 'temporary-failure'

tests/app/celery/test_provider_tasks.py

@@ -65,7 +65,7 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_sms_task(s
 
     with pytest.raises(NotificationTechnicalFailureException) as e:
         deliver_sms(sample_notification.id)
-        assert sample_notification.id in e.value
+    assert str(sample_notification.id) in str(e.value)
 
     provider_tasks.deliver_sms.retry.assert_called_with(queue="retry-tasks", countdown=0)
 
@@ -78,7 +78,7 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_email_task
 
     with pytest.raises(NotificationTechnicalFailureException) as e:
         deliver_email(sample_notification.id)
-        assert sample_notification.id in e.value
+    assert str(sample_notification.id) in str(e.value)
 
     provider_tasks.deliver_email.retry.assert_called_with(queue="retry-tasks")
     assert sample_notification.status == 'technical-failure'

tests/app/conftest.py

@@ -272,17 +272,15 @@ def sample_template_without_letter_permission(notify_db, notify_db_session):
 
 
 @pytest.fixture(scope='function')
-def sample_template_with_placeholders(notify_db, notify_db_session):
+def sample_template_with_placeholders(sample_service):
     # deliberate space and title case in placeholder
-    return sample_template(notify_db, notify_db_session, content="Hello (( Name))\nYour thing is due soon")
+    return create_template(sample_service, content="Hello (( Name))\nYour thing is due soon")
 
 
 @pytest.fixture(scope='function')
-def sample_sms_template_with_html(notify_db, notify_db_session):
+def sample_sms_template_with_html(sample_service):
     # deliberate space and title case in placeholder
-    return sample_template(notify_db, notify_db_session, content=(
-        "Hello (( Name))\nHere is <em>some HTML</em> & entities"
-    ))
+    return create_template(sample_service, content="Hello (( Name))\nHere is <em>some HTML</em> & entities")
 
 
 @pytest.fixture(scope='function')
@@ -411,31 +409,20 @@ def sample_job(
 
 @pytest.fixture(scope='function')
 def sample_job_with_placeholdered_template(
-        notify_db,
-        notify_db_session,
-        service=None
+        sample_job,
+        sample_template_with_placeholders,
 ):
-    return sample_job(
-        notify_db,
-        notify_db_session,
-        service=service,
-        template=sample_template_with_placeholders(notify_db, notify_db_session)
-    )
+    sample_job.template = sample_template_with_placeholders
+
+    return sample_job
 
 
 @pytest.fixture(scope='function')
-def sample_scheduled_job(
-    notify_db,
-    notify_db_session,
-    service=None
-):
-    return sample_job(
-        notify_db,
-        notify_db_session,
-        service=service,
-        template=sample_template_with_placeholders(notify_db, notify_db_session),
-        scheduled_for=(datetime.utcnow() + timedelta(minutes=60)).isoformat(),
-        job_status='scheduled'
+def sample_scheduled_job(sample_template_with_placeholders):
+    return create_job(
+        sample_template_with_placeholders,
+        job_status='scheduled',
+        scheduled_for=(datetime.utcnow() + timedelta(minutes=60)).isoformat()
     )
 
 
@@ -628,18 +615,6 @@ def sample_letter_notification(sample_letter_template):
     return create_notification(sample_letter_template, reference='foo', personalisation=address)
 
 
-@pytest.fixture(scope='function')
-def sample_notification_with_api_key(notify_db, notify_db_session):
-    notification = sample_notification(notify_db, notify_db_session)
-    notification.api_key = sample_api_key(
-        notify_db,
-        notify_db_session,
-        name='Test key'
-    )
-    notification.api_key_id = notification.api_key.id
-    return notification
-
-
 @pytest.fixture(scope='function')
 def sample_email_notification(notify_db, notify_db_session):
     created_at = datetime.utcnow()

tests/app/dao/test_services_dao.py

@@ -293,17 +293,21 @@ def test_dao_add_user_to_service_ignores_folders_which_do_not_exist_when_setting
 
 
 def test_dao_add_user_to_service_raises_error_if_adding_folder_permissions_for_a_different_service(
-    sample_user,
     sample_service,
 ):
+    user = create_user()
     other_service = create_service(service_name='other service')
     other_service_folder = create_template_folder(other_service)
     folder_permissions = [str(other_service_folder.id)]
 
+    assert ServiceUser.query.count() == 2
+
     with pytest.raises(IntegrityError) as e:
-        dao_add_user_to_service(sample_service, sample_user, folder_permissions=folder_permissions)
-        assert 'insert or update on table "user_folder_permissions" violates foreign key constraint' in str(e.value)
-        assert ServiceUser.query.count() == 0
+        dao_add_user_to_service(sample_service, user, folder_permissions=folder_permissions)
+
+    db.session.rollback()
+    assert 'insert or update on table "user_folder_permissions" violates foreign key constraint' in str(e.value)
+    assert ServiceUser.query.count() == 2
 
 
 def test_should_remove_user_from_service(notify_db_session):

tests/app/db.py

@@ -511,11 +511,14 @@ def create_letter_rate(start_date=None, end_date=None, crown=True, sheet_count=1
     return rate
 
 
-def create_api_key(service, key_type=KEY_TYPE_NORMAL):
+def create_api_key(service, key_type=KEY_TYPE_NORMAL, key_name=None):
     id_ = uuid.uuid4()
+
+    name = key_name if key_name else '{} api key {}'.format(key_type, id_)
+
     api_key = ApiKey(
         service=service,
-        name='{} api key {}'.format(key_type, id_),
+        name=name,
         created_by=service.created_by,
         key_type=key_type,
         id=id_,

tests/app/delivery/test_send_to_providers.py

@@ -139,7 +139,7 @@ def test_should_not_send_email_message_when_service_is_inactive_notifcation_is_i
 
     with pytest.raises(NotificationTechnicalFailureException) as e:
         send_to_providers.send_email_to_provider(sample_notification)
-        assert sample_notification.id in e.value
+    assert str(sample_notification.id) in str(e.value)
     send_mock.assert_not_called()
     assert Notification.query.get(sample_notification.id).status == 'technical-failure'
 
@@ -152,7 +152,7 @@ def test_should_not_send_sms_message_when_service_is_inactive_notifcation_is_in_
 
     with pytest.raises(NotificationTechnicalFailureException) as e:
         send_to_providers.send_sms_to_provider(sample_notification)
-        assert sample_notification.id in e.value
+    assert str(sample_notification.id) in str(e.value)
     send_mock.assert_not_called()
     assert Notification.query.get(sample_notification.id).status == 'technical-failure'
 

tests/app/notifications/test_process_notification.py

@@ -287,7 +287,7 @@ def test_send_notification_to_queue_throws_exception_deletes_notification(sample
     mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async', side_effect=Boto3Error("EXPECTED"))
     with pytest.raises(Boto3Error):
         send_notification_to_queue(sample_notification, False)
-        mocked.assert_called_once_with([(str(sample_notification.id))], queue='send-sms')
+    mocked.assert_called_once_with([(str(sample_notification.id))], queue='send-sms-tasks')
 
     assert Notification.query.count() == 0
     assert NotificationHistory.query.count() == 0

tests/app/platform_stats/test_rest.py

@@ -97,8 +97,8 @@ def test_validate_date_range_is_within_a_financial_year(start_date, end_date):
 def test_validate_date_range_is_within_a_financial_year_raises(start_date, end_date):
     with pytest.raises(expected_exception=InvalidRequest) as e:
         validate_date_range_is_within_a_financial_year(start_date, end_date)
-        assert e.message == 'Date must be in a single financial year.'
-        assert e.code == 400
+    assert e.value.message == 'Date must be in a single financial year.'
+    assert e.value.status_code == 400
 
 
 def test_validate_date_is_within_a_financial_year_raises_validation_error():
@@ -107,8 +107,8 @@ def test_validate_date_is_within_a_financial_year_raises_validation_error():
 
     with pytest.raises(expected_exception=InvalidRequest) as e:
         validate_date_range_is_within_a_financial_year(start_date, end_date)
-        assert e.message == 'Start date must be before end date'
-        assert e.code == 400
+    assert e.value.message == 'Start date must be before end date'
+    assert e.value.status_code == 400
 
 
 @pytest.mark.parametrize('start_date, end_date',
@@ -116,9 +116,9 @@ def test_validate_date_is_within_a_financial_year_raises_validation_error():
                           ('2019-07-01', 'not-date')])
 def test_validate_date_is_within_a_financial_year_when_input_is_not_a_date(start_date, end_date):
     with pytest.raises(expected_exception=InvalidRequest) as e:
-        assert validate_date_range_is_within_a_financial_year(start_date, end_date)
-        assert e.message == 'Input must be a date in the format: YYYY-MM-DD'
-        assert e.code == 400
+        validate_date_range_is_within_a_financial_year(start_date, end_date)
+    assert e.value.message == 'Input must be a date in the format: YYYY-MM-DD'
+    assert e.value.status_code == 400
 
 
 def test_get_usage_for_all_services(notify_db_session, admin_request):

tests/app/test_schemas.py

@@ -4,6 +4,7 @@ from sqlalchemy import desc
 
 from app.dao.provider_details_dao import dao_update_provider_details
 from app.models import ProviderDetailsHistory
+from tests.app.db import create_api_key
 
 
 def test_job_schema_doesnt_return_notifications(sample_notification_with_job):
@@ -25,10 +26,13 @@ def test_notification_schema_ignores_absent_api_key(sample_notification_with_job
     assert data['key_name'] is None
 
 
-def test_notification_schema_adds_api_key_name(sample_notification_with_api_key):
+def test_notification_schema_adds_api_key_name(sample_notification):
     from app.schemas import notification_with_template_schema
 
-    data = notification_with_template_schema.dump(sample_notification_with_api_key).data
+    api_key = create_api_key(sample_notification.service, key_name='Test key')
+    sample_notification.api_key = api_key
+
+    data = notification_with_template_schema.dump(sample_notification).data
     assert data['key_name'] == 'Test key'