From aeb79a0de9ab92b6d20f270c205eabf67ddb6607 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 28 Dec 2016 12:30:21 +0000 Subject: [PATCH 1/5] refactor sample_user to create a new create_user function this create_user function can be imported for use in creating specific users in your tests, for example ``` from tests.app.db import create_user def test_create_user_persists_to_database(notify_db_session): user = create_user() assert User.query.count() == 1 ``` this has the benefit of not requiring you to pass the notify_db and notify_db_session fixtures around, and separating custom object creation from the fixture dependency trees to aid clarity I started with sample_user since it has no downstream dependencies, but the intention is to push this out to all db fixtures eventually. This is a total conversion, but can be rolled out in a non-breaking manner by keeping arguments in the fixture, and passing them through to the new db function - then tests can be updated to use the create_* instead of sample_* functions as and when you want to --- tests/app/conftest.py | 41 ++++++++++++++--------------------------- tests/app/db.py | 17 +++++++++++++++++ 2 files changed, 31 insertions(+), 27 deletions(-) create mode 100644 tests/app/db.py 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/db.py b/tests/app/db.py new file mode 100644 index 000000000..136ce5c50 --- /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' + } + usr = User.query.filter_by(email_address=email).first() + if not usr: + usr = User(**data) + save_model_user(usr) + return usr From d55089337771eb8a871970e11bb29065d1a79ed1 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 28 Dec 2016 12:30:26 +0000 Subject: [PATCH 2/5] 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), From 996bd2579afcea8e0421e8e3d48e488d8d4c297b Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 28 Dec 2016 13:35:41 +0000 Subject: [PATCH 3/5] fix failing tests make sure we're using notify_db_session to ensure test independence --- tests/app/dao/test_users_dao.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index db3805039..75ddd279f 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -46,16 +46,17 @@ def test_get_all_users(notify_db_session): def test_get_user(notify_db_session): - user = create_user(email='1@test.com') + 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_db, fake_uuid): +def test_get_user_not_exists(notify_db_session, fake_uuid): with pytest.raises(NoResultFound): get_user_by_id(user_id=fake_uuid) -def test_get_user_invalid_id(notify_db): +def test_get_user_invalid_id(notify_db_session): with pytest.raises(DataError): get_user_by_id(user_id="blah") From 44a8526807a78554f8aaa38160948890064ed827 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 28 Dec 2016 13:38:55 +0000 Subject: [PATCH 4/5] use `pytest.mark.usefixtures` instead of funcargs when you invoke the fixture `sample_user`, it does two things: it creates the user in the database, but also returns the user, a useful object that you may want to manipulate or reference in your test. however, when you invoke the fixture `notify_db_session`, it doesn't do anything - rather, it *promises* to clear up the database tables at the end of the test run. because we have no need of the notify_db_session object in our tests (indeed, for a long time this fixture just returned `None`), using `pytest.mark.usefixtures('notify_db_session')` brings attention to the fact that this is a side-effect fixture rather than a data setup fixture. Functionally it is identical to passing as a parameter --- tests/app/dao/test_users_dao.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index 75ddd279f..0bc417b21 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, @@ -21,7 +21,8 @@ from app.models import User, VerifyCode from tests.app.db import create_user -def test_create_user(notify_db_session): +@pytest.mark.usefixtures('notify_db_session') +def test_create_user(): email = 'notify@digital.cabinet-office.gov.uk' data = { 'name': 'Test User', @@ -37,7 +38,8 @@ def test_create_user(notify_db_session): assert not user.platform_admin -def test_get_all_users(notify_db_session): +@pytest.mark.usefixtures('notify_db_session') +def test_get_all_users(): create_user(email='1@test.com') create_user(email='2@test.com') @@ -45,18 +47,21 @@ def test_get_all_users(notify_db_session): assert len(get_user_by_id()) == 2 -def test_get_user(notify_db_session): +@pytest.mark.usefixtures('notify_db_session') +def test_get_user(): 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_db_session, fake_uuid): +@pytest.mark.usefixtures('notify_db_session') +def test_get_user_not_exists(fake_uuid): with pytest.raises(NoResultFound): get_user_by_id(user_id=fake_uuid) -def test_get_user_invalid_id(notify_db_session): +@pytest.mark.usefixtures('notify_db_session') +def test_get_user_invalid_id(): with pytest.raises(DataError): get_user_by_id(user_id="blah") From 0f327a0995a73ddfa193e9babb4dbc953ebc64db Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 10 Jan 2017 15:00:10 +0000 Subject: [PATCH 5/5] remove pytest.mark.usefixtures decorator while it's nice to use the decorator to signify fixtures with side effects, it has unfortunate problems of completely overriding any fixtures you've declared in the funcargs - so isn't really suitable for our usecase where we often have other fixtures we rely on to return values to us. So for consistency, let's remove this and stick to using funcargs to define our fixtures --- tests/app/dao/test_users_dao.py | 15 +++++---------- tests/app/db.py | 10 +++++----- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index 0bc417b21..6b4b982b7 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -21,8 +21,7 @@ from app.models import User, VerifyCode from tests.app.db import create_user -@pytest.mark.usefixtures('notify_db_session') -def test_create_user(): +def test_create_user(notify_db_session): email = 'notify@digital.cabinet-office.gov.uk' data = { 'name': 'Test User', @@ -38,8 +37,7 @@ def test_create_user(): assert not user.platform_admin -@pytest.mark.usefixtures('notify_db_session') -def test_get_all_users(): +def test_get_all_users(notify_db_session): create_user(email='1@test.com') create_user(email='2@test.com') @@ -47,21 +45,18 @@ def test_get_all_users(): assert len(get_user_by_id()) == 2 -@pytest.mark.usefixtures('notify_db_session') -def test_get_user(): +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 -@pytest.mark.usefixtures('notify_db_session') -def test_get_user_not_exists(fake_uuid): +def test_get_user_not_exists(notify_db_session, fake_uuid): with pytest.raises(NoResultFound): get_user_by_id(user_id=fake_uuid) -@pytest.mark.usefixtures('notify_db_session') -def test_get_user_invalid_id(): +def test_get_user_invalid_id(notify_db_session): with pytest.raises(DataError): get_user_by_id(user_id="blah") diff --git a/tests/app/db.py b/tests/app/db.py index 136ce5c50..cb64f86db 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -10,8 +10,8 @@ def create_user(mobile_number="+447700900986", email="notify@digital.cabinet-off 'mobile_number': mobile_number, 'state': 'active' } - usr = User.query.filter_by(email_address=email).first() - if not usr: - usr = User(**data) - save_model_user(usr) - return usr + user = User.query.filter_by(email_address=email).first() + if not user: + user = User(**data) + save_model_user(user) + return user