From d55089337771eb8a871970e11bb29065d1a79ed1 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 28 Dec 2016 12:30:26 +0000 Subject: [PATCH] update tests to use create_user instead of sample_user note that all of these tests have to be checked to ensure that they still call through to notify_db_session (notify_db not required) to tear down the database after the test runs - since it's no longer required to pass it in to the function just to invoke the sample_user function --- tests/app/celery/test_tasks.py | 14 ++--- tests/app/dao/test_jobs_dao.py | 4 +- tests/app/dao/test_users_dao.py | 54 ++++++++----------- .../rest/test_send_notification.py | 2 +- tests/app/service/test_api_key_endpoints.py | 4 +- tests/app/service/test_rest.py | 30 ++++------- 6 files changed, 42 insertions(+), 66 deletions(-) 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/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..db3805039 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -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,42 @@ 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): + user = create_user(email='1@test.com') + 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, 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): 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 +80,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 +93,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/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),