From b1cccdcc6b8694a35a40461bf69d18ca30f39ff7 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 28 Nov 2017 17:00:01 +0000 Subject: [PATCH] First batch of flake8 changes. Many unused variables, and replacing some old fixtures with admin_request before I realised just how many there where :weary: --- .../test_receive_notification.py | 9 +- tests/app/notifications/test_rest.py | 36 ++--- tests/app/service/test_rest.py | 142 +++++------------- tests/app/service/test_statistics.py | 2 - .../service/test_suspend_resume_service.py | 2 +- tests/app/template/test_rest.py | 73 ++------- tests/app/user/test_rest_verify.py | 1 - .../notifications/test_get_notifications.py | 5 +- .../notifications/test_post_notifications.py | 16 +- 9 files changed, 84 insertions(+), 202 deletions(-) diff --git a/tests/app/notifications/test_receive_notification.py b/tests/app/notifications/test_receive_notification.py index 1139ecf04..982cd0ac1 100644 --- a/tests/app/notifications/test_receive_notification.py +++ b/tests/app/notifications/test_receive_notification.py @@ -1,4 +1,3 @@ -import uuid import base64 from datetime import datetime from unittest.mock import call @@ -6,7 +5,6 @@ from unittest.mock import call import pytest from flask import json -from app.dao.services_dao import dao_fetch_service_by_inbound_number from app.notifications.receive_notifications import ( format_mmg_message, format_mmg_datetime, @@ -80,7 +78,7 @@ def test_receive_notification_from_mmg_without_permissions_does_not_persist( permissions ): mocked = mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async") - service = create_service_with_inbound_number(inbound_number='07111111111', service_permissions=permissions) + create_service_with_inbound_number(inbound_number='07111111111', service_permissions=permissions) data = { "ID": "1234", "MSISDN": "07111111111", @@ -113,8 +111,7 @@ def test_receive_notification_from_firetext_without_permissions_does_not_persist return_value=service) mocked_send_inbound_sms = mocker.patch( "app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async") - mocked_has_permissions = mocker.patch( - "app.notifications.receive_notifications.has_inbound_sms_permissions", return_value=False) + mocker.patch("app.notifications.receive_notifications.has_inbound_sms_permissions", return_value=False) data = "source=07999999999&destination=07111111111&message=this is a message&time=2017-01-01 12:00:00" response = firetext_post(client, data) @@ -339,7 +336,7 @@ def test_receive_notification_from_firetext_persists_message(notify_db_session, def test_receive_notification_from_firetext_persists_message_with_normalized_phone(notify_db_session, client, mocker): mocker.patch("app.notifications.receive_notifications.tasks.send_inbound_sms_to_service.apply_async") - mock = mocker.patch('app.notifications.receive_notifications.statsd_client.incr') + mocker.patch('app.notifications.receive_notifications.statsd_client.incr') create_service_with_inbound_number( inbound_number='07111111111', service_name='b', service_permissions=[EMAIL_TYPE, SMS_TYPE, INBOUND_SMS_TYPE]) diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index 9341ecc02..f35e262c1 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -244,7 +244,7 @@ def test_get_all_notifications_only_returns_notifications_of_matching_type( @pytest.mark.parametrize('key_type', [KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST]) -def test_no_api_keys_return_job_notifications_by_default( +def test_do_not_return_job_notifications_by_default( client, sample_template, sample_job, @@ -254,7 +254,7 @@ def test_no_api_keys_return_job_notifications_by_default( normal_api_key = create_api_key(sample_template.service, KEY_TYPE_NORMAL) test_api_key = create_api_key(sample_template.service, KEY_TYPE_TEST) - job_notification = create_notification(sample_template, job=sample_job) + create_notification(sample_template, job=sample_job) normal_notification = create_notification(sample_template, api_key=normal_api_key) team_notification = create_notification(sample_template, api_key=team_api_key) test_notification = create_notification(sample_template, api_key=test_api_key) @@ -307,7 +307,7 @@ def test_only_normal_api_keys_can_return_job_notifications( key_type=KEY_TYPE_TEST) save_model_api_key(test_api_key) - job_notification = create_sample_notification( + create_sample_notification( notify_db, notify_db_session, job=sample_job @@ -381,8 +381,8 @@ def test_should_reject_invalid_page_param(client, sample_email_template): def test_valid_page_size_param(notify_api, notify_db, notify_db_session, sample_email_template): with notify_api.test_request_context(): - n1 = create_sample_notification(notify_db, notify_db_session) - n2 = create_sample_notification(notify_db, notify_db_session) + create_sample_notification(notify_db, notify_db_session) + create_sample_notification(notify_db, notify_db_session) with notify_api.test_client() as client: auth_header = create_authorization_header(service_id=sample_email_template.service_id) @@ -399,8 +399,8 @@ def test_valid_page_size_param(notify_api, notify_db, notify_db_session, sample_ def test_invalid_page_size_param(client, notify_db, notify_db_session, sample_email_template): - n1 = create_sample_notification(notify_db, notify_db_session) - n2 = create_sample_notification(notify_db, notify_db_session) + create_sample_notification(notify_db, notify_db_session) + create_sample_notification(notify_db, notify_db_session) auth_header = create_authorization_header(service_id=sample_email_template.service_id) response = client.get( @@ -454,12 +454,12 @@ def test_get_all_notifications_returns_empty_list(client, sample_api_key): def test_filter_by_template_type(client, notify_db, notify_db_session, sample_template, sample_email_template): - notification_1 = create_sample_notification( + create_sample_notification( notify_db, notify_db_session, service=sample_email_template.service, template=sample_template) - notification_2 = create_sample_notification( + create_sample_notification( notify_db, notify_db_session, service=sample_email_template.service, @@ -480,8 +480,8 @@ def test_filter_by_template_type(client, notify_db, notify_db_session, sample_te def test_filter_by_multiple_template_types(client, sample_template, sample_email_template): - notification_1 = create_notification(sample_template) - notification_2 = create_notification(sample_email_template) + create_notification(sample_template) + create_notification(sample_email_template) auth_header = create_authorization_header(service_id=sample_email_template.service_id) @@ -496,8 +496,8 @@ def test_filter_by_multiple_template_types(client, def test_filter_by_status(client, sample_email_template): - notification_1 = create_notification(sample_email_template, status="delivered") - notification_2 = create_notification(sample_email_template) + create_notification(sample_email_template, status="delivered") + create_notification(sample_email_template) auth_header = create_authorization_header(service_id=sample_email_template.service_id) @@ -512,8 +512,8 @@ def test_filter_by_status(client, sample_email_template): def test_filter_by_multiple_statuses(client, sample_email_template): - notification_1 = create_notification(sample_email_template, status="delivered") - notification_2 = create_notification(sample_email_template, status='sending') + create_notification(sample_email_template, status="delivered") + create_notification(sample_email_template, status='sending') auth_header = create_authorization_header(service_id=sample_email_template.service_id) @@ -529,9 +529,9 @@ def test_filter_by_multiple_statuses(client, sample_email_template): def test_filter_by_status_and_template_type(client, sample_template, sample_email_template): - notification_1 = create_notification(sample_template) - notification_2 = create_notification(sample_email_template) - notification_3 = create_notification(sample_email_template, status="delivered") + create_notification(sample_template) + create_notification(sample_email_template) + create_notification(sample_email_template, status="delivered") auth_header = create_authorization_header(service_id=sample_email_template.service_id) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 109430a3c..aff2c1a0a 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -77,7 +77,7 @@ def test_get_service_list_with_only_active_flag(client, service_factory): def test_get_service_list_with_user_id_and_only_active_flag( - client, + admin_request, sample_user, service_factory ): @@ -85,72 +85,47 @@ def test_get_service_list_with_user_id_and_only_active_flag( inactive = service_factory.get('one', user=sample_user) active = service_factory.get('two', user=sample_user) - from_other_user = service_factory.get('three', user=other_user) + # from other user + service_factory.get('three', user=other_user) inactive.active = False - auth_header = create_authorization_header() - response = client.get( - '/service?user_id={}&only_active=True'.format(sample_user.id), - headers=[auth_header] + json_resp = admin_request.get( + 'service.get_services', + user_id=sample_user.id, + only_active=True ) - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) assert len(json_resp['data']) == 1 assert json_resp['data'][0]['id'] == str(active.id) -def test_get_service_list_by_user(client, sample_user, service_factory): +def test_get_service_list_by_user(admin_request, sample_user, service_factory): other_user = create_user(email='foo@bar.gov.uk') service_factory.get('one', sample_user) service_factory.get('two', sample_user) service_factory.get('three', other_user) - auth_header = create_authorization_header() - response = client.get( - '/service?user_id={}'.format(sample_user.id), - headers=[auth_header] - ) - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 + json_resp = admin_request.get('service.get_services', user_id=sample_user.id) assert len(json_resp['data']) == 2 assert json_resp['data'][0]['name'] == 'one' assert json_resp['data'][1]['name'] == 'two' -def test_get_service_list_by_user_should_return_empty_list_if_no_services(client, sample_service): +def test_get_service_list_by_user_should_return_empty_list_if_no_services(admin_request, sample_service): # service is already created by sample user new_user = create_user(email='foo@bar.gov.uk') - auth_header = create_authorization_header() - response = client.get( - '/service?user_id={}'.format(new_user.id), - headers=[auth_header] - ) - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 + json_resp = admin_request.get('service.get_services', user_id=new_user.id) + assert json_resp['data'] == [] + + +def test_get_service_list_should_return_empty_list_if_no_services(admin_request): + json_resp = admin_request.get('service.get_services') assert len(json_resp['data']) == 0 -def test_get_service_list_should_return_empty_list_if_no_services(client): - auth_header = create_authorization_header() - response = client.get( - '/service', - headers=[auth_header] - ) - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 0 - - -def test_get_service_by_id(client, sample_service): - auth_header = create_authorization_header() - resp = client.get( - '/service/{}'.format(sample_service.id), - headers=[auth_header] - ) - assert resp.status_code == 200 - json_resp = json.loads(resp.get_data(as_text=True)) +def test_get_service_by_id(admin_request, sample_service): + json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id) assert json_resp['data']['name'] == sample_service.name assert json_resp['data']['id'] == str(sample_service.id) assert not json_resp['data']['research_mode'] @@ -161,55 +136,29 @@ def test_get_service_by_id(client, sample_service): assert json_resp['data']['prefix_sms'] is True -def test_get_service_by_id_returns_free_sms_limit(client, sample_service): - - auth_header = create_authorization_header() - resp = client.get( - '/service/{}'.format(sample_service.id), - headers=[auth_header] - ) - assert resp.status_code == 200 - json_resp = json.loads(resp.get_data(as_text=True)) +def test_get_service_by_id_returns_free_sms_limit(admin_request, sample_service): + json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id) assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] -def test_get_detailed_service_by_id_returns_free_sms_limit(client, sample_service): - - auth_header = create_authorization_header() - resp = client.get( - '/service/{}?detailed=True'.format(sample_service.id), - headers=[auth_header] - ) - assert resp.status_code == 200 - json_resp = json.loads(resp.get_data(as_text=True)) +def test_get_detailed_service_by_id_returns_free_sms_limit(admin_request, sample_service): + json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id, detailed=True) assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] -@pytest.mark.parametrize('endpoint', ['/service/{}', '/service/{}?detailed=True']) -def test_get_service_by_id_returns_organisation_type(client, sample_service, endpoint): - - auth_header = create_authorization_header() - resp = client.get( - endpoint.format(sample_service.id), - headers=[auth_header] - ) - assert resp.status_code == 200 - json_resp = json.loads(resp.get_data(as_text=True)) +@pytest.mark.parametrize('detailed', [True, False]) +def test_get_service_by_id_returns_organisation_type(admin_request, sample_service, detailed): + json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id, detailed=detailed) assert json_resp['data']['organisation_type'] is None -def test_get_service_list_has_default_permissions(client, service_factory): +def test_get_service_list_has_default_permissions(admin_request, service_factory): service_factory.get('one') service_factory.get('one') service_factory.get('two') service_factory.get('three') - auth_header = create_authorization_header() - response = client.get( - '/service', - headers=[auth_header] - ) - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) + + json_resp = admin_request.get('service.get_services') assert len(json_resp['data']) == 3 assert all( set( @@ -221,13 +170,8 @@ def test_get_service_list_has_default_permissions(client, service_factory): ) -def test_get_service_by_id_has_default_service_permissions(client, sample_service): - auth_header = create_authorization_header() - resp = client.get( - '/service/{}'.format(sample_service.id), - headers=[auth_header] - ) - json_resp = json.loads(resp.get_data(as_text=True)) +def test_get_service_by_id_has_default_service_permissions(admin_request, sample_service): + json_resp = admin_request.get('service.get_service_by_id', service_id=sample_service.id) assert set( json_resp['data']['permissions'] @@ -236,19 +180,15 @@ def test_get_service_by_id_has_default_service_permissions(client, sample_servic ]) -def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - service_id = str(uuid.uuid4()) - auth_header = create_authorization_header() - resp = client.get( - '/service/{}'.format(service_id), - headers=[auth_header] - ) - assert resp.status_code == 404 - json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp['result'] == 'error' - assert json_resp['message'] == 'No result found' +def test_get_service_by_id_should_404_if_no_service(admin_request, notify_db_session): + json_resp = admin_request.get( + 'service.get_service_by_id', + service_id=uuid.uuid4(), + _expected_status=404 + ) + + assert json_resp['result'] == 'error' + assert json_resp['message'] == 'No result found' def test_get_service_by_id_and_user(client, sample_service, sample_user): @@ -1596,7 +1536,7 @@ def test_get_detailed_service(notify_db, notify_db_session, notify_api, sample_s ] ) def test_get_monthly_notification_stats(mocker, client, sample_service, url, expected_status, expected_json): - mock_dao = mocker.patch( + mocker.patch( 'app.service.rest.dao_fetch_monthly_historical_stats_for_service', return_value={'foo': 'bar'}, ) @@ -2577,7 +2517,7 @@ def test_get_letter_contacts_when_there_are_no_letter_contacts(client, sample_se def test_get_letter_contacts_with_one_letter_contact(client, notify_db, notify_db_session): service = create_service() - letter_contact = create_letter_contact(service, 'Aberdeen, AB23 1XH') + create_letter_contact(service, 'Aberdeen, AB23 1XH') response = client.get('/service/{}/letter-contact'.format(service.id), headers=[create_authorization_header()]) diff --git a/tests/app/service/test_statistics.py b/tests/app/service/test_statistics.py index 9c35ad9c7..7f56a5b9a 100644 --- a/tests/app/service/test_statistics.py +++ b/tests/app/service/test_statistics.py @@ -1,8 +1,6 @@ -from datetime import datetime import collections import pytest -from freezegun import freeze_time from app.service.statistics import ( format_statistics, diff --git a/tests/app/service/test_suspend_resume_service.py b/tests/app/service/test_suspend_resume_service.py index 7f38d1be6..997e3101d 100644 --- a/tests/app/service/test_suspend_resume_service.py +++ b/tests/app/service/test_suspend_resume_service.py @@ -6,7 +6,7 @@ import uuid from freezegun import freeze_time -from app.models import Service, ApiKey +from app.models import Service from tests import create_authorization_header diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 6d666bfa3..e0394f860 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -1,4 +1,3 @@ -import uuid import json import random import string @@ -17,7 +16,7 @@ from tests.app.conftest import ( sample_template_without_letter_permission, sample_template_without_sms_permission, ) -from tests.app.db import create_service, create_letter_contact +from tests.app.db import create_service, create_letter_contact, create_template @pytest.mark.parametrize('template_type, subject', [ @@ -309,68 +308,18 @@ def test_should_be_able_to_get_all_templates_for_a_service(client, sample_user, assert update_json_resp['data'][1]['created_at'] -def test_should_get_only_templates_for_that_service(client, sample_user, service_factory): +def test_should_get_only_templates_for_that_service(admin_request, notify_db_sesision): + service_1 = create_service(service_name='service_1') + service_2 = create_service(service_name='service_2') + id_1 = create_template(service_1).id + id_2 = create_template(service_1).id + id_3 = create_template(service_2).id - service_1 = service_factory.get('service 1', email_from='service.1') - service_2 = service_factory.get('service 2', email_from='service.2') + json_resp_1 = admin_request.get('template.get_all_templates_for_service', service_id=service_1.id) + json_resp_2 = admin_request.get('template.get_all_templates_for_service', service_id=service_2.id) - auth_header_1 = create_authorization_header() - - response_1 = client.get( - '/service/{}/template'.format(service_1.id), - headers=[auth_header_1] - ) - - auth_header_2 = create_authorization_header() - - response_2 = client.get( - '/service/{}/template'.format(service_2.id), - headers=[auth_header_2] - ) - - assert response_1.status_code == 200 - assert response_2.status_code == 200 - - json_resp_1 = json.loads(response_1.get_data(as_text=True)) - json_resp_2 = json.loads(response_2.get_data(as_text=True)) - - assert len(json_resp_1['data']) == 1 - assert len(json_resp_2['data']) == 1 - - data = { - 'name': 'my template 2', - 'template_type': EMAIL_TYPE, - 'subject': 'subject 2', - 'content': 'template content', - 'service': str(service_1.id), - 'created_by': str(sample_user.id) - } - data = json.dumps(data) - create_auth_header = create_authorization_header() - resp = client.post( - '/service/{}/template'.format(service_1.id), - headers=[('Content-Type', 'application/json'), create_auth_header], - data=data - ) - - response_3 = client.get( - '/service/{}/template'.format(service_1.id), - headers=[auth_header_1] - ) - - response_4 = client.get( - '/service/{}/template'.format(service_2.id), - headers=[auth_header_2] - ) - - assert response_3.status_code == 200 - assert response_4.status_code == 200 - - json_resp_3 = json.loads(response_3.get_data(as_text=True)) - json_resp_4 = json.loads(response_4.get_data(as_text=True)) - - assert len(json_resp_3['data']) == 2 - assert len(json_resp_4['data']) == 1 + assert {template['id'] for template in json_resp_1['data']} == {str(id_1), str(id_2)} + assert {template['id'] for template in json_resp_2['data']} == {str(id_3)} @pytest.mark.parametrize( diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index d361987d6..c43507784 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -382,7 +382,6 @@ def test_send_user_email_code_with_urlencoded_next_param(admin_request, mocker, _expected_status=204 ) noti = Notification.query.one() - code = VerifyCode.query.one() assert noti.personalisation['url'].endswith('?next=%2Fservices') diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 6bef10a57..0e6a3d911 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -7,7 +7,7 @@ from tests import create_authorization_header from tests.app.db import ( create_notification, create_template, - create_service) +) from tests.app.conftest import ( sample_notification, @@ -30,7 +30,8 @@ def test_get_notification_by_id_returns_200( scheduled_for="2017-05-12 15:15" ) - another = create_notification( + # another + create_notification( template=sample_template, billable_units=billable_units, sent_by=provider, diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 10ca07c96..361f729ef 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -634,7 +634,7 @@ def test_post_email_notification_with_valid_reply_to_id_returns_201(client, samp def test_post_email_notification_with_invalid_reply_to_id_returns_400(client, sample_email_template, mocker, fake_uuid): - mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') + mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') data = { "email_address": sample_email_template.service.users[0].email_address, "template_id": sample_email_template.id, @@ -652,10 +652,9 @@ def test_post_email_notification_with_invalid_reply_to_id_returns_400(client, sa assert 'BadRequestError' in resp_json['errors'][0]['error'] -def test_persist_sender_to_notification_mapping_for_email(notify_db_session): - service = create_service() - template = create_template(service=service, template_type=EMAIL_TYPE) - sender = create_reply_to_email(service=service, email_address='reply@test.com', is_default=False) +def test_persist_sender_to_notification_mapping_for_email(sample_service): + template = create_template(service=sample_service, template_type=EMAIL_TYPE) + sender = create_reply_to_email(service=sample_service, email_address='reply@test.com', is_default=False) form = { "email_address": "recipient@test.com", "template_id": str(template.id), @@ -669,10 +668,9 @@ def test_persist_sender_to_notification_mapping_for_email(notify_db_session): assert notification_to_email_reply_to[0].service_email_reply_to_id == sender.id -def test_persist_sender_to_notification_mapping_for_sms(notify_db_session): - service = create_service() - template = create_template(service=service, template_type=SMS_TYPE) - sender = create_service_sms_sender(service=service, sms_sender='12345', is_default=False) +def test_persist_sender_to_notification_mapping_for_sms(sample_service): + template = create_template(service=sample_service, template_type=SMS_TYPE) + sender = create_service_sms_sender(service=sample_service, sms_sender='12345', is_default=False) form = { 'phone_number': '+447700900855', 'template_id': str(template.id),