From d805985a4ecd4cef40b43288c76fcafbcd312cbd Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Mon, 27 Feb 2017 13:18:42 +0000 Subject: [PATCH] Refactor tests to use cleaner fixture and be more verbose in tests --- tests/app/celery/test_scheduled_tasks.py | 50 ++++++++++++---------- tests/app/dao/test_provider_details_dao.py | 28 +++++++----- 2 files changed, 44 insertions(+), 34 deletions(-) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index c7d78d53b..06ab343fa 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -60,9 +60,10 @@ def _create_slow_delivery_notification(provider='mmg'): @pytest.fixture(scope='function') -def set_provider_updated_at(current_sms_provider): - current_sms_provider.updated_at = datetime.utcnow() - timedelta(minutes=30) - dao_update_provider_details(current_sms_provider) +def prepare_current_provider(restore_provider_details): + initial_provider = get_current_provider('sms') + initial_provider.updated_at = datetime.utcnow() - timedelta(minutes=30) + dao_update_provider_details(initial_provider) def test_should_have_decorated_tasks_functions(): @@ -297,7 +298,7 @@ def test_switch_current_sms_provider_on_slow_delivery_does_not_run_if_config_uns def test_switch_providers_on_slow_delivery_runs_if_config_set( notify_api, mocker, - set_provider_updated_at + prepare_current_provider ): get_notifications_mock = mocker.patch( 'app.celery.scheduled_tasks.is_delivery_slow_for_provider', @@ -315,29 +316,29 @@ def test_switch_providers_on_slow_delivery_runs_if_config_set( def test_switch_providers_triggers_on_slow_notification_delivery( notify_api, - restore_provider_details, - current_sms_provider, - set_provider_updated_at + prepare_current_provider ): + starting_provider = get_current_provider('sms') + with set_config_values(notify_api, { 'FUNCTIONAL_TEST_PROVIDER_SERVICE_ID': '7954469d-8c6d-43dc-b8f7-86be2d69f5f3', 'FUNCTIONAL_TEST_PROVIDER_SMS_TEMPLATE_ID': '331a63e6-f1aa-4588-ad3f-96c268788ae7' }): - _create_slow_delivery_notification(current_sms_provider.identifier) - _create_slow_delivery_notification(current_sms_provider.identifier) + _create_slow_delivery_notification(starting_provider.identifier) + _create_slow_delivery_notification(starting_provider.identifier) switch_current_sms_provider_on_slow_delivery() new_provider = get_current_provider('sms') - assert new_provider.identifier != current_sms_provider.identifier - assert new_provider.priority < current_sms_provider.priority + assert new_provider.identifier != starting_provider.identifier + assert new_provider.priority < starting_provider.priority def test_switch_providers_on_slow_delivery_does_not_switch_if_already_switched( notify_api, - restore_provider_details, - current_sms_provider, - set_provider_updated_at + prepare_current_provider ): + starting_provider = get_current_provider('sms') + with set_config_values(notify_api, { 'FUNCTIONAL_TEST_PROVIDER_SERVICE_ID': '7954469d-8c6d-43dc-b8f7-86be2d69f5f3', 'FUNCTIONAL_TEST_PROVIDER_SMS_TEMPLATE_ID': '331a63e6-f1aa-4588-ad3f-96c268788ae7' @@ -349,15 +350,13 @@ def test_switch_providers_on_slow_delivery_does_not_switch_if_already_switched( switch_current_sms_provider_on_slow_delivery() new_provider = get_current_provider('sms') - assert new_provider.identifier != current_sms_provider.identifier - assert new_provider.priority < current_sms_provider.priority + assert new_provider.identifier != starting_provider.identifier + assert new_provider.priority < starting_provider.priority def test_switch_providers_on_slow_delivery_does_not_switch_based_on_older_notifications( notify_api, - restore_provider_details, - current_sms_provider, - set_provider_updated_at + prepare_current_provider ): """ Assume we have three slow delivery notifications for the current provider x. This triggers @@ -368,24 +367,29 @@ def test_switch_providers_on_slow_delivery_does_not_switch_based_on_older_notifi based on these as they are old. We only want to look for slow notifications after the point at which we switched back to provider x. """ + starting_provider = get_current_provider('sms') with set_config_values(notify_api, { 'FUNCTIONAL_TEST_PROVIDER_SERVICE_ID': '7954469d-8c6d-43dc-b8f7-86be2d69f5f3', 'FUNCTIONAL_TEST_PROVIDER_SMS_TEMPLATE_ID': '331a63e6-f1aa-4588-ad3f-96c268788ae7' }): # Provider x -> y - _create_slow_delivery_notification(current_sms_provider.identifier) - _create_slow_delivery_notification(current_sms_provider.identifier) - _create_slow_delivery_notification(current_sms_provider.identifier) + _create_slow_delivery_notification(starting_provider.identifier) + _create_slow_delivery_notification(starting_provider.identifier) + _create_slow_delivery_notification(starting_provider.identifier) switch_current_sms_provider_on_slow_delivery() current_provider = get_current_provider('sms') + assert current_provider.identifier != starting_provider.identifier # Provider y -> x _create_slow_delivery_notification(current_provider.identifier) _create_slow_delivery_notification(current_provider.identifier) switch_current_sms_provider_on_slow_delivery() + new_provider = get_current_provider('sms') + assert new_provider.identifier != current_provider.identifier + # Expect to stay on provider x switch_current_sms_provider_on_slow_delivery() current_provider = get_current_provider('sms') - assert current_sms_provider.identifier == current_provider.identifier + assert starting_provider.identifier == current_provider.identifier diff --git a/tests/app/dao/test_provider_details_dao.py b/tests/app/dao/test_provider_details_dao.py index b9569e37e..9add4fb9c 100644 --- a/tests/app/dao/test_provider_details_dao.py +++ b/tests/app/dao/test_provider_details_dao.py @@ -19,7 +19,7 @@ from app.dao.provider_details_dao import ( ) -def set_primary_sms_provider(identifier='mmg'): +def set_primary_sms_provider(restore_provider_details, identifier='mmg'): primary_provider = get_provider_details_by_identifier(identifier) secondary_provider = get_alternative_sms_provider(identifier) @@ -153,22 +153,28 @@ def test_toggle_sms_provider_switches_provider( ): dao_toggle_sms_provider(current_sms_provider.identifier) new_provider = get_current_provider('sms') - assert new_provider.identifier != current_sms_provider.identifier - assert new_provider.priority < current_sms_provider.priority + + old_starting_provider = get_provider_details_by_identifier(current_sms_provider.identifier) + + assert new_provider.identifier != old_starting_provider.identifier + assert new_provider.priority < old_starting_provider.priority def test_toggle_sms_provider_switches_when_provider_priorities_are_equal( - restore_provider_details, - current_sms_provider + restore_provider_details ): - new_provider = get_alternative_sms_provider(current_sms_provider.identifier) - current_sms_provider.priority = new_provider.priority - dao_update_provider_details(current_sms_provider) + current_provider = get_current_provider('sms') + new_provider = get_alternative_sms_provider(current_provider.identifier) - dao_toggle_sms_provider(current_sms_provider.identifier) + current_provider.priority = new_provider.priority + dao_update_provider_details(current_provider) - assert new_provider.identifier != current_sms_provider.identifier - assert new_provider.priority < current_sms_provider.priority + dao_toggle_sms_provider(current_provider.identifier) + + old_starting_provider = get_provider_details_by_identifier(current_provider.identifier) + + assert new_provider.identifier != old_starting_provider.identifier + assert new_provider.priority < old_starting_provider.priority def test_toggle_sms_provider_updates_provider_history(