From ac7665bfc625146a62549d201044e8f65e950a8c Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 19 Jun 2017 14:58:38 +0100 Subject: [PATCH] celery test cleanup * Alter config so an error will be raised if you forget to mock out a celery call in one of your tests * Remove an unneeded exception type that was masking errors --- app/config.py | 2 ++ app/errors.py | 6 ------ app/notifications/__init__.py | 7 ------- app/notifications/process_notifications.py | 7 +++---- app/v2/errors.py | 8 -------- .../app/notifications/rest/test_send_notification.py | 12 +++++++----- tests/app/notifications/test_process_notification.py | 3 +-- tests/app/service/test_rest.py | 4 +++- 8 files changed, 16 insertions(+), 33 deletions(-) diff --git a/app/config.py b/app/config.py index 5fc8bfd39..8852f6782 100644 --- a/app/config.py +++ b/app/config.py @@ -297,6 +297,8 @@ class Test(Config): STATSD_HOST = "localhost" STATSD_PORT = 1000 + BROKER_URL = 'you-forgot-to-mock-celery-in-your-tests://' + for queue in QueueNames.all_queues(): Config.CELERY_QUEUES.append( Queue(queue, Exchange('default'), routing_key=queue) diff --git a/app/errors.py b/app/errors.py index 5a0d4710a..a6119d98d 100644 --- a/app/errors.py +++ b/app/errors.py @@ -7,7 +7,6 @@ from sqlalchemy.orm.exc import NoResultFound from marshmallow import ValidationError from jsonschema import ValidationError as JsonSchemaValidationError from app.authentication.auth import AuthError -from app.notifications import SendNotificationToQueueError class InvalidRequest(Exception): @@ -90,11 +89,6 @@ def register_errors(blueprint): current_app.logger.exception(e) return jsonify(result='error', message="No result found"), 404 - @blueprint.errorhandler(SendNotificationToQueueError) - def failed_to_create_notification(e): - current_app.logger.exception(e) - return jsonify(result='error', message=e.message), 500 - @blueprint.errorhandler(SQLAlchemyError) def db_error(e): current_app.logger.exception(e) diff --git a/app/notifications/__init__.py b/app/notifications/__init__.py index a5a575f60..e69de29bb 100644 --- a/app/notifications/__init__.py +++ b/app/notifications/__init__.py @@ -1,7 +0,0 @@ - - -class SendNotificationToQueueError(Exception): - status_code = 500 - - def __init__(self): - self.message = "Failed to create the notification" diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index b1cf05e84..6709bc502 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -17,7 +17,7 @@ from app.models import SMS_TYPE, Notification, KEY_TYPE_TEST, EMAIL_TYPE, Schedu from app.dao.notifications_dao import (dao_create_notification, dao_delete_notifications_and_history_by_id, dao_created_scheduled_notification) -from app.v2.errors import BadRequestError, SendNotificationToQueueError +from app.v2.errors import BadRequestError from app.utils import get_template_instance, cache_key_for_service_template_counter, convert_bst_to_utc @@ -110,10 +110,9 @@ def send_notification_to_queue(notification, research_mode, queue=None): try: deliver_task.apply_async([str(notification.id)], queue=queue) - except Exception as e: - current_app.logger.exception(e) + except Exception: dao_delete_notifications_and_history_by_id(notification.id) - raise SendNotificationToQueueError() + raise current_app.logger.info( "{} {} sent to the {} queue for delivery".format(notification.notification_type, diff --git a/app/v2/errors.py b/app/v2/errors.py index cb441deb9..d38477474 100644 --- a/app/v2/errors.py +++ b/app/v2/errors.py @@ -5,7 +5,6 @@ from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound from app.authentication.auth import AuthError from app.errors import InvalidRequest -from app.notifications import SendNotificationToQueueError class TooManyRequestsError(InvalidRequest): @@ -61,13 +60,6 @@ def register_errors(blueprint): def auth_error(error): return jsonify(error.to_dict_v2()), error.code - @blueprint.errorhandler(SendNotificationToQueueError) - def failed_to_create_notification(error): - current_app.logger.exception(error) - return jsonify( - status_code=500, - errors=[{"error": error.__class__.__name__, "message": error.message}]), 500 - @blueprint.errorhandler(Exception) def internal_server_error(error): current_app.logger.exception(error) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 5781770f5..9e8577873 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -755,13 +755,15 @@ def test_should_delete_notification_and_return_error_if_sqs_fails( save_model_api_key(api_key) auth_header = create_jwt_token(secret=api_key.secret, client_id=str(api_key.service_id)) - response = client.post( - path='/notifications/{}'.format(template_type), - data=json.dumps(data), - headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))]) + with pytest.raises(Exception) as e: + client.post( + path='/notifications/{}'.format(template_type), + data=json.dumps(data), + headers=[('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(auth_header))] + ) + assert str(e.value) == 'failed to talk to SQS' mocked.assert_called_once_with([fake_uuid], queue='send-tasks') - assert response.status_code == 500 assert not notifications_dao.get_notification_by_id(fake_uuid) assert not NotificationHistory.query.get(fake_uuid) diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 3220ef2ed..e7f0c6eae 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -8,7 +8,6 @@ from freezegun import freeze_time from collections import namedtuple from app.models import Template, Notification, NotificationHistory, ScheduledNotification -from app.notifications import SendNotificationToQueueError from app.notifications.process_notifications import ( create_content_for_notification, persist_notification, @@ -238,7 +237,7 @@ def test_send_notification_to_queue(notify_db, notify_db_session, def test_send_notification_to_queue_throws_exception_deletes_notification(sample_notification, mocker): mocked = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async', side_effect=Boto3Error("EXPECTED")) - with pytest.raises(SendNotificationToQueueError): + with pytest.raises(Boto3Error): send_notification_to_queue(sample_notification, False) mocked.assert_called_once_with([(str(sample_notification.id))], queue='send-sms') diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 157bc6dc2..26fa9034e 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2247,7 +2247,9 @@ def test_fetch_service_inbound_api(client, sample_service): assert json.loads(response.get_data(as_text=True))["data"] == service_inbound_api.serialize() -def test_send_one_off_notification(admin_request, sample_template): +def test_send_one_off_notification(admin_request, sample_template, mocker): + mocker.patch('app.service.send_notification.send_notification_to_queue') + response = admin_request.post( 'service.create_one_off_notification', service_id=sample_template.service_id,