From dd2e754730f65054fd03621672eab00fc97bc5c9 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 7 Feb 2017 12:47:07 +0000 Subject: [PATCH 1/3] Refactor test into loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes it less repetitive, and will let us add more assertions for each month that we’re checking. --- tests/app/dao/test_services_dao.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 1a76eb50d..fde32621f 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -526,23 +526,25 @@ def test_fetch_monthly_historical_stats_separates_weeks(notify_db, notify_db_ses result = dao_fetch_monthly_historical_stats_for_service(sample_template.service_id, 2016) - assert result['2016-04']['sms']['created'] == 1 - assert result['2016-04']['sms']['sending'] == 0 - assert result['2016-04']['sms']['delivered'] == 0 - assert result['2016-04']['sms']['pending'] == 0 - assert result['2016-04']['sms']['failed'] == 0 - assert result['2016-04']['sms']['technical-failure'] == 0 - assert result['2016-04']['sms']['temporary-failure'] == 0 - assert result['2016-04']['sms']['permanent-failure'] == 0 + for date, status, count in ( + ('2016-04', 'sending', 0), + ('2016-04', 'delivered', 0), + ('2016-04', 'pending', 0), + ('2016-04', 'failed', 0), + ('2016-04', 'technical-failure', 0), + ('2016-04', 'temporary-failure', 0), + ('2016-04', 'permanent-failure', 0), - assert result['2016-06']['sms']['created'] == 1 + ('2016-06', 'created', 1), - assert result['2016-10']['sms']['created'] == 1 + ('2016-10', 'created', 1), - assert result['2016-12']['sms']['created'] == 0 - assert result['2016-12']['sms']['delivered'] == 1 + ('2016-12', 'created', 0), + ('2016-12', 'delivered', 1), - assert result['2017-03']['sms']['created'] == 2 + ('2017-03', 'created', 2), + ): + assert result[date]['sms'][status] == count assert result.keys() == { '2016-04', '2016-05', '2016-06', From 4805ccdeed20b54438778def19a4b6bcdf895564 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 7 Feb 2017 12:47:50 +0000 Subject: [PATCH 2/3] Check email and letter count in monthly stats test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before we were only checking counts for SMS, because we’d only created SMS notifications. This commit also checks the counts for email and letter, which should be 0. But they’re not. So this commit is adding the test case which demonstrates the bug. --- tests/app/dao/test_services_dao.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index fde32621f..af72342d0 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -545,6 +545,8 @@ def test_fetch_monthly_historical_stats_separates_weeks(notify_db, notify_db_ses ('2017-03', 'created', 2), ): assert result[date]['sms'][status] == count + assert result[date]['email'][status] == count + assert result[date]['letter'][status] == count assert result.keys() == { '2016-04', '2016-05', '2016-06', From 0dfa4a93d5e9bcd7045b7482f571dec5987aa91a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 7 Feb 2017 13:06:32 +0000 Subject: [PATCH 3/3] Make sure status dictionary is assinged each time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The status dictionary was being assigned once, and then subsequent uses of it were by reference. This meant that each template type was pointing at the same dictionary, and updating one meant updating all. This commit adds a dictionary comprehension, which gets evaluated once for each template type, so each template type has its own `dict` of statuses. Before -- ``` Email SMS Letter | | | {'sending':, 'failed', …} ``` After -- ``` Email SMS Letter | | | {'sending':, {'sending':, {'sending':, 'failed', 'failed', 'failed', …} …} …} ``` --- app/dao/services_dao.py | 8 ++++---- tests/app/dao/test_services_dao.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index b9ad66ae9..867f103b3 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -267,13 +267,13 @@ def dao_fetch_monthly_historical_stats_for_service(service_id, year): ) months = { - datetime.strftime(date, '%Y-%m'): dict.fromkeys( - TEMPLATE_TYPES, - dict.fromkeys( + datetime.strftime(date, '%Y-%m'): { + template_type: dict.fromkeys( NOTIFICATION_STATUS_TYPES, 0 ) - ) + for template_type in TEMPLATE_TYPES + } for date in [ datetime(year, month, 1) for month in range(4, 13) ] + [ diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index af72342d0..6e4afa8b3 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -545,8 +545,8 @@ def test_fetch_monthly_historical_stats_separates_weeks(notify_db, notify_db_ses ('2017-03', 'created', 2), ): assert result[date]['sms'][status] == count - assert result[date]['email'][status] == count - assert result[date]['letter'][status] == count + assert result[date]['email'][status] == 0 + assert result[date]['letter'][status] == 0 assert result.keys() == { '2016-04', '2016-05', '2016-06',