From 2182b5715ebe96ceefa99212bc597c4619690d55 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 29 Nov 2016 13:27:57 +0000 Subject: [PATCH] Refactored a couple of tests that were slightly testing the wrong thing. - Ensure mock used correctly. --- app/notifications/validators.py | 1 + tests/app/notifications/test_validators.py | 38 ++++++++++++---------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 2be1e54f5..5631e9284 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -12,6 +12,7 @@ def check_service_message_limit(key_type, service): if key_type != KEY_TYPE_TEST: cache_key = redis.daily_limit_cache_key(service.id) service_stats = redis_store.get(cache_key) + print(service_stats) if not service_stats: service_stats = services_dao.fetch_todays_total_message_count(service.id) redis_store.set(cache_key, service_stats, ex=3600) diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 2e1974359..37f13b78b 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -21,25 +21,29 @@ from tests.app.conftest import ( @pytest.mark.parametrize('key_type', ['team', 'normal']) -def test_exception_thown_by_redis_store_get_should_not_be_fatal( +def test_exception_thrown_by_redis_store_get_should_not_be_fatal( notify_db, notify_db_session, notify_api, key_type, mocker): - mocker.patch('app.notifications.validators.redis_store.redis_store.get', side_effect=Exception("broken redis")) - mocker.patch('app.notifications.validators.redis_store.set') + with freeze_time("2016-01-01 12:00:00.000000"): - service = create_service(notify_db, notify_db_session, restricted=True, limit=4) - for x in range(5): - create_notification(notify_db, notify_db_session, service=service) + mocker.patch('app.notifications.validators.redis_store.redis_store.get', side_effect=Exception("broken redis")) + mocker.patch('app.notifications.validators.redis_store.redis_store.set') - with pytest.raises(TooManyRequestsError) as e: - check_service_message_limit(key_type, service) - assert e.value.status_code == 429 - assert e.value.message == 'Exceeded send limits (4) for today' - assert e.value.fields == [] - app.notifications.validators.redis_store.set.assert_not_called() + service = create_service(notify_db, notify_db_session, restricted=True, limit=4) + for x in range(5): + create_notification(notify_db, notify_db_session, service=service) + + with pytest.raises(TooManyRequestsError) as e: + check_service_message_limit(key_type, service) + assert e.value.status_code == 429 + assert e.value.message == 'Exceeded send limits (4) for today' + assert e.value.fields == [] + app.notifications.validators.redis_store.redis_store.set.assert_called_with( + "{}-2016-01-01-count".format(str(service.id)), 5, 3600, None, False, False + ) @pytest.mark.parametrize('key_type', ['test', 'team', 'normal']) @@ -53,14 +57,14 @@ def test_exception_thown_by_redis_store_set_should_not_be_fatal( @pytest.mark.parametrize('key_type', ['test', 'team', 'normal']) -def test_check_service_message_limit_in_cache_with_unrestricted_service_passes( +def test_check_service_message_limit_in_cache_with_unrestricted_service_is_allowed( key_type, sample_service, mocker): mocker.patch('app.notifications.validators.redis_store.get', return_value=1) mocker.patch('app.notifications.validators.redis_store.set') mocker.patch('app.notifications.validators.services_dao') - assert not check_service_message_limit(key_type, sample_service) + check_service_message_limit(key_type, sample_service) app.notifications.validators.redis_store.set.assert_not_called() assert not app.notifications.validators.services_dao.mock_calls @@ -73,14 +77,14 @@ def test_check_service_message_limit_in_cache_under_message_limit_passes( mocker.patch('app.notifications.validators.redis_store.get', return_value=1) mocker.patch('app.notifications.validators.redis_store.set') mocker.patch('app.notifications.validators.services_dao') - assert not check_service_message_limit(key_type, sample_service) + check_service_message_limit(key_type, sample_service) app.notifications.validators.redis_store.set.assert_not_called() assert not app.notifications.validators.services_dao.mock_calls def test_should_not_interact_with_cache_for_test_key(sample_service, mocker): mocker.patch('app.notifications.validators.redis_store') - assert not check_service_message_limit('test', sample_service) + check_service_message_limit('test', sample_service) assert not app.notifications.validators.redis_store.mock_calls @@ -97,7 +101,7 @@ def test_should_set_cache_value_as_value_from_database_if_cache_not_set( create_notification(notify_db, notify_db_session, service=sample_service) mocker.patch('app.notifications.validators.redis_store.get', return_value=None) mocker.patch('app.notifications.validators.redis_store.set') - assert not check_service_message_limit(key_type, sample_service) + check_service_message_limit(key_type, sample_service) app.notifications.validators.redis_store.set.assert_called_with( str(sample_service.id) + "-2016-01-01-count", 5, ex=3600 )