From 09e8ac9644e1478f125d6f9369798596c303c485 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 13 Sep 2019 11:40:05 +0100 Subject: [PATCH] Fix assertions when we catch an error in the tests Code that is within a `with Python.raises(...)` context manager but comes after the line that raises the exception doesn't get evaluated. We had some assertions that we never being tested because of this, so this ensures that they will always get run and fixes them where necessary. --- tests/app/celery/test_ftp_update_tasks.py | 2 +- tests/app/celery/test_letters_pdf_tasks.py | 2 +- tests/app/celery/test_nightly_tasks.py | 2 +- tests/app/celery/test_provider_tasks.py | 4 ++-- tests/app/dao/test_services_dao.py | 12 ++++++++---- tests/app/delivery/test_send_to_providers.py | 4 ++-- .../app/notifications/test_process_notification.py | 2 +- tests/app/platform_stats/test_rest.py | 14 +++++++------- 8 files changed, 23 insertions(+), 19 deletions(-) diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index 96ec98302..9b649d7b1 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/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 diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index fd500c062..e7bca98dd 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -681,7 +681,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 diff --git a/tests/app/celery/test_nightly_tasks.py b/tests/app/celery/test_nightly_tasks.py index 482531d79..bb2c29748 100644 --- a/tests/app/celery/test_nightly_tasks.py +++ b/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' diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index f4e8607fa..8ea1a5a8e 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/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' diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 74505f18d..1cc7311f4 100644 --- a/tests/app/dao/test_services_dao.py +++ b/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): diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 0a198f4f3..92c6f8fed 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/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' diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 656a7da47..cd36ef5e2 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/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 diff --git a/tests/app/platform_stats/test_rest.py b/tests/app/platform_stats/test_rest.py index f189968f0..45348e9bd 100644 --- a/tests/app/platform_stats/test_rest.py +++ b/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):