diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 7e3cff311..4315d24f6 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -17,12 +17,12 @@ from app.models import Notification, KEY_TYPE_TEAM, KEY_TYPE_TEST, KEY_TYPE_NORM from tests.app import load_example_csv from tests.app.conftest import ( sample_service, - sample_user, sample_template, sample_job, sample_email_template, sample_notification ) +from tests.app.db import create_user class AnyStringWith(str): @@ -393,7 +393,7 @@ def test_should_put_send_sms_task_in_research_mode_queue_if_research_mode_servic def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notify_db_session, mocker): - user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900890") + user = create_user(mobile_number="07700 900890") service = sample_service(notify_db, notify_db_session, user=user, restricted=True) template = sample_template(notify_db, notify_db_session, service=service) notification = _notification_json(template, "+447700900890") # The user’s own number, but in a different format @@ -429,7 +429,7 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif def test_should_send_sms_if_restricted_service_and_non_team_number_with_test_key(notify_db, notify_db_session, mocker): - user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900205") + user = create_user(mobile_number="07700 900205") service = sample_service(notify_db, notify_db_session, user=user, restricted=True) template = sample_template(notify_db, notify_db_session, service=service) @@ -455,7 +455,7 @@ def test_should_send_sms_if_restricted_service_and_non_team_number_with_test_key def test_should_send_email_if_restricted_service_and_non_team_email_address_with_test_key(notify_db, notify_db_session, mocker): - user = sample_user(notify_db, notify_db_session) + user = create_user() service = sample_service(notify_db, notify_db_session, user=user, restricted=True) template = sample_template( notify_db, notify_db_session, service=service, template_type='email', subject_line='Hello' @@ -481,7 +481,7 @@ def test_should_send_email_if_restricted_service_and_non_team_email_address_with def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db, notify_db_session, mocker): - user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900205") + user = create_user(mobile_number="07700 900205") service = sample_service(notify_db, notify_db_session, user=user, restricted=True) template = sample_template(notify_db, notify_db_session, service=service) @@ -500,7 +500,7 @@ def test_should_not_send_sms_if_restricted_service_and_invalid_number(notify_db, def test_should_not_send_email_if_restricted_service_and_invalid_email_address(notify_db, notify_db_session, mocker): - user = sample_user(notify_db, notify_db_session) + user = create_user() service = sample_service(notify_db, notify_db_session, user=user, restricted=True) template = sample_template( notify_db, notify_db_session, service=service, template_type='email', subject_line='Hello' @@ -617,7 +617,7 @@ def test_should_not_send_email_if_team_key_and_recipient_not_in_team(sample_emai def test_should_not_send_sms_if_team_key_and_recipient_not_in_team(notify_db, notify_db_session, mocker): assert Notification.query.count() == 0 - user = sample_user(notify_db, notify_db_session, mobile_numnber="07700 900205") + user = create_user(mobile_number="07700 900205") service = sample_service(notify_db, notify_db_session, user=user, restricted=True) template = sample_template(notify_db, notify_db_session, service=service) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 4dbf288fa..ea702ecd4 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -25,7 +25,7 @@ from app.models import ( ServiceWhitelist, KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM, MOBILE_TYPE, EMAIL_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED) -from app.dao.users_dao import (save_model_user, create_user_code, create_secret_code) +from app.dao.users_dao import (create_user_code, create_secret_code) from app.dao.services_dao import (dao_create_service, dao_add_user_to_service) from app.dao.templates_dao import dao_create_template from app.dao.api_key_dao import save_model_api_key @@ -35,6 +35,8 @@ from app.dao.invited_user_dao import save_invited_user from app.dao.provider_rates_dao import create_provider_rates from app.clients.sms.firetext import FiretextClient +from tests.app.db import create_user + @pytest.yield_fixture def rmock(): @@ -47,7 +49,7 @@ def service_factory(notify_db, notify_db_session): class ServiceFactory(object): def get(self, service_name, user=None, template_type=None, email_from=None): if not user: - user = sample_user(notify_db, notify_db_session) + user = create_user() if not email_from: email_from = service_name service = sample_service(notify_db, notify_db_session, service_name, user, email_from=email_from) @@ -71,30 +73,15 @@ def service_factory(notify_db, notify_db_session): @pytest.fixture(scope='function') -def sample_user(notify_db, - notify_db_session, - mobile_numnber="+447700900986", - email="notify@digital.cabinet-office.gov.uk"): - data = { - 'name': 'Test User', - 'email_address': email, - 'password': 'password', - 'mobile_number': mobile_numnber, - 'state': 'active' - } - usr = User.query.filter_by(email_address=email).first() - if not usr: - usr = User(**data) - save_model_user(usr) - - return usr +def sample_user(notify_db_session): + return create_user() def create_code(notify_db, notify_db_session, code_type, usr=None, code=None): if code is None: code = create_secret_code() if usr is None: - usr = sample_user(notify_db, notify_db_session) + usr = create_user() return create_user_code(usr, code, code_type), code @@ -138,7 +125,7 @@ def sample_service(notify_db, limit=1000, email_from=None): if user is None: - user = sample_user(notify_db, notify_db_session) + user = create_user() if email_from is None: email_from = service_name.lower().replace(' ', '.') data = { @@ -171,11 +158,11 @@ def sample_template(notify_db, service=None, created_by=None): if user is None: - user = sample_user(notify_db, notify_db_session) + user = create_user() if service is None: service = sample_service(notify_db, notify_db_session) if created_by is None: - created_by = sample_user(notify_db, notify_db_session) + created_by = create_user() data = { 'name': template_name, 'template_type': template_type, @@ -210,7 +197,7 @@ def sample_email_template( subject_line='Email Subject', service=None): if user is None: - user = sample_user(notify_db, notify_db_session) + user = create_user() if service is None: service = sample_service(notify_db, notify_db_session) data = { @@ -590,7 +577,7 @@ def sample_permission(notify_db, user=None, permission="manage_settings"): if user is None: - user = sample_user(notify_db, notify_db_session) + user = create_user() data = { 'user': user, 'permission': permission @@ -617,7 +604,7 @@ def sample_service_permission(notify_db, user=None, permission="manage_settings"): if user is None: - user = sample_user(notify_db, notify_db_session) + user = create_user() if service is None: service = sample_service(notify_db, notify_db_session, user=user) data = { @@ -816,7 +803,7 @@ def create_notify_template(service, user, template_config_name, content, templat def notify_service(notify_db, notify_db_session): - user = sample_user(notify_db, notify_db_session) + user = create_user() service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID']) if not service: data = { diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index b9ccd326d..dca1eb7d8 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -20,7 +20,7 @@ from tests.app.conftest import sample_notification as create_notification from tests.app.conftest import sample_job as create_job from tests.app.conftest import sample_service as create_service from tests.app.conftest import sample_template as create_template -from tests.app.conftest import sample_user as create_user +from tests.app.db import create_user def test_should_have_decorated_notifications_dao_functions(): @@ -140,7 +140,7 @@ def test_get_job_by_id(sample_job): def test_get_jobs_for_service(notify_db, notify_db_session, sample_template): one_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template) - other_user = create_user(notify_db, notify_db_session, email="test@digital.cabinet-office.gov.uk") + other_user = create_user(email="test@digital.cabinet-office.gov.uk") other_service = create_service(notify_db, notify_db_session, user=other_user, service_name="other service", email_from='other.service') other_template = create_template(notify_db, notify_db_session, service=other_service) diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index cc814b160..6b4b982b7 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -1,10 +1,10 @@ from datetime import datetime, timedelta + from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound - -from app import db import pytest +from app import db from app.dao.users_dao import ( save_model_user, save_user_attribute, @@ -16,11 +16,12 @@ from app.dao.users_dao import ( delete_codes_older_created_more_than_a_day_ago, ) -from tests.app.conftest import sample_user as create_sample_user from app.models import User, VerifyCode +from tests.app.db import create_user -def test_create_user(notify_api, notify_db, notify_db_session): + +def test_create_user(notify_db_session): email = 'notify@digital.cabinet-office.gov.uk' data = { 'name': 'Test User', @@ -36,53 +37,43 @@ def test_create_user(notify_api, notify_db, notify_db_session): assert not user.platform_admin -def test_get_all_users(notify_api, notify_db, notify_db_session, sample_user): - assert User.query.count() == 1 - assert len(get_user_by_id()) == 1 - email = "another.notify@digital.cabinet-office.gov.uk" - another_user = create_sample_user(notify_db, - notify_db_session, - email=email) +def test_get_all_users(notify_db_session): + create_user(email='1@test.com') + create_user(email='2@test.com') + assert User.query.count() == 2 assert len(get_user_by_id()) == 2 -def test_get_user(notify_api, notify_db, notify_db_session): - email = "another.notify@digital.cabinet-office.gov.uk" - another_user = create_sample_user(notify_db, - notify_db_session, - email=email) - assert get_user_by_id(user_id=another_user.id).email_address == email +def test_get_user(notify_db_session): + email = '1@test.com' + user = create_user(email=email) + assert get_user_by_id(user_id=user.id).email_address == email -def test_get_user_not_exists(notify_api, notify_db, notify_db_session, fake_uuid): - try: +def test_get_user_not_exists(notify_db_session, fake_uuid): + with pytest.raises(NoResultFound): get_user_by_id(user_id=fake_uuid) - pytest.fail("NoResultFound exception not thrown.") - except NoResultFound as e: - pass -def test_get_user_invalid_id(notify_api, notify_db, notify_db_session): +def test_get_user_invalid_id(notify_db_session): with pytest.raises(DataError): get_user_by_id(user_id="blah") -def test_delete_users(notify_api, notify_db, notify_db_session, sample_user): +def test_delete_users(sample_user): assert User.query.count() == 1 delete_model_user(sample_user) assert User.query.count() == 0 -def test_increment_failed_login_should_increment_failed_logins(notify_api, notify_db, notify_db_session, sample_user): - assert User.query.count() == 1 +def test_increment_failed_login_should_increment_failed_logins(sample_user): assert sample_user.failed_login_count == 0 increment_failed_login_count(sample_user) assert sample_user.failed_login_count == 1 -def test_reset_failed_login_should_set_failed_logins_to_0(notify_api, notify_db, notify_db_session, sample_user): - assert User.query.count() == 1 +def test_reset_failed_login_should_set_failed_logins_to_0(sample_user): increment_failed_login_count(sample_user) assert sample_user.failed_login_count == 1 reset_failed_login_count(sample_user) @@ -90,8 +81,7 @@ def test_reset_failed_login_should_set_failed_logins_to_0(notify_api, notify_db, def test_get_user_by_email(sample_user): - email = sample_user.email_address - user_from_db = get_user_by_email(email) + user_from_db = get_user_by_email(sample_user.email_address) assert sample_user == user_from_db @@ -104,19 +94,18 @@ def test_get_user_by_email_is_case_insensitive(sample_user): def test_should_delete_all_verification_codes_more_than_one_day_old(sample_user): make_verify_code(sample_user, age=timedelta(hours=24), code="54321") make_verify_code(sample_user, age=timedelta(hours=24), code="54321") - assert len(VerifyCode.query.all()) == 2 + assert VerifyCode.query.count() == 2 delete_codes_older_created_more_than_a_day_ago() - assert len(VerifyCode.query.all()) == 0 + assert VerifyCode.query.count() == 0 def test_should_not_delete_verification_codes_less_than_one_day_old(sample_user): make_verify_code(sample_user, age=timedelta(hours=23, minutes=59, seconds=59), code="12345") make_verify_code(sample_user, age=timedelta(hours=24), code="54321") - assert len(VerifyCode.query.all()) == 2 + assert VerifyCode.query.count() == 2 delete_codes_older_created_more_than_a_day_ago() - assert len(VerifyCode.query.all()) == 1 - assert VerifyCode.query.first()._code == "12345" + assert VerifyCode.query.one()._code == "12345" def make_verify_code(user, age=timedelta(hours=0), code="12335"): diff --git a/tests/app/db.py b/tests/app/db.py new file mode 100644 index 000000000..cb64f86db --- /dev/null +++ b/tests/app/db.py @@ -0,0 +1,17 @@ +from app.models import User +from app.dao.users_dao import save_model_user + + +def create_user(mobile_number="+447700900986", email="notify@digital.cabinet-office.gov.uk"): + data = { + 'name': 'Test User', + 'email_address': email, + 'password': 'password', + 'mobile_number': mobile_number, + 'state': 'active' + } + user = User.query.filter_by(email_address=email).first() + if not user: + user = User(**data) + save_model_user(user) + return user diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index a1c532ff2..637d7771e 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -445,7 +445,7 @@ def test_should_not_return_html_in_body(notify_api, notify_db, notify_db_session with notify_api.test_request_context(): with notify_api.test_client() as client: mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') - email_template = create_sample_email_template(notify_db, notify_db.session, content='hello\nthere') + email_template = create_sample_email_template(notify_db, notify_db_session, content='hello\nthere') data = { 'to': 'ok@ok.com', diff --git a/tests/app/service/test_api_key_endpoints.py b/tests/app/service/test_api_key_endpoints.py index dfee92e2c..4f8c0b84d 100644 --- a/tests/app/service/test_api_key_endpoints.py +++ b/tests/app/service/test_api_key_endpoints.py @@ -7,7 +7,7 @@ from app.dao.api_key_dao import expire_api_key from tests import create_authorization_header from tests.app.conftest import sample_api_key as create_sample_api_key from tests.app.conftest import sample_service as create_sample_service -from tests.app.conftest import sample_user as create_user +from tests.app.db import create_user def test_api_key_should_create_new_api_key_for_service(notify_api, sample_service): @@ -101,7 +101,7 @@ def test_get_api_keys_should_return_all_keys_for_service(notify_api, notify_db, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: - another_user = create_user(notify_db, notify_db_session, email='another@it.gov.uk') + another_user = create_user(email='another@it.gov.uk') another_service = create_sample_service( notify_db, diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 8a291a1a6..805f27f3f 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -13,11 +13,12 @@ from tests import create_authorization_header from tests.app.conftest import ( sample_service as create_service, sample_service_permission as create_service_permission, - sample_user as create_sample_user, sample_notification as create_sample_notification, sample_notification_with_job) from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST +from tests.app.db import create_user + def test_get_service_list(notify_api, service_factory): with notify_api.test_request_context(): @@ -56,13 +57,11 @@ def test_get_service_list_with_only_active_flag(client, service_factory): def test_get_service_list_with_user_id_and_only_active_flag( - notify_db, - notify_db_session, client, sample_user, service_factory ): - other_user = create_sample_user(notify_db, notify_db_session, email='foo@bar.gov.uk') + other_user = create_user(email='foo@bar.gov.uk') inactive = service_factory.get('one', user=sample_user) active = service_factory.get('two', user=sample_user) @@ -81,8 +80,8 @@ def test_get_service_list_with_user_id_and_only_active_flag( assert json_resp['data'][0]['id'] == str(active.id) -def test_get_service_list_by_user(notify_db, notify_db_session, client, sample_user, service_factory): - other_user = create_sample_user(notify_db, notify_db_session, email='foo@bar.gov.uk') +def test_get_service_list_by_user(client, 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) @@ -99,14 +98,9 @@ def test_get_service_list_by_user(notify_db, notify_db_session, client, sample_u assert json_resp['data'][1]['name'] == 'two' -def test_get_service_list_by_user_should_return_empty_list_if_no_services( - notify_db, - notify_db_session, - client, - sample_service -): +def test_get_service_list_by_user_should_return_empty_list_if_no_services(client, sample_service): # service is already created by sample user - new_user = create_sample_user(notify_db, notify_db_session, email='foo@bar.gov.uk') + new_user = create_user(email='foo@bar.gov.uk') auth_header = create_authorization_header() response = client.get( @@ -943,10 +937,7 @@ def test_add_unknown_user_to_service_returns404(notify_api, notify_db, notify_db def test_remove_user_from_service(notify_api, notify_db, notify_db_session, sample_service_permission): with notify_api.test_request_context(): with notify_api.test_client() as client: - second_user = create_sample_user( - notify_db, - notify_db_session, - email="new@digital.cabinet-office.gov.uk") + second_user = create_user(email="new@digital.cabinet-office.gov.uk") # Simulates successfully adding a user to the service second_permission = create_service_permission( notify_db, @@ -966,10 +957,7 @@ def test_remove_user_from_service(notify_api, notify_db, notify_db_session, samp def test_remove_user_from_service(notify_api, notify_db, notify_db_session, sample_service_permission): with notify_api.test_request_context(): with notify_api.test_client() as client: - second_user = create_sample_user( - notify_db, - notify_db_session, - email="new@digital.cabinet-office.gov.uk") + second_user = create_user(email="new@digital.cabinet-office.gov.uk") endpoint = url_for( 'service.remove_user_from_service', service_id=str(sample_service_permission.service.id),