From ac0b2d79a2f0624cd688fff11b8f8c0500b6a9d2 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 9 Apr 2018 11:45:43 +0100 Subject: [PATCH] sys.exit rather than throwing an assertion error Assertions should only be used in tests - they can be disabled at runtime by setting the python flag -O (though I don't believe we use that flag under normal circumstances). also clean up test asserts - mock_redis is the redis object, so its `called` property will always be false, because we never say `redis_store()`. Rather, we should use the `mock_calls` property to see all calls to all of its children --- app/commands.py | 10 +++++++--- tests/app/commands/test_populate_redis.py | 14 +++++++++----- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/app/commands.py b/app/commands.py index a3cd9b6da..c1bc95710 100644 --- a/app/commands.py +++ b/app/commands.py @@ -1,3 +1,4 @@ +import sys import functools import uuid from datetime import datetime, timedelta @@ -504,9 +505,11 @@ def populate_redis_template_usage(service_id, day): Recalculate and replace the stats in redis for a day. To be used if redis data is lost for some reason. """ - # the day variable is set to midnight of that day - assert current_app.config['REDIS_ENABLED'] + if not current_app.config['REDIS_ENABLED']: + current_app.logger.error('Cannot populate redis template usage - redis not enabled') + sys.exit(1) + # the day variable is set by click to be midnight of that day start_time = get_london_midnight_in_utc(day) end_time = get_london_midnight_in_utc(day + timedelta(days=1)) @@ -533,5 +536,6 @@ def populate_redis_template_usage(service_id, day): redis_store.set_hash_and_expire( key, usage, - current_app.config['EXPIRE_CACHE_EIGHT_DAYS'] + current_app.config['EXPIRE_CACHE_EIGHT_DAYS'], + raise_exception=True ) diff --git a/tests/app/commands/test_populate_redis.py b/tests/app/commands/test_populate_redis.py index 8be92f277..25001642a 100644 --- a/tests/app/commands/test_populate_redis.py +++ b/tests/app/commands/test_populate_redis.py @@ -12,10 +12,12 @@ from tests.app.db import create_notification, create_template, create_service def test_populate_redis_template_usage_does_nothing_if_redis_disabled(mocker, notify_api, sample_service): mock_redis = mocker.patch('app.commands.redis_store') with set_config(notify_api, 'REDIS_ENABLED', False): - with pytest.raises(AssertionError): + with pytest.raises(SystemExit) as exit_signal: populate_redis_template_usage.callback.__wrapped__(sample_service.id, datetime.utcnow()) - assert not mock_redis.called + assert mock_redis.mock_calls == [] + # sys.exit with nonzero exit code + assert exit_signal.value.code != 0 def test_populate_redis_template_usage_does_nothing_if_no_data(mocker, notify_api, sample_service): @@ -23,7 +25,7 @@ def test_populate_redis_template_usage_does_nothing_if_no_data(mocker, notify_ap with set_config(notify_api, 'REDIS_ENABLED', True): populate_redis_template_usage.callback.__wrapped__(sample_service.id, datetime.utcnow()) - assert not mock_redis.called + assert mock_redis.mock_calls == [] @freeze_time('2017-06-12') @@ -41,7 +43,8 @@ def test_populate_redis_template_usage_only_populates_for_today(mocker, notify_a mock_redis.set_hash_and_expire.assert_called_once_with( 'service-{}-template-usage-2017-06-10'.format(sample_template.service_id), {str(sample_template.id): 3}, - notify_api.config['EXPIRE_CACHE_EIGHT_DAYS'] + notify_api.config['EXPIRE_CACHE_EIGHT_DAYS'], + raise_exception=True ) @@ -65,5 +68,6 @@ def test_populate_redis_template_usage_only_populates_for_given_service(mocker, mock_redis.set_hash_and_expire.assert_called_once_with( 'service-{}-template-usage-2017-06-10'.format(s1.id), {str(t1.id): 2}, - notify_api.config['EXPIRE_CACHE_EIGHT_DAYS'] + notify_api.config['EXPIRE_CACHE_EIGHT_DAYS'], + raise_exception=True )