From 6181c60f758e0f4cb1b982116523774a289d865c Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 3 May 2022 17:00:51 +0100 Subject: [PATCH] remove usage of notify_db fixture in unit tests * notify_db fixture creates the database connection and ensures the test db exists and has migrations applied etc. It will run once per session (test run). * notify_db_session fixture runs after your test finishes and deletes all non static (eg type table) data. In unit tests that hit the database (ie: most of them), 99% of the time we will need to use notify_db_session to ensure everything is reset. The only time we don't need to use it is when we're querying things such as "ensure get X works when database is empty". This is such a low percentage of tests that it's easier for us to just use notify_db_session every time, and ensure that all our tests run much more consistently, at the cost of a small bit of performance when running tests. We used to use notify_db to access the session object for manually adding, committing, etc. To dissuade usage of that fixture I've moved that to the `notify_db_session`. I've then removed all uses of notify_db that I could find in the codebase. As a note, if you're writing a test that uses a `sample_x` fixture, all of those fixtures rely on notify_db_session so you'll get the teardown functionality for free. If you're just calling eg `create_x` db.py functions, then you'll need to make you add notify_db_session fixture to your test, even if you aren't manually accessing the session. --- tests/app/broadcast_message/test_utils.py | 1 - .../celery/test_broadcast_message_tasks.py | 4 +- tests/app/celery/test_nightly_tasks.py | 10 +- .../celery/test_process_ses_receipts_tasks.py | 8 +- tests/app/celery/test_tasks.py | 4 +- tests/app/conftest.py | 28 ++--- .../notification_dao/test_notification_dao.py | 15 ++- tests/app/dao/test_api_key_dao.py | 2 +- tests/app/dao/test_complaint_dao.py | 2 +- tests/app/dao/test_daily_sorted_letter_dao.py | 5 +- tests/app/dao/test_email_branding_dao.py | 10 +- tests/app/dao/test_events_dao.py | 2 +- tests/app/dao/test_inbound_numbers_dao.py | 12 +- tests/app/dao/test_invited_user_dao.py | 16 ++- tests/app/dao/test_jobs_dao.py | 4 +- tests/app/dao/test_letter_branding_dao.py | 2 +- .../app/dao/test_service_callback_api_dao.py | 2 +- tests/app/dao/test_service_guest_list_dao.py | 6 +- tests/app/dao/test_service_inbound_api_dao.py | 2 +- tests/app/dao/test_service_permissions_dao.py | 2 +- tests/app/dao/test_services_dao.py | 4 +- tests/app/dao/test_users_dao.py | 2 +- tests/app/delivery/test_send_to_providers.py | 18 +-- tests/app/email_branding/test_rest.py | 12 +- .../test_process_notification.py | 1 - .../test_receive_notification.py | 2 +- tests/app/notifications/test_rest.py | 4 +- tests/app/notifications/test_validators.py | 1 - tests/app/provider_details/test_rest.py | 4 +- tests/app/service/test_archived_service.py | 4 +- tests/app/service/test_rest.py | 108 ++++++++---------- .../test_service_invite_rest.py | 4 +- tests/conftest.py | 12 +- 33 files changed, 151 insertions(+), 162 deletions(-) diff --git a/tests/app/broadcast_message/test_utils.py b/tests/app/broadcast_message/test_utils.py index c604821be..b29bbd0a2 100644 --- a/tests/app/broadcast_message/test_utils.py +++ b/tests/app/broadcast_message/test_utils.py @@ -270,7 +270,6 @@ def test_update_broadcast_message_status_rejects_approval_of_broadcast_with_no_a def test_update_broadcast_message_status_allows_trial_mode_services_to_approve_own_message( - notify_db, sample_broadcast_service, mocker ): diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index d67fcfe41..7efa6190f 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -77,7 +77,6 @@ def test_send_broadcast_event_calls_publish_govuk_alerts_task( def test_send_broadcast_event_only_sends_to_one_provider_if_set_on_service( mocker, - notify_db, notify_api, sample_broadcast_service ): @@ -101,7 +100,6 @@ def test_send_broadcast_event_only_sends_to_one_provider_if_set_on_service( def test_send_broadcast_event_does_nothing_if_provider_set_on_service_isnt_enabled_globally( mocker, - notify_db, notify_api, sample_broadcast_service ): @@ -186,7 +184,7 @@ def test_send_broadcast_provider_message_sends_data_correctly( ]) @pytest.mark.parametrize('channel', ['operator', 'test', 'severe', 'government']) def test_send_broadcast_provider_message_uses_channel_set_on_broadcast_service( - notify_db, mocker, sample_broadcast_service, provider, provider_capitalised, channel + mocker, sample_broadcast_service, provider, provider_capitalised, channel ): sample_broadcast_service.broadcast_channel = channel template = create_template(sample_broadcast_service, BROADCAST_TYPE) diff --git a/tests/app/celery/test_nightly_tasks.py b/tests/app/celery/test_nightly_tasks.py index 43c1395b3..318521a6f 100644 --- a/tests/app/celery/test_nightly_tasks.py +++ b/tests/app/celery/test_nightly_tasks.py @@ -52,7 +52,7 @@ def mock_s3_get_list_diff(bucket_name, subfolder='', suffix='', last_modified=No @freeze_time('2016-10-18T10:00:00') def test_will_remove_csv_files_for_jobs_older_than_seven_days( - notify_db, notify_db_session, mocker, sample_template + notify_db_session, mocker, sample_template ): """ Jobs older than seven days are deleted, but only two day's worth (two-day window) @@ -84,7 +84,7 @@ def test_will_remove_csv_files_for_jobs_older_than_seven_days( @freeze_time('2016-10-18T10:00:00') def test_will_remove_csv_files_for_jobs_older_than_retention_period( - notify_db, notify_db_session, mocker + notify_db_session, mocker ): """ Jobs older than retention period are deleted, but only two day's worth (two-day window) @@ -319,7 +319,7 @@ def test_get_letter_notifications_still_sending_when_they_shouldnt_finds_friday_ @freeze_time('2018-01-11T23:00:00') -def test_letter_raise_alert_if_no_ack_file_for_zip_does_not_raise_when_files_match_zip_list(mocker, notify_db): +def test_letter_raise_alert_if_no_ack_file_for_zip_does_not_raise_when_files_match_zip_list(mocker, notify_db_session): mock_file_list = mocker.patch("app.aws.s3.get_list_of_files_by_suffix", side_effect=mock_s3_get_list_match) letter_raise_alert_if_no_ack_file_for_zip() @@ -334,7 +334,7 @@ def test_letter_raise_alert_if_no_ack_file_for_zip_does_not_raise_when_files_mat @freeze_time('2018-01-11T23:00:00') -def test_letter_raise_alert_if_ack_files_not_match_zip_list(mocker, notify_db): +def test_letter_raise_alert_if_ack_files_not_match_zip_list(mocker, notify_db_session): mock_file_list = mocker.patch("app.aws.s3.get_list_of_files_by_suffix", side_effect=mock_s3_get_list_diff) mock_create_ticket = mocker.spy(NotifySupportTicket, '__init__') mock_send_ticket_to_zendesk = mocker.patch( @@ -360,7 +360,7 @@ def test_letter_raise_alert_if_ack_files_not_match_zip_list(mocker, notify_db): @freeze_time('2018-01-11T23:00:00') -def test_letter_not_raise_alert_if_no_files_do_not_cause_error(mocker, notify_db): +def test_letter_not_raise_alert_if_no_files_do_not_cause_error(mocker, notify_db_session): mock_file_list = mocker.patch("app.aws.s3.get_list_of_files_by_suffix", side_effect=None) letter_raise_alert_if_no_ack_file_for_zip() diff --git a/tests/app/celery/test_process_ses_receipts_tasks.py b/tests/app/celery/test_process_ses_receipts_tasks.py index 290b1029b..4cb614ae8 100644 --- a/tests/app/celery/test_process_ses_receipts_tasks.py +++ b/tests/app/celery/test_process_ses_receipts_tasks.py @@ -29,7 +29,7 @@ def test_process_ses_results(sample_email_template): assert process_ses_results(response=ses_notification_callback(reference='ref1')) -def test_process_ses_results_retry_called(sample_email_template, notify_db, mocker): +def test_process_ses_results_retry_called(sample_email_template, mocker): create_notification(sample_email_template, reference='ref1', sent_at=datetime.utcnow(), status='sending') mocker.patch("app.dao.notifications_dao.dao_update_notifications_by_reference", side_effect=Exception("EXPECTED")) @@ -102,7 +102,7 @@ def test_ses_callback_should_not_update_notification_status_if_already_delivered assert mock_upd.call_count == 0 -def test_ses_callback_should_retry_if_notification_is_new(client, notify_db, mocker): +def test_ses_callback_should_retry_if_notification_is_new(client, notify_db_session, mocker): mock_retry = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry') mock_logger = mocker.patch('app.celery.process_ses_receipts_tasks.current_app.logger.error') @@ -112,7 +112,7 @@ def test_ses_callback_should_retry_if_notification_is_new(client, notify_db, moc assert mock_retry.call_count == 1 -def test_ses_callback_should_log_if_notification_is_missing(client, notify_db, mocker): +def test_ses_callback_should_log_if_notification_is_missing(client, notify_db_session, mocker): mock_retry = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry') mock_logger = mocker.patch('app.celery.process_ses_receipts_tasks.current_app.logger.warning') @@ -122,7 +122,7 @@ def test_ses_callback_should_log_if_notification_is_missing(client, notify_db, m mock_logger.assert_called_once_with('notification not found for reference: ref (update to delivered)') -def test_ses_callback_should_not_retry_if_notification_is_old(client, notify_db, mocker): +def test_ses_callback_should_not_retry_if_notification_is_old(client, notify_db_session, mocker): mock_retry = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry') mock_logger = mocker.patch('app.celery.process_ses_receipts_tasks.current_app.logger.error') diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 08bba3c38..fed9a13a0 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -98,7 +98,7 @@ def test_should_have_decorated_tasks_functions(): @pytest.fixture -def email_job_with_placeholders(notify_db, notify_db_session, sample_email_template_with_placeholders): +def email_job_with_placeholders(notify_db_session, sample_email_template_with_placeholders): return create_job(template=sample_email_template_with_placeholders) @@ -481,7 +481,7 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi ) -def test_should_put_save_sms_task_in_research_mode_queue_if_research_mode_service(notify_db, notify_db_session, mocker): +def test_should_put_save_sms_task_in_research_mode_queue_if_research_mode_service(notify_db_session, mocker): service = create_service(research_mode=True, ) template = create_template(service=service) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 962923775..b7709a01c 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -526,7 +526,7 @@ def sample_email_notification(notify_db_session): @pytest.fixture(scope='function') -def sample_notification_history(notify_db, notify_db_session, sample_template): +def sample_notification_history(notify_db_session, sample_template): created_at = datetime.utcnow() sent_at = datetime.utcnow() notification_type = sample_template.template_type @@ -545,8 +545,8 @@ def sample_notification_history(notify_db, notify_db_session, sample_template): api_key_id=api_key and api_key.id, sent_at=sent_at ) - notify_db.session.add(notification_history) - notify_db.session.commit() + notify_db_session.add(notification_history) + notify_db_session.commit() return notification_history @@ -852,7 +852,7 @@ def letter_volumes_email_template(notify_service): @pytest.fixture -def notify_service(sample_user): +def notify_service(notify_db_session, sample_user): service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID']) if not service: service = Service( @@ -876,19 +876,19 @@ def notify_service(sample_user): } reply_to = ServiceEmailReplyTo(**data) - db.session.add(reply_to) - db.session.commit() + notify_db_session.add(reply_to) + notify_db_session.commit() return service @pytest.fixture(scope='function') -def sample_service_guest_list(notify_db, notify_db_session): +def sample_service_guest_list(notify_db_session): service = create_service(check_if_service_exists=True) guest_list_user = ServiceGuestList.from_string(service.id, EMAIL_TYPE, 'guest_list_user@digital.gov.uk') - notify_db.session.add(guest_list_user) - notify_db.session.commit() + notify_db_session.add(guest_list_user) + notify_db_session.commit() return guest_list_user @@ -933,7 +933,7 @@ def nhs_email_branding(notify_db_session): @pytest.fixture -def restore_provider_details(notify_db, notify_db_session): +def restore_provider_details(notify_db_session): """ We view ProviderDetails as a static in notify_db_session, since we don't modify it... except we do, we updated priority. This fixture is designed to be used in tests that will knowingly touch provider details, to restore them @@ -955,10 +955,10 @@ def restore_provider_details(notify_db, notify_db_session): # also delete these as they depend on provider_details ProviderDetails.query.delete() ProviderDetailsHistory.query.delete() - notify_db.session.commit() - notify_db.session.add_all(existing_provider_details) - notify_db.session.add_all(existing_provider_details_history) - notify_db.session.commit() + notify_db_session.commit() + notify_db_session.add_all(existing_provider_details) + notify_db_session.add_all(existing_provider_details_history) + notify_db_session.commit() @pytest.fixture diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 03b75e345..e31151527 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -401,7 +401,7 @@ def test_save_notification_and_increment_job(sample_template, sample_job, mmg_pr assert Notification.query.count() == 2 -def test_save_notification_and_increment_correct_job(notify_db, notify_db_session, sample_template, mmg_provider): +def test_save_notification_and_increment_correct_job(sample_template, mmg_provider): job_1 = create_job(sample_template) job_2 = create_job(sample_template) @@ -459,7 +459,7 @@ def test_get_notification_by_id_when_notification_exists(sample_notification): assert sample_notification == notification_from_db -def test_get_notification_by_id_when_notification_does_not_exist(notify_db, fake_uuid): +def test_get_notification_by_id_when_notification_does_not_exist(notify_db_session, fake_uuid): notification_from_db = get_notification_by_id(fake_uuid) assert notification_from_db is None @@ -738,7 +738,6 @@ def test_should_not_count_pages_when_given_a_flag(sample_user, sample_template): def test_get_notifications_created_by_api_or_csv_are_returned_correctly_excluding_test_key_notifications( - notify_db, notify_db_session, sample_service, sample_job, @@ -1529,7 +1528,7 @@ def test_dao_update_notifications_by_reference_updates_history_no_notifications_ assert updated_history_count == 2 -def test_dao_update_notifications_by_reference_returns_zero_when_no_notifications_to_update(notify_db): +def test_dao_update_notifications_by_reference_returns_zero_when_no_notifications_to_update(notify_db_session): updated_count, updated_history_count = dao_update_notifications_by_reference( references=['ref'], update_dict={ @@ -1574,14 +1573,14 @@ def test_dao_update_notifications_by_reference_updates_history_when_one_of_two_n assert NotificationHistory.query.get(notification1.id).status == 'returned-letter' -def test_dao_get_notification_by_reference_with_one_match_returns_notification(sample_letter_template, notify_db): +def test_dao_get_notification_by_reference_with_one_match_returns_notification(sample_letter_template): create_notification(template=sample_letter_template, reference='REF1') notification = dao_get_notification_by_reference('REF1') assert notification.reference == 'REF1' -def test_dao_get_notification_by_reference_with_multiple_matches_raises_error(sample_letter_template, notify_db): +def test_dao_get_notification_by_reference_with_multiple_matches_raises_error(sample_letter_template): create_notification(template=sample_letter_template, reference='REF1') create_notification(template=sample_letter_template, reference='REF1') @@ -1589,7 +1588,7 @@ def test_dao_get_notification_by_reference_with_multiple_matches_raises_error(sa dao_get_notification_by_reference('REF1') -def test_dao_get_notification_by_reference_with_no_matches_raises_error(notify_db): +def test_dao_get_notification_by_reference_with_no_matches_raises_error(notify_db_session): with pytest.raises(SQLAlchemyError): dao_get_notification_by_reference('REF1') @@ -1613,7 +1612,7 @@ def test_dao_get_notification_or_history_by_reference_with_multiple_matches_rais dao_get_notification_or_history_by_reference('REF1') -def test_dao_get_notification_or_history_by_reference_with_no_matches_raises_error(notify_db): +def test_dao_get_notification_or_history_by_reference_with_no_matches_raises_error(notify_db_session): with pytest.raises(SQLAlchemyError): dao_get_notification_or_history_by_reference('REF1') diff --git a/tests/app/dao/test_api_key_dao.py b/tests/app/dao/test_api_key_dao.py index 3254a1037..2caa1efac 100644 --- a/tests/app/dao/test_api_key_dao.py +++ b/tests/app/dao/test_api_key_dao.py @@ -56,7 +56,7 @@ def test_get_api_key_should_raise_exception_when_api_key_does_not_exist(sample_s get_model_api_keys(sample_service.id, id=fake_uuid) -def test_should_return_api_key_for_service(notify_api, notify_db, notify_db_session, sample_api_key): +def test_should_return_api_key_for_service(notify_api, notify_db_session, sample_api_key): api_key = get_model_api_keys(service_id=sample_api_key.service_id, id=sample_api_key.id) assert api_key == sample_api_key diff --git a/tests/app/dao/test_complaint_dao.py b/tests/app/dao/test_complaint_dao.py index b1e21fdb4..130bd183f 100644 --- a/tests/app/dao/test_complaint_dao.py +++ b/tests/app/dao/test_complaint_dao.py @@ -120,7 +120,7 @@ def test_fetch_count_of_complaints(sample_email_notification): assert count_of_complaints == 3 -def test_fetch_count_of_complaints_returns_zero(notify_db): +def test_fetch_count_of_complaints_returns_zero(notify_db_session): count_of_complaints = fetch_count_of_complaints(start_date=datetime(2018, 6, 7), end_date=datetime(2018, 6, 7)) assert count_of_complaints == 0 diff --git a/tests/app/dao/test_daily_sorted_letter_dao.py b/tests/app/dao/test_daily_sorted_letter_dao.py index 8c1183b58..a762fc8bc 100644 --- a/tests/app/dao/test_daily_sorted_letter_dao.py +++ b/tests/app/dao/test_daily_sorted_letter_dao.py @@ -8,7 +8,7 @@ from app.models import DailySortedLetter from tests.app.db import create_daily_sorted_letter -def test_dao_get_daily_sorted_letter_by_billing_day(notify_db, notify_db_session): +def test_dao_get_daily_sorted_letter_by_billing_day(notify_db_session): billing_day = date(2018, 2, 1) other_day = date(2017, 9, 8) @@ -18,7 +18,7 @@ def test_dao_get_daily_sorted_letter_by_billing_day(notify_db, notify_db_session assert not dao_get_daily_sorted_letter_by_billing_day(other_day) -def test_dao_create_or_update_daily_sorted_letter_creates_a_new_entry(notify_db, notify_db_session): +def test_dao_create_or_update_daily_sorted_letter_creates_a_new_entry(notify_db_session): billing_day = date(2018, 2, 1) dsl = DailySortedLetter(billing_day=billing_day, file_name="Notify-201802011234.rs.txt", @@ -35,7 +35,6 @@ def test_dao_create_or_update_daily_sorted_letter_creates_a_new_entry(notify_db, def test_dao_create_or_update_daily_sorted_letter_updates_an_existing_entry( - notify_db, notify_db_session ): create_daily_sorted_letter(billing_day=date(2018, 1, 18), diff --git a/tests/app/dao/test_email_branding_dao.py b/tests/app/dao/test_email_branding_dao.py index df9377cca..3fac87280 100644 --- a/tests/app/dao/test_email_branding_dao.py +++ b/tests/app/dao/test_email_branding_dao.py @@ -8,7 +8,7 @@ from app.models import EmailBranding from tests.app.db import create_email_branding -def test_get_email_branding_options_gets_all_email_branding(notify_db, notify_db_session): +def test_get_email_branding_options_gets_all_email_branding(notify_db_session): email_branding_1 = create_email_branding(name='test_email_branding_1') email_branding_2 = create_email_branding(name='test_email_branding_2') @@ -19,7 +19,7 @@ def test_get_email_branding_options_gets_all_email_branding(notify_db, notify_db assert email_branding_2 == email_branding[1] -def test_get_email_branding_by_id_gets_correct_email_branding(notify_db, notify_db_session): +def test_get_email_branding_by_id_gets_correct_email_branding(notify_db_session): email_branding = create_email_branding() email_branding_from_db = dao_get_email_branding_by_id(email_branding.id) @@ -27,7 +27,7 @@ def test_get_email_branding_by_id_gets_correct_email_branding(notify_db, notify_ assert email_branding_from_db == email_branding -def test_get_email_branding_by_name_gets_correct_email_branding(notify_db, notify_db_session): +def test_get_email_branding_by_name_gets_correct_email_branding(notify_db_session): email_branding = create_email_branding(name="Crystal Gems") email_branding_from_db = dao_get_email_branding_by_name("Crystal Gems") @@ -35,7 +35,7 @@ def test_get_email_branding_by_name_gets_correct_email_branding(notify_db, notif assert email_branding_from_db == email_branding -def test_update_email_branding(notify_db, notify_db_session): +def test_update_email_branding(notify_db_session): updated_name = 'new name' create_email_branding() @@ -52,7 +52,7 @@ def test_update_email_branding(notify_db, notify_db_session): assert email_branding[0].name == updated_name -def test_email_branding_has_no_domain(notify_db, notify_db_session): +def test_email_branding_has_no_domain(notify_db_session): create_email_branding() email_branding = EmailBranding.query.all() assert not hasattr(email_branding, 'domain') diff --git a/tests/app/dao/test_events_dao.py b/tests/app/dao/test_events_dao.py index c6b523150..f58235620 100644 --- a/tests/app/dao/test_events_dao.py +++ b/tests/app/dao/test_events_dao.py @@ -3,7 +3,7 @@ from app.dao.events_dao import dao_create_event from app.models import Event -def test_create_event(notify_db, notify_db_session): +def test_create_event(notify_db_session): assert Event.query.count() == 0 data = { 'event_type': 'sucessful_login', diff --git a/tests/app/dao/test_inbound_numbers_dao.py b/tests/app/dao/test_inbound_numbers_dao.py index d9f788f3f..53e0d4b12 100644 --- a/tests/app/dao/test_inbound_numbers_dao.py +++ b/tests/app/dao/test_inbound_numbers_dao.py @@ -13,14 +13,14 @@ from app.models import InboundNumber from tests.app.db import create_inbound_number, create_service -def test_get_inbound_numbers(notify_db, notify_db_session, sample_inbound_numbers): +def test_get_inbound_numbers(notify_db_session, sample_inbound_numbers): res = dao_get_inbound_numbers() assert len(res) == len(sample_inbound_numbers) assert res == sample_inbound_numbers -def test_get_available_inbound_numbers(notify_db, notify_db_session): +def test_get_available_inbound_numbers(notify_db_session): inbound_number = create_inbound_number(number='1') res = dao_get_available_inbound_numbers() @@ -29,7 +29,7 @@ def test_get_available_inbound_numbers(notify_db, notify_db_session): assert res[0] == inbound_number -def test_set_service_id_on_inbound_number(notify_db, notify_db_session, sample_inbound_numbers): +def test_set_service_id_on_inbound_number(notify_db_session, sample_inbound_numbers): service = create_service(service_name='test service') numbers = dao_get_available_inbound_numbers() @@ -42,7 +42,7 @@ def test_set_service_id_on_inbound_number(notify_db, notify_db_session, sample_i def test_after_setting_service_id_that_inbound_number_is_unavailable( - notify_db, notify_db_session, sample_inbound_numbers): + notify_db_session, sample_inbound_numbers): service = create_service(service_name='test service') numbers = dao_get_available_inbound_numbers() @@ -55,7 +55,7 @@ def test_after_setting_service_id_that_inbound_number_is_unavailable( assert len(res) == 0 -def test_setting_a_service_twice_will_raise_an_error(notify_db, notify_db_session): +def test_setting_a_service_twice_will_raise_an_error(notify_db_session): create_inbound_number(number='1') create_inbound_number(number='2') service = create_service(service_name='test service') @@ -70,7 +70,7 @@ def test_setting_a_service_twice_will_raise_an_error(notify_db, notify_db_sessio @pytest.mark.parametrize("active", [True, False]) -def test_set_inbound_number_active_flag(notify_db, notify_db_session, sample_service, active): +def test_set_inbound_number_active_flag(notify_db_session, sample_service, active): inbound_number = create_inbound_number(number='1') dao_set_inbound_number_to_service(sample_service.id, inbound_number) diff --git a/tests/app/dao/test_invited_user_dao.py b/tests/app/dao/test_invited_user_dao.py index a2b0d3b28..df5364d46 100644 --- a/tests/app/dao/test_invited_user_dao.py +++ b/tests/app/dao/test_invited_user_dao.py @@ -16,7 +16,7 @@ from app.models import InvitedUser from tests.app.db import create_invited_user -def test_create_invited_user(notify_db, notify_db_session, sample_service): +def test_create_invited_user(notify_db_session, sample_service): assert InvitedUser.query.count() == 0 email_address = 'invited_user@service.gov.uk' invite_from = sample_service.users[0] @@ -43,8 +43,6 @@ def test_create_invited_user(notify_db, notify_db_session, sample_service): def test_create_invited_user_sets_default_folder_permissions_of_empty_list( - notify_db, - notify_db_session, sample_service, ): assert InvitedUser.query.count() == 0 @@ -64,17 +62,17 @@ def test_create_invited_user_sets_default_folder_permissions_of_empty_list( assert invited_user.folder_permissions == [] -def test_get_invited_user_by_service_and_id(notify_db, notify_db_session, sample_invited_user): +def test_get_invited_user_by_service_and_id(notify_db_session, sample_invited_user): from_db = get_invited_user_by_service_and_id(sample_invited_user.service.id, sample_invited_user.id) assert from_db == sample_invited_user -def test_get_invited_user_by_id(notify_db, notify_db_session, sample_invited_user): +def test_get_invited_user_by_id(notify_db_session, sample_invited_user): from_db = get_invited_user_by_id(sample_invited_user.id) assert from_db == sample_invited_user -def test_get_unknown_invited_user_returns_none(notify_db, notify_db_session, sample_service): +def test_get_unknown_invited_user_returns_none(notify_db_session, sample_service): unknown_id = uuid.uuid4() with pytest.raises(NoResultFound) as e: @@ -82,7 +80,7 @@ def test_get_unknown_invited_user_returns_none(notify_db, notify_db_session, sam assert 'No row was found when one was required' in str(e.value) -def test_get_invited_users_for_service(notify_db, notify_db_session, sample_service): +def test_get_invited_users_for_service(notify_db_session, sample_service): invites = [] for i in range(0, 5): email = 'invited_user_{}@service.gov.uk'.format(i) @@ -96,12 +94,12 @@ def test_get_invited_users_for_service(notify_db, notify_db_session, sample_serv assert invite in all_from_db -def test_get_invited_users_for_service_that_has_no_invites(notify_db, notify_db_session, sample_service): +def test_get_invited_users_for_service_that_has_no_invites(notify_db_session, sample_service): invites = get_invited_users_for_service(sample_service.id) assert len(invites) == 0 -def test_save_invited_user_sets_status_to_cancelled(notify_db, notify_db_session, sample_invited_user): +def test_save_invited_user_sets_status_to_cancelled(notify_db_session, sample_invited_user): assert InvitedUser.query.count() == 1 saved = InvitedUser.query.get(sample_invited_user.id) assert saved.status == 'pending' diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index c632c496f..99815b66d 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -154,7 +154,7 @@ def test_get_jobs_for_service_with_limit_days_edge_case(sample_template): assert just_before_midnight_job not in jobs_limit_days -def test_get_jobs_for_service_in_processed_at_then_created_at_order(notify_db, notify_db_session, sample_template): +def test_get_jobs_for_service_in_processed_at_then_created_at_order(notify_db_session, sample_template): from_hour = partial(datetime, 2001, 1, 1) created_jobs = [ @@ -269,7 +269,7 @@ def test_should_get_jobs_seven_days_old(sample_template): assert jobs[0].id == job_to_delete.id -def test_get_jobs_for_service_is_paginated(notify_db, notify_db_session, sample_service, sample_template): +def test_get_jobs_for_service_is_paginated(notify_db_session, sample_service, sample_template): with freeze_time('2015-01-01T00:00:00') as the_time: for _ in range(10): the_time.tick(timedelta(hours=1)) diff --git a/tests/app/dao/test_letter_branding_dao.py b/tests/app/dao/test_letter_branding_dao.py index 1d1e35949..6122f2dcc 100644 --- a/tests/app/dao/test_letter_branding_dao.py +++ b/tests/app/dao/test_letter_branding_dao.py @@ -38,7 +38,7 @@ def test_dao_get_all_letter_branding(notify_db_session): assert len(results) == 2 -def test_dao_get_all_letter_branding_returns_empty_list_if_no_brands_exist(notify_db): +def test_dao_get_all_letter_branding_returns_empty_list_if_no_brands_exist(notify_db_session): assert dao_get_all_letter_branding() == [] diff --git a/tests/app/dao/test_service_callback_api_dao.py b/tests/app/dao/test_service_callback_api_dao.py index 8f5e1c59c..504e470d8 100644 --- a/tests/app/dao/test_service_callback_api_dao.py +++ b/tests/app/dao/test_service_callback_api_dao.py @@ -45,7 +45,7 @@ def test_save_service_callback_api(sample_service): assert versioned.version == 1 -def test_save_service_callback_api_fails_if_service_does_not_exist(notify_db, notify_db_session): +def test_save_service_callback_api_fails_if_service_does_not_exist(notify_db_session): service_callback_api = ServiceCallbackApi( service_id=uuid.uuid4(), url="https://some_service/callback_endpoint", diff --git a/tests/app/dao/test_service_guest_list_dao.py b/tests/app/dao/test_service_guest_list_dao.py index 100a8960f..b5a710e1c 100644 --- a/tests/app/dao/test_service_guest_list_dao.py +++ b/tests/app/dao/test_service_guest_list_dao.py @@ -29,7 +29,7 @@ def test_add_and_commit_guest_list_contacts_saves_data(sample_service): assert db_contents[0].id == guest_list.id -def test_remove_service_guest_list_only_removes_for_my_service(notify_db, notify_db_session): +def test_remove_service_guest_list_only_removes_for_my_service(notify_db_session): service_1 = create_service(service_name="service 1") service_2 = create_service(service_name="service 2") dao_add_and_commit_guest_list_contacts([ @@ -43,10 +43,10 @@ def test_remove_service_guest_list_only_removes_for_my_service(notify_db, notify assert len(service_2.guest_list) == 1 -def test_remove_service_guest_list_does_not_commit(notify_db, sample_service_guest_list): +def test_remove_service_guest_list_does_not_commit(notify_db_session, sample_service_guest_list): dao_remove_service_guest_list(sample_service_guest_list.service_id) # since dao_remove_service_guest_list doesn't commit, we can still rollback its changes - notify_db.session.rollback() + notify_db_session.rollback() assert ServiceGuestList.query.count() == 1 diff --git a/tests/app/dao/test_service_inbound_api_dao.py b/tests/app/dao/test_service_inbound_api_dao.py index ba22c3454..26a750533 100644 --- a/tests/app/dao/test_service_inbound_api_dao.py +++ b/tests/app/dao/test_service_inbound_api_dao.py @@ -45,7 +45,7 @@ def test_save_service_inbound_api(sample_service): assert versioned.version == 1 -def test_save_service_inbound_api_fails_if_service_does_not_exist(notify_db, notify_db_session): +def test_save_service_inbound_api_fails_if_service_does_not_exist(notify_db_session): service_inbound_api = ServiceInboundApi( service_id=uuid.uuid4(), url="https://some_service/inbound_messages", diff --git a/tests/app/dao/test_service_permissions_dao.py b/tests/app/dao/test_service_permissions_dao.py index 0378151ff..f1cc125a0 100644 --- a/tests/app/dao/test_service_permissions_dao.py +++ b/tests/app/dao/test_service_permissions_dao.py @@ -15,7 +15,7 @@ from tests.app.db import create_service, create_service_permission @pytest.fixture(scope='function') -def service_without_permissions(notify_db, notify_db_session): +def service_without_permissions(notify_db_session): return create_service(service_permissions=[]) diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index d34184305..f88853e75 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -505,7 +505,7 @@ def test_dao_fetch_live_services_data(sample_user): ] -def test_get_service_by_id_returns_none_if_no_service(notify_db): +def test_get_service_by_id_returns_none_if_no_service(notify_db_session): with pytest.raises(NoResultFound) as e: dao_fetch_service_by_id(str(uuid.uuid4())) assert 'No row was found when one was required' in str(e.value) @@ -1021,7 +1021,7 @@ def test_dao_fetch_todays_stats_for_all_services_only_includes_today(notify_db_s assert stats['failed'] == 1 -def test_dao_fetch_todays_stats_for_all_services_groups_correctly(notify_db, notify_db_session): +def test_dao_fetch_todays_stats_for_all_services_groups_correctly(notify_db_session): service1 = create_service(service_name='service 1', email_from='service.1') service2 = create_service(service_name='service 2', email_from='service.2') template_sms = create_template(service=service1) diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index e163d1e14..c128d594d 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -159,7 +159,7 @@ def test_update_user_attribute(client, sample_user, user_attribute, user_value): @freeze_time('2020-01-24T12:00:00') -def test_update_user_password(notify_api, notify_db, notify_db_session, sample_user): +def test_update_user_password(notify_api, notify_db_session, sample_user): sample_user.password_changed_at = datetime.utcnow() - timedelta(days=1) password = 'newpassword' assert not sample_user.check_password(password) diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 404d9c513..e47b0e173 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -258,15 +258,15 @@ def test_send_sms_should_use_template_version_from_notification_not_latest( (False, KEY_TYPE_TEST) ]) def test_should_call_send_sms_response_task_if_research_mode( - notify_db, sample_service, sample_notification, mocker, research_mode, key_type + notify_db_session, sample_service, sample_notification, mocker, research_mode, key_type ): mocker.patch('app.mmg_client.send_sms') mocker.patch('app.delivery.send_to_providers.send_sms_response') if research_mode: sample_service.research_mode = True - notify_db.session.add(sample_service) - notify_db.session.commit() + notify_db_session.add(sample_service) + notify_db_session.commit() sample_notification.key_type = key_type @@ -459,7 +459,7 @@ def test_get_html_email_renderer_should_return_for_normal_service(sample_service (BRANDING_BOTH, True), (BRANDING_ORG_BANNER, False) ]) -def test_get_html_email_renderer_with_branding_details(branding_type, govuk_banner, notify_db, sample_service): +def test_get_html_email_renderer_with_branding_details(branding_type, govuk_banner, notify_db_session, sample_service): email_branding = EmailBranding( brand_type=branding_type, @@ -469,8 +469,8 @@ def test_get_html_email_renderer_with_branding_details(branding_type, govuk_bann text='League of Justice', ) sample_service.email_branding = email_branding - notify_db.session.add_all([sample_service, email_branding]) - notify_db.session.commit() + notify_db_session.add_all([sample_service, email_branding]) + notify_db_session.commit() options = send_to_providers.get_html_email_options(sample_service) @@ -485,10 +485,10 @@ def test_get_html_email_renderer_with_branding_details(branding_type, govuk_bann assert options['brand_banner'] is False -def test_get_html_email_renderer_with_branding_details_and_render_govuk_banner_only(notify_db, sample_service): +def test_get_html_email_renderer_with_branding_details_and_render_govuk_banner_only(notify_db_session, sample_service): sample_service.email_branding = None - notify_db.session.add_all([sample_service]) - notify_db.session.commit() + notify_db_session.add_all([sample_service]) + notify_db_session.commit() options = send_to_providers.get_html_email_options(sample_service) diff --git a/tests/app/email_branding/test_rest.py b/tests/app/email_branding/test_rest.py index 80ed00e66..9cf08e66e 100644 --- a/tests/app/email_branding/test_rest.py +++ b/tests/app/email_branding/test_rest.py @@ -4,11 +4,11 @@ from app.models import BRANDING_ORG, EmailBranding from tests.app.db import create_email_branding -def test_get_email_branding_options(admin_request, notify_db, notify_db_session): +def test_get_email_branding_options(admin_request, notify_db_session): email_branding1 = EmailBranding(colour='#FFFFFF', logo='/path/image.png', name='Org1') email_branding2 = EmailBranding(colour='#000000', logo='/path/other.png', name='Org2') - notify_db.session.add_all([email_branding1, email_branding2]) - notify_db.session.commit() + notify_db_session.add_all([email_branding1, email_branding2]) + notify_db_session.commit() email_branding = admin_request.get( 'email_branding.get_email_branding_options' @@ -22,10 +22,10 @@ def test_get_email_branding_options(admin_request, notify_db, notify_db_session) } -def test_get_email_branding_by_id(admin_request, notify_db, notify_db_session): +def test_get_email_branding_by_id(admin_request, notify_db_session): email_branding = EmailBranding(colour='#FFFFFF', logo='/path/image.png', name='Some Org', text='My Org') - notify_db.session.add(email_branding) - notify_db.session.commit() + notify_db_session.add(email_branding) + notify_db_session.commit() response = admin_request.get( 'email_branding.get_email_branding_by_id', diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index 266bfa9c1..df7e9a5e1 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -263,7 +263,6 @@ def test_persist_notification_sets_daily_limit_cache_if_one_does_not_exists( (False, 'notify-internal-tasks', 'sms', 'test', 'research-mode-tasks', 'provider_tasks.deliver_sms'), ]) def test_send_notification_to_queue( - notify_db, notify_db_session, research_mode, requested_queue, diff --git a/tests/app/notifications/test_receive_notification.py b/tests/app/notifications/test_receive_notification.py index a08caf3b1..56efffdfd 100644 --- a/tests/app/notifications/test_receive_notification.py +++ b/tests/app/notifications/test_receive_notification.py @@ -169,7 +169,7 @@ def test_receive_notification_without_permissions_does_not_create_inbound_even_w ([INBOUND_SMS_TYPE], False), ([SMS_TYPE], False), ]) -def test_check_permissions_for_inbound_sms(notify_db, notify_db_session, permissions, expected_response): +def test_check_permissions_for_inbound_sms(notify_db_session, permissions, expected_response): service = create_service(service_permissions=permissions) assert has_inbound_sms_permissions(service.permissions) is expected_response diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index ef76607d0..44b893363 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -544,13 +544,13 @@ def test_get_notifications_for_service_returns_merged_template_content(client, s def test_get_notification_selects_correct_template_for_personalisation(client, - notify_db, + notify_db_session, sample_template): create_notification(sample_template) original_content = sample_template.content sample_template.content = '((name))' dao_update_template(sample_template) - notify_db.session.commit() + notify_db_session.commit() create_notification(sample_template, personalisation={"name": "foo"}) diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index d09f90091..1ac710548 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -217,7 +217,6 @@ def test_service_can_send_to_recipient_passes_for_guest_list_recipient_passes(sa {"mobile_number": "07513332413"}, ]) def test_service_can_send_to_recipient_fails_when_ignoring_guest_list( - notify_db, notify_db_session, sample_service, recipient, diff --git a/tests/app/provider_details/test_rest.py b/tests/app/provider_details/test_rest.py index 6d1e28ffc..2b347fbba 100644 --- a/tests/app/provider_details/test_rest.py +++ b/tests/app/provider_details/test_rest.py @@ -14,7 +14,7 @@ def test_get_provider_details_returns_all_providers(admin_request, notify_db_ses assert {'ses', 'firetext', 'mmg', 'dvla'} <= {x['identifier'] for x in json_resp} -def test_get_provider_details_by_id(client, notify_db): +def test_get_provider_details_by_id(client, notify_db_session): response = client.get( '/provider-details', headers=[create_admin_authorization_header()] @@ -103,7 +103,7 @@ def test_should_not_be_able_to_update_disallowed_fields(client, restore_provider assert resp.status_code == 400 -def test_get_provider_versions_contains_correct_fields(client, notify_db): +def test_get_provider_versions_contains_correct_fields(client, notify_db_session): provider = ProviderDetailsHistory.query.first() response = client.get( '/provider-details/{}/versions'.format(provider.id), diff --git a/tests/app/service/test_archived_service.py b/tests/app/service/test_archived_service.py index b6d18dce6..b7b55b72e 100644 --- a/tests/app/service/test_archived_service.py +++ b/tests/app/service/test_archived_service.py @@ -34,13 +34,13 @@ def test_deactivating_inactive_service_does_nothing(client, sample_service): @pytest.fixture -def archived_service(client, notify_db, sample_service): +def archived_service(client, notify_db_session, sample_service): create_template(sample_service, template_name='a') create_template(sample_service, template_name='b') create_api_key(sample_service) create_api_key(sample_service) - notify_db.session.commit() + notify_db_session.commit() auth_header = create_admin_authorization_header() response = client.post('/service/{}/archive'.format(sample_service.id), headers=[auth_header]) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index a80bef45b..ba61fea32 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -154,7 +154,7 @@ def test_get_service_list_should_return_empty_list_if_no_services(admin_request) assert len(json_resp['data']) == 0 -def test_find_services_by_name_finds_services(notify_db, admin_request, mocker): +def test_find_services_by_name_finds_services(notify_db_session, admin_request, mocker): service_1 = create_service(service_name="ABCDEF") service_2 = create_service(service_name="ABCGHT") mock_get_services_by_partial_name = mocker.patch( @@ -166,7 +166,7 @@ def test_find_services_by_name_finds_services(notify_db, admin_request, mocker): assert len(response) == 2 -def test_find_services_by_name_handles_no_results(notify_db, admin_request, mocker): +def test_find_services_by_name_handles_no_results(notify_db_session, admin_request, mocker): mock_get_services_by_partial_name = mocker.patch( 'app.service.rest.get_services_by_partial_name', return_value=[] @@ -176,7 +176,7 @@ def test_find_services_by_name_handles_no_results(notify_db, admin_request, mock assert len(response) == 0 -def test_find_services_by_name_handles_no_service_name(notify_db, admin_request, mocker): +def test_find_services_by_name_handles_no_service_name(notify_db_session, admin_request, mocker): mock_get_services_by_partial_name = mocker.patch( 'app.service.rest.get_services_by_partial_name' ) @@ -300,7 +300,7 @@ def test_get_service_by_id(admin_request, sample_service): ('government', 'vodafone'), )) def test_get_service_by_id_for_broadcast_service_returns_broadcast_keys( - notify_db, admin_request, sample_broadcast_service, broadcast_channel, allowed_broadcast_provider + notify_db_session, admin_request, sample_broadcast_service, broadcast_channel, allowed_broadcast_provider ): sample_broadcast_service.broadcast_channel = broadcast_channel sample_broadcast_service.allowed_broadcast_provider = allowed_broadcast_provider @@ -606,7 +606,6 @@ def test_should_error_if_created_by_missing(notify_api, sample_user): def test_should_not_create_service_with_missing_if_user_id_is_not_in_database(notify_api, - notify_db, notify_db_session, fake_uuid): with notify_api.test_request_context(): @@ -704,10 +703,10 @@ def test_create_service_should_throw_duplicate_key_constraint_for_existing_email assert "Duplicate service name '{}'".format(service_name) in json_resp['message']['name'] -def test_update_service(client, notify_db, sample_service): +def test_update_service(client, notify_db_session, sample_service): brand = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League') - notify_db.session.add(brand) - notify_db.session.commit() + notify_db_session.add(brand) + notify_db_session.commit() assert sample_service.email_branding is None @@ -752,7 +751,7 @@ def test_cant_update_service_org_type_to_random_value(client, sample_service): assert resp.status_code == 500 -def test_update_service_letter_branding(client, notify_db, sample_service): +def test_update_service_letter_branding(client, notify_db_session, sample_service): letter_branding = create_letter_branding(name='test brand', filename='test-brand') data = { 'letter_branding': str(letter_branding.id) @@ -770,7 +769,7 @@ def test_update_service_letter_branding(client, notify_db, sample_service): assert result['data']['letter_branding'] == str(letter_branding.id) -def test_update_service_remove_letter_branding(client, notify_db, sample_service): +def test_update_service_remove_letter_branding(client, notify_db_session, sample_service): letter_branding = create_letter_branding(name='test brand', filename='test-brand') sample_service data = { @@ -799,10 +798,10 @@ def test_update_service_remove_letter_branding(client, notify_db, sample_service assert result['data']['letter_branding'] is None -def test_update_service_remove_email_branding(admin_request, notify_db, sample_service): +def test_update_service_remove_email_branding(admin_request, notify_db_session, sample_service): brand = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League') sample_service.email_branding = brand - notify_db.session.commit() + notify_db_session.commit() resp = admin_request.post( 'service.update_service', @@ -812,12 +811,12 @@ def test_update_service_remove_email_branding(admin_request, notify_db, sample_s assert resp['data']['email_branding'] is None -def test_update_service_change_email_branding(admin_request, notify_db, sample_service): +def test_update_service_change_email_branding(admin_request, notify_db_session, sample_service): brand1 = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League') brand2 = EmailBranding(colour='#111111', logo='avengers.png', name='Avengers') - notify_db.session.add_all([brand1, brand2]) + notify_db_session.add_all([brand1, brand2]) sample_service.email_branding = brand1 - notify_db.session.commit() + notify_db_session.commit() resp = admin_request.post( 'service.update_service', @@ -910,7 +909,7 @@ def test_update_service_sets_research_consent( @pytest.fixture(scope='function') -def service_with_no_permissions(notify_db, notify_db_session): +def service_with_no_permissions(notify_db_session): return create_service(service_permissions=[]) @@ -931,7 +930,7 @@ def test_update_service_flags_with_service_without_default_service_permissions(c assert set(result['data']['permissions']) == set([LETTER_TYPE, INTERNATIONAL_SMS_TYPE]) -def test_update_service_flags_will_remove_service_permissions(client, notify_db, notify_db_session): +def test_update_service_flags_will_remove_service_permissions(client, notify_db_session): auth_header = create_admin_authorization_header() service = create_service(service_permissions=[SMS_TYPE, EMAIL_TYPE, INTERNATIONAL_SMS_TYPE]) @@ -1092,7 +1091,6 @@ def test_update_service_research_mode_throws_validation_error(notify_api, sample def test_should_not_update_service_with_duplicate_name(notify_api, - notify_db, notify_db_session, sample_user, sample_service): @@ -1122,7 +1120,6 @@ def test_should_not_update_service_with_duplicate_name(notify_api, def test_should_not_update_service_with_duplicate_email_from(notify_api, - notify_db, notify_db_session, sample_user, sample_service): @@ -1210,7 +1207,7 @@ def test_get_users_for_service_returns_empty_list_if_no_users_associated_with_se assert result['data'] == [] -def test_get_users_for_service_returns_404_when_service_does_not_exist(notify_api, notify_db, notify_db_session): +def test_get_users_for_service_returns_404_when_service_does_not_exist(notify_api, notify_db_session): with notify_api.test_request_context(): with notify_api.test_client() as client: service_id = uuid.uuid4() @@ -1227,7 +1224,6 @@ def test_get_users_for_service_returns_404_when_service_does_not_exist(notify_ap def test_default_permissions_are_added_for_user_service(notify_api, - notify_db, notify_db_session, sample_service, sample_user): @@ -1274,7 +1270,6 @@ def test_default_permissions_are_added_for_user_service(notify_api, def test_add_existing_user_to_another_service_with_all_permissions( notify_api, - notify_db, notify_db_session, sample_service, sample_user @@ -1353,7 +1348,6 @@ def test_add_existing_user_to_another_service_with_all_permissions( def test_add_existing_user_to_another_service_with_send_permissions(notify_api, - notify_db, notify_db_session, sample_service, sample_user): @@ -1401,7 +1395,6 @@ def test_add_existing_user_to_another_service_with_send_permissions(notify_api, def test_add_existing_user_to_another_service_with_manage_permissions(notify_api, - notify_db, notify_db_session, sample_service, sample_user): @@ -1448,7 +1441,6 @@ def test_add_existing_user_to_another_service_with_manage_permissions(notify_api def test_add_existing_user_to_another_service_with_folder_permissions(notify_api, - notify_db, notify_db_session, sample_service, sample_user): @@ -1489,7 +1481,6 @@ def test_add_existing_user_to_another_service_with_folder_permissions(notify_api def test_add_existing_user_to_another_service_with_manage_api_keys(notify_api, - notify_db, notify_db_session, sample_service, sample_user): @@ -1530,7 +1521,6 @@ def test_add_existing_user_to_another_service_with_manage_api_keys(notify_api, def test_add_existing_user_to_non_existing_service_returns404(notify_api, - notify_db, notify_db_session, sample_user): with notify_api.test_request_context(): @@ -1562,7 +1552,7 @@ def test_add_existing_user_to_non_existing_service_returns404(notify_api, assert result['message'] == expected_message -def test_add_existing_user_of_service_to_service_returns400(notify_api, notify_db, notify_db_session, sample_service): +def test_add_existing_user_of_service_to_service_returns400(notify_api, notify_db_session, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: existing_user_id = sample_service.users[0].id @@ -1584,7 +1574,7 @@ def test_add_existing_user_of_service_to_service_returns400(notify_api, notify_d assert result['message'] == expected_message -def test_add_unknown_user_to_service_returns404(notify_api, notify_db, notify_db_session, sample_service): +def test_add_unknown_user_to_service_returns404(notify_api, notify_db_session, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: incorrect_id = 9876 @@ -1646,7 +1636,6 @@ def test_remove_non_existant_user_from_service( def test_cannot_remove_only_user_from_service(notify_api, - notify_db, notify_db_session, sample_user_service_permission): with notify_api.test_request_context(): @@ -1785,7 +1774,7 @@ def test_get_all_notifications_for_service_formatted_for_csv(client, sample_temp assert resp['notifications'][0]['status'] == 'Sending' -def test_get_notification_for_service_without_uuid(client, notify_db, notify_db_session): +def test_get_notification_for_service_without_uuid(client, notify_db_session): service_1 = create_service(service_name="1", email_from='1') response = client.get( path='/service/{}/notifications/{}'.format(service_1.id, 'foo'), @@ -2342,7 +2331,6 @@ def test_search_for_notification_by_to_field_returns_no_next_link_if_50_or_less( def test_search_for_notification_by_to_field_for_letter( client, - notify_db, notify_db_session, sample_letter_template, sample_email_template, @@ -2364,7 +2352,7 @@ def test_search_for_notification_by_to_field_for_letter( assert notifications[0]['id'] == str(letter_notification.id) -def test_update_service_calls_send_notification_as_service_becomes_live(notify_db, notify_db_session, client, mocker): +def test_update_service_calls_send_notification_as_service_becomes_live(notify_db_session, client, mocker): send_notification_mock = mocker.patch('app.service.rest.send_notification_to_service_users') restricted_service = create_service(restricted=True) @@ -2708,7 +2696,7 @@ def test_get_email_reply_to_addresses_when_there_are_no_reply_to_email_addresses assert response.status_code == 200 -def test_get_email_reply_to_addresses_with_one_email_address(client, notify_db, notify_db_session): +def test_get_email_reply_to_addresses_with_one_email_address(client, notify_db_session): service = create_service() create_reply_to_email(service, 'test@mail.com') @@ -2724,7 +2712,7 @@ def test_get_email_reply_to_addresses_with_one_email_address(client, notify_db, assert response.status_code == 200 -def test_get_email_reply_to_addresses_with_multiple_email_addresses(client, notify_db, notify_db_session): +def test_get_email_reply_to_addresses_with_multiple_email_addresses(client, notify_db_session): service = create_service() reply_to_a = create_reply_to_email(service, 'test_a@mail.com') reply_to_b = create_reply_to_email(service, 'test_b@mail.com', False) @@ -2752,7 +2740,7 @@ def test_get_email_reply_to_addresses_with_multiple_email_addresses(client, noti def test_verify_reply_to_email_address_should_send_verification_email( - admin_request, notify_db, notify_db_session, mocker, verify_reply_to_address_email_template + admin_request, notify_db_session, mocker, verify_reply_to_address_email_template ): service = create_service() mocked = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') @@ -2772,7 +2760,7 @@ def test_verify_reply_to_email_address_should_send_verification_email( assert notification.reply_to_text == notify_service.get_default_reply_to_email_address() -def test_verify_reply_to_email_address_doesnt_allow_duplicates(admin_request, notify_db, notify_db_session, mocker): +def test_verify_reply_to_email_address_doesnt_allow_duplicates(admin_request, notify_db_session, mocker): data = {'email': 'reply-here@example.gov.uk'} service = create_service() create_reply_to_email(service, 'reply-here@example.gov.uk') @@ -2800,7 +2788,7 @@ def test_add_service_reply_to_email_address(admin_request, sample_service): def test_add_service_reply_to_email_address_doesnt_allow_duplicates( - admin_request, notify_db, notify_db_session, mocker + admin_request, notify_db_session, mocker ): data = {"email_address": "reply-here@example.gov.uk", "is_default": True} service = create_service() @@ -2848,7 +2836,7 @@ def test_add_service_reply_to_email_address_raise_exception_if_no_default(admin_ assert response['message'] == 'You must have at least one reply to email address as the default.' -def test_add_service_reply_to_email_address_404s_when_invalid_service_id(admin_request, notify_db, notify_db_session): +def test_add_service_reply_to_email_address_404s_when_invalid_service_id(admin_request, notify_db_session): response = admin_request.post( 'service.add_service_reply_to_email_address', service_id=uuid.uuid4(), @@ -2891,7 +2879,7 @@ def test_update_service_reply_to_email_address_returns_400_when_no_default(admin def test_update_service_reply_to_email_address_404s_when_invalid_service_id( - admin_request, notify_db, notify_db_session + admin_request, notify_db_session ): response = admin_request.post( 'service.update_service_reply_to_email_address', @@ -2939,7 +2927,7 @@ def test_delete_service_reply_to_email_address_returns_400_if_archiving_default_ assert reply_to.archived is False -def test_get_email_reply_to_address(client, notify_db, notify_db_session): +def test_get_email_reply_to_address(client, notify_db_session): service = create_service() reply_to = create_reply_to_email(service, 'test_a@mail.com') @@ -2958,7 +2946,7 @@ def test_get_letter_contacts_when_there_are_no_letter_contacts(client, sample_se assert response.status_code == 200 -def test_get_letter_contacts_with_one_letter_contact(client, notify_db, notify_db_session): +def test_get_letter_contacts_with_one_letter_contact(client, notify_db_session): service = create_service() create_letter_contact(service, 'Aberdeen, AB23 1XH') @@ -2974,7 +2962,7 @@ def test_get_letter_contacts_with_one_letter_contact(client, notify_db, notify_d assert response.status_code == 200 -def test_get_letter_contacts_with_multiple_letter_contacts(client, notify_db, notify_db_session): +def test_get_letter_contacts_with_multiple_letter_contacts(client, notify_db_session): service = create_service() letter_contact_a = create_letter_contact(service, 'Aberdeen, AB23 1XH') letter_contact_b = create_letter_contact(service, 'London, E1 8QS', False) @@ -3001,7 +2989,7 @@ def test_get_letter_contacts_with_multiple_letter_contacts(client, notify_db, no assert not json_response[1]['updated_at'] -def test_get_letter_contact_by_id(client, notify_db, notify_db_session): +def test_get_letter_contact_by_id(client, notify_db_session): service = create_service() letter_contact = create_letter_contact(service, 'London, E1 8QS') @@ -3012,7 +3000,7 @@ def test_get_letter_contact_by_id(client, notify_db, notify_db_session): assert json.loads(response.get_data(as_text=True)) == letter_contact.serialize() -def test_get_letter_contact_return_404_when_invalid_contact_id(client, notify_db, notify_db_session): +def test_get_letter_contact_return_404_when_invalid_contact_id(client, notify_db_session): service = create_service() response = client.get('/service/{}/letter-contact/{}'.format(service.id, '93d59f88-4aa1-453c-9900-f61e2fc8a2de'), @@ -3062,7 +3050,7 @@ def test_add_service_letter_contact_block_fine_if_no_default(client, sample_serv assert response.status_code == 201 -def test_add_service_letter_contact_block_404s_when_invalid_service_id(client, notify_db, notify_db_session): +def test_add_service_letter_contact_block_404s_when_invalid_service_id(client, notify_db_session): response = client.post('/service/{}/letter-contact'.format(uuid.uuid4()), data={}, headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) @@ -3096,7 +3084,7 @@ def test_update_service_letter_contact_returns_200_when_no_default(client, sampl assert response.status_code == 200 -def test_update_service_letter_contact_returns_404_when_invalid_service_id(client, notify_db, notify_db_session): +def test_update_service_letter_contact_returns_404_when_invalid_service_id(client, notify_db_session): response = client.post('/service/{}/letter-contact/{}'.format(uuid.uuid4(), uuid.uuid4()), data={}, headers=[('Content-Type', 'application/json'), create_admin_authorization_header()]) @@ -3804,7 +3792,7 @@ def test_set_as_broadcast_service_rejects_unknown_channels( def test_set_as_broadcast_service_rejects_if_no_channel( - admin_request, notify_db, sample_service, broadcast_organisation + admin_request, notify_db_session, sample_service, broadcast_organisation ): data = { 'service_mode': 'training', @@ -3936,11 +3924,11 @@ def test_set_as_broadcast_service_does_not_error_if_run_on_a_service_that_is_alr @freeze_time('2021-02-02') def test_set_as_broadcast_service_sets_service_to_live_mode( - admin_request, notify_db, sample_service, broadcast_organisation + admin_request, notify_db_session, sample_service, broadcast_organisation ): sample_service.restricted = True - notify_db.session.add(sample_service) - notify_db.session.commit() + notify_db_session.add(sample_service) + notify_db_session.commit() assert sample_service.restricted is True assert sample_service.go_live_at is None data = { @@ -3960,12 +3948,12 @@ def test_set_as_broadcast_service_sets_service_to_live_mode( def test_set_as_broadcast_service_doesnt_override_existing_go_live_at( - admin_request, notify_db, sample_broadcast_service + admin_request, notify_db_session, sample_broadcast_service ): sample_broadcast_service.restricted = False sample_broadcast_service.go_live_at = datetime(2021, 1, 1) - notify_db.session.add(sample_broadcast_service) - notify_db.session.commit() + notify_db_session.add(sample_broadcast_service) + notify_db_session.commit() assert sample_broadcast_service.restricted is False assert sample_broadcast_service.go_live_at is not None data = { @@ -3985,12 +3973,12 @@ def test_set_as_broadcast_service_doesnt_override_existing_go_live_at( def test_set_as_broadcast_service_sets_service_to_training_mode( - admin_request, notify_db, sample_broadcast_service + admin_request, notify_db_session, sample_broadcast_service ): sample_broadcast_service.restricted = False sample_broadcast_service.go_live_at = datetime(2021, 1, 1) - notify_db.session.add(sample_broadcast_service) - notify_db.session.commit() + notify_db_session.add(sample_broadcast_service) + notify_db_session.commit() assert sample_broadcast_service.restricted is False assert sample_broadcast_service.go_live_at is not None @@ -4071,11 +4059,11 @@ def test_set_as_broadcast_service_sets_mobile_provider_restriction( @pytest.mark.parametrize('provider', ["all", "vodafone"]) def test_set_as_broadcast_service_updates_mobile_provider_restriction( - admin_request, notify_db, sample_broadcast_service, provider + admin_request, notify_db_session, sample_broadcast_service, provider ): sample_broadcast_service.service_broadcast_settings.provider = "o2" - notify_db.session.add(sample_broadcast_service) - notify_db.session.commit() + notify_db_session.add(sample_broadcast_service) + notify_db_session.commit() assert sample_broadcast_service.service_broadcast_settings.provider == "o2" data = { diff --git a/tests/app/service_invite/test_service_invite_rest.py b/tests/app/service_invite/test_service_invite_rest.py index bcb268d5e..3d2f71c27 100644 --- a/tests/app/service_invite/test_service_invite_rest.py +++ b/tests/app/service_invite/test_service_invite_rest.py @@ -177,7 +177,7 @@ def test_create_invited_user_invalid_email(client, sample_service, mocker, fake_ assert mocked.call_count == 0 -def test_get_all_invited_users_by_service(client, notify_db, notify_db_session, sample_service): +def test_get_all_invited_users_by_service(client, notify_db_session, sample_service): invites = [] for i in range(0, 5): email = 'invited_user_{}@service.gov.uk'.format(i) @@ -205,7 +205,7 @@ def test_get_all_invited_users_by_service(client, notify_db, notify_db_session, assert invite['id'] -def test_get_invited_users_by_service_with_no_invites(client, notify_db, notify_db_session, sample_service): +def test_get_invited_users_by_service_with_no_invites(client, notify_db_session, sample_service): url = '/service/{}/invite'.format(sample_service.id) auth_header = create_admin_authorization_header() diff --git a/tests/conftest.py b/tests/conftest.py index 085825367..1593d2cab 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -69,6 +69,10 @@ def create_test_db(database_uri): @pytest.fixture(scope='session') def notify_db(notify_api, worker_id): + """ + Manages the connection to the database. Generally this shouldn't be used, instead you should use the + `notify_db_session` fixture which also cleans up any data you've got left over after your test run. + """ assert 'test_notification_api' in db.engine.url.database, 'dont run tests against main db' # create a database for this worker thread - @@ -103,7 +107,13 @@ def sms_providers(notify_db): @pytest.fixture(scope='function') def notify_db_session(notify_db, sms_providers): - yield notify_db + """ + This fixture clears down all non static data after your test run. It yields the sqlalchemy session variable + so you can manually add, commit, etc if needed. + + `notify_db_session.commit()` + """ + yield notify_db.session notify_db.session.remove() for tbl in reversed(notify_db.metadata.sorted_tables):