From 5c4f3e246c3f39dddf07d5716dead6b481df55fb Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 17 Jul 2018 15:27:43 +0100 Subject: [PATCH] make test dvla response file timestamps in a random order since there'll be a bunch of threads running functional test tasks at the same time, there's no point always trying to start from the same second and then stepping back to the same one-second-back file each time. Also, this leads us to an increased risk of race conditions. This change takes the same thirty second range, but shuffles it. The tests, since they're no longer deterministic, now use a new Matcher object (w/ credit to alexey) to match any filename from within that thirty second range --- app/celery/research_mode_tasks.py | 5 ++- tests/app/celery/test_research_mode_tasks.py | 39 ++++++++++++++------ tests/conftest.py | 12 ++++++ 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/app/celery/research_mode_tasks.py b/app/celery/research_mode_tasks.py index 5fbe09b01..b03d8ee69 100644 --- a/app/celery/research_mode_tasks.py +++ b/app/celery/research_mode_tasks.py @@ -1,3 +1,4 @@ +import random from datetime import datetime, timedelta import json @@ -125,8 +126,8 @@ def create_fake_letter_response_file(self, reference): now = datetime.utcnow() dvla_response_data = '{}|Sent|0|Sorted'.format(reference) - # try and find a filename that hasn't been taken yet - going back in time 60 seconds - for i in range(30): + # try and find a filename that hasn't been taken yet - from a random time within the last 30 seconds + for i in sorted(range(30), key=lambda _: random.random()): upload_file_name = 'NOTIFY-{}-RSP.TXT'.format((now - timedelta(seconds=i)).strftime('%Y%m%d%H%M%S')) if not file_exists(current_app.config['DVLA_RESPONSE_BUCKET_NAME'], upload_file_name): break diff --git a/tests/app/celery/test_research_mode_tasks.py b/tests/app/celery/test_research_mode_tasks.py index aacfd81d5..7a00f7163 100644 --- a/tests/app/celery/test_research_mode_tasks.py +++ b/tests/app/celery/test_research_mode_tasks.py @@ -15,7 +15,13 @@ from app.celery.research_mode_tasks import ( ses_notification_callback, create_fake_letter_response_file, ) -from tests.conftest import set_config_values +from tests.conftest import set_config_values, Matcher + + +dvla_response_file_matcher = Matcher( + 'dvla_response_file', + lambda x: 'NOTIFY-20180125140000-RSP.TXT' < x <= 'NOTIFY-20180125140030-RSP.TXT' +) def test_make_mmg_callback(notify_api, rmock): @@ -108,12 +114,11 @@ def test_failure_firetext_callback(phone_number): } -@freeze_time("2018-01-25 14:00:00") +@freeze_time("2018-01-25 14:00:30") def test_create_fake_letter_response_file_uploads_response_file_s3( notify_api, mocker): mocker.patch('app.celery.research_mode_tasks.file_exists', return_value=False) mock_s3upload = mocker.patch('app.celery.research_mode_tasks.s3upload') - filename = 'NOTIFY-20180125140000-RSP.TXT' with requests_mock.Mocker() as request_mock: request_mock.post( @@ -128,16 +133,15 @@ def test_create_fake_letter_response_file_uploads_response_file_s3( filedata='random-ref|Sent|0|Sorted', region=current_app.config['AWS_REGION'], bucket_name=current_app.config['DVLA_RESPONSE_BUCKET_NAME'], - file_location=filename + file_location=dvla_response_file_matcher ) -@freeze_time("2018-01-25 14:00:00") +@freeze_time("2018-01-25 14:00:30") def test_create_fake_letter_response_file_calls_dvla_callback_on_development( notify_api, mocker): mocker.patch('app.celery.research_mode_tasks.file_exists', return_value=False) mocker.patch('app.celery.research_mode_tasks.s3upload') - filename = 'NOTIFY-20180125140000-RSP.TXT' with set_config_values(notify_api, { 'NOTIFY_ENVIRONMENT': 'development' @@ -154,11 +158,22 @@ def test_create_fake_letter_response_file_calls_dvla_callback_on_development( assert request_mock.last_request.json() == { "Type": "Notification", "MessageId": "some-message-id", - "Message": '{"Records":[{"s3":{"object":{"key":"' + filename + '"}}}]}' + "Message": ANY + } + assert json.loads(request_mock.last_request.json()['Message']) == { + "Records": [ + { + "s3": { + "object": { + "key": dvla_response_file_matcher + } + } + } + ] } -@freeze_time("2018-01-25 14:00:00") +@freeze_time("2018-01-25 14:00:30") def test_create_fake_letter_response_file_does_not_call_dvla_callback_on_preview( notify_api, mocker): mocker.patch('app.celery.research_mode_tasks.file_exists', return_value=False) @@ -181,15 +196,15 @@ def test_create_fake_letter_response_file_tries_to_create_files_with_other_filen create_fake_letter_response_file('random-ref') assert mock_file_exists.mock_calls == [ - call('test.notify.com-ftp', 'NOTIFY-20180125140030-RSP.TXT'), - call('test.notify.com-ftp', 'NOTIFY-20180125140029-RSP.TXT'), - call('test.notify.com-ftp', 'NOTIFY-20180125140028-RSP.TXT'), + call('test.notify.com-ftp', dvla_response_file_matcher), + call('test.notify.com-ftp', dvla_response_file_matcher), + call('test.notify.com-ftp', dvla_response_file_matcher), ] mock_s3upload.assert_called_once_with( filedata=ANY, region=ANY, bucket_name=ANY, - file_location='NOTIFY-20180125140028-RSP.TXT' + file_location=dvla_response_file_matcher ) diff --git a/tests/conftest.py b/tests/conftest.py index ee47643ce..e3762afd9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -162,3 +162,15 @@ def set_config_values(app, dict): finally: for key in dict: app.config[key] = old_values[key] + + +class Matcher: + def __init__(self, description, key): + self.description = description + self.key = key + + def __eq__(self, other): + return self.key(other) + + def __repr__(self): + return ''.format(self.description)