From b487a915f6772b4bc6709af84f1497da557df679 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Mon, 30 Oct 2023 08:42:11 -0700 Subject: [PATCH] fix remaining tests --- .../test_service_setting_permissions.py | 10 ++-- .../service_settings/test_service_settings.py | 55 +++++++++---------- tests/app/main/views/test_platform_admin.py | 6 +- tests/app/main/views/test_send.py | 36 +++++++----- tests/app/main/views/test_template_folders.py | 36 +++++++----- tests/conftest.py | 2 +- 6 files changed, 77 insertions(+), 68 deletions(-) diff --git a/tests/app/main/views/service_settings/test_service_setting_permissions.py b/tests/app/main/views/service_settings/test_service_setting_permissions.py index e64c6c829..16240d716 100644 --- a/tests/app/main/views/service_settings/test_service_setting_permissions.py +++ b/tests/app/main/views/service_settings/test_service_setting_permissions.py @@ -190,12 +190,10 @@ def test_service_setting_link_toggles_index_error( index, text, ): - with pytest.raises( # noqa: PT012 # Requires more research to refactor. - expected_exception=IndexError - ): - url_for(endpoint, service_id=service_one["id"]) - service_one.update(service_fields) - page = get_service_settings_page() + url_for(endpoint, service_id=service_one["id"]) + service_one.update(service_fields) + page = get_service_settings_page() + with pytest.raises(expected_exception=IndexError): page.select(".page-footer-link a")[index] diff --git a/tests/app/main/views/service_settings/test_service_settings.py b/tests/app/main/views/service_settings/test_service_settings.py index ea2818473..3ee19fad2 100644 --- a/tests/app/main/views/service_settings/test_service_settings.py +++ b/tests/app/main/views/service_settings/test_service_settings.py @@ -744,13 +744,11 @@ def test_should_check_for_reply_to_on_go_live_index_error( return_value=volume, ) - with pytest.raises( # noqa: PT012 # This will require more research for refactoring. - expected_exception=IndexError - ): - page = client_request.get("main.request_to_go_live", service_id=SERVICE_ONE_ID) - assert page.h1.text == "Before you request to go live" + page = client_request.get("main.request_to_go_live", service_id=SERVICE_ONE_ID) + assert page.h1.text == "Before you request to go live" + checklist_items = page.select(".task-list .task-list-item") - checklist_items = page.select(".task-list .task-list-item") + with pytest.raises(expected_exception=IndexError): assert ( normalize_spaces(checklist_items[3].text) == expected_reply_to_checklist_item @@ -1030,19 +1028,24 @@ def test_should_check_for_sms_sender_on_go_live( return_value=volume, ) - with pytest.raises( # noqa: PT012 # Requires more research for how to refactor. - expected_exception=IndexError - ): - page = client_request.get("main.request_to_go_live", service_id=SERVICE_ONE_ID) - assert page.h1.text == "Before you request to go live" - - checklist_items = page.select(".task-list .task-list-item") - assert ( - normalize_spaces(checklist_items[3].text) - == expected_sms_sender_checklist_item + with pytest.raises(expected_exception=IndexError): + simple_statement_for_test_should_check_for_sms_sender_on_go_live( + client_request, expected_sms_sender_checklist_item, mock_get_sms_senders ) - mock_get_sms_senders.assert_called_once_with(SERVICE_ONE_ID) + +def simple_statement_for_test_should_check_for_sms_sender_on_go_live( + client_request, expected_sms_sender_checklist_item, mock_get_sms_senders +): + page = client_request.get("main.request_to_go_live", service_id=SERVICE_ONE_ID) + assert page.h1.text == "Before you request to go live" + checklist_items = page.select(".task-list .task-list-item") + + assert ( + normalize_spaces(checklist_items[3].text) == expected_sms_sender_checklist_item + ) + + mock_get_sms_senders.assert_called_once_with(SERVICE_ONE_ID) def test_non_gov_user_is_told_they_cant_go_live( @@ -3145,10 +3148,8 @@ def test_should_set_sms_allowance_fails( mock_get_free_sms_fragment_limit, mock_create_or_update_free_sms_fragment_limit, ): - with pytest.raises( # noqa: PT012 # Needs more research for refactoring. - expected_exception=AssertionError - ): - client_request.login(platform_admin_user) + client_request.login(platform_admin_user) + with pytest.raises(expected_exception=AssertionError): client_request.post( "main.set_free_sms_allowance", service_id=SERVICE_ONE_ID, @@ -3460,10 +3461,8 @@ def test_archive_service_after_confirm_error( mocker.patch("app.notify_client.service_api_client.redis_client.delete") mocker.patch("app.notify_client.service_api_client.redis_client.delete_by_pattern") - with pytest.raises( # noqa: PT012 # Needs more research for refactoring. - expected_exception=AssertionError - ): - client_request.login(user) + client_request.login(user) + with pytest.raises(expected_exception=AssertionError): client_request.post( "main.archive_service", service_id=SERVICE_ONE_ID, @@ -3596,10 +3595,8 @@ def test_suspend_service_after_confirm_error( ): mocker.patch("app.service_api_client.post") mocker.patch("app.main.views.service_settings.create_suspend_service_event") - with pytest.raises( # noqa: PT012 # Needs more research for refactoring. - expected_exception=AssertionError - ): - client_request.login(user) + client_request.login(user) + with pytest.raises(expected_exception=AssertionError): client_request.post( "main.suspend_service", service_id=SERVICE_ONE_ID, diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 568ec2c74..49206d873 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -770,10 +770,12 @@ def test_clear_cache_shows_form( [ call("service-????????-????-????-????-????????????-templates"), call( - "service-????????-????-????-????-????????????-template-????????-????-????-????-????????????-version-*" # noqa too long + "service-????????-????-????-????-????????????-template" + "-????????-????-????-????-????????????-version-*" ), call( - "service-????????-????-????-????-????????????-template-????????-????-????-????-????????????-versions" # noqa too long + "service-????????-????-????-????-????????????-template" + "-????????-????-????-????-????????????-versions" ), ], "Removed 6 objects across 3 key formats for template", diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 674f2dbd6..65dc7f1b1 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -2319,25 +2319,31 @@ def test_warns_if_file_sent_already_errors( "app.main.views.send.get_csv_metadata", return_value={"original_file_name": uploaded_file_name}, ) - # Should be botocore.errorfactory.NoSuchKey but for some reason can't use that - with pytest.raises( # noqa: PT011,PT012 # Requires more research on how to refactor. - expected_exception=Exception + + with pytest.raises( + expected_exception=Exception, match="Unable to locate credentials" ): - page = client_request.get( - "main.check_messages", - service_id=SERVICE_ONE_ID, - template_id="5d729fbd-239c-44ab-b498-75a985f3198f", - upload_id=fake_uuid, - original_file_name=uploaded_file_name, - _test_page_title=False, + stmt_for_test_warns_if_file_sent_already_errors( + client_request, uploaded_file_name, fake_uuid, mock_get_jobs ) - assert normalize_spaces(page.select_one(".banner-dangerous").text) == ( - "These messages have already been sent today " - "If you need to resend them, rename the file and upload it again." - ) - mock_get_jobs.assert_called_once_with(SERVICE_ONE_ID, limit_days=0) +def stmt_for_test_warns_if_file_sent_already_errors( + client_request, uploaded_file_name, fake_uuid, mock_get_jobs +): + page = client_request.get( + "main.check_messages", + service_id=SERVICE_ONE_ID, + template_id="5d729fbd-239c-44ab-b498-75a985f3198f", + upload_id=fake_uuid, + original_file_name=uploaded_file_name, + _test_page_title=False, + ) + assert normalize_spaces(page.select_one(".banner-dangerous").text) == ( + "These messages have already been sent today " + "If you need to resend them, rename the file and upload it again." + ) + mock_get_jobs.assert_called_once_with(SERVICE_ONE_ID, limit_days=0) def test_check_messages_column_error_doesnt_show_optional_columns( diff --git a/tests/app/main/views/test_template_folders.py b/tests/app/main/views/test_template_folders.py index d1bf6c9d0..c4c2da4c1 100644 --- a/tests/app/main/views/test_template_folders.py +++ b/tests/app/main/views/test_template_folders.py @@ -1117,25 +1117,31 @@ def test_should_show_checkboxes_for_selecting_templates_assertion_error( mock_get_no_api_keys, user, ): - with pytest.raises( # noqa: PT012 # This will require more research into refactoring. - expected_exception=AssertionError - ): - client_request.login(user) - - page = client_request.get( - "main.choose_template", - service_id=SERVICE_ONE_ID, + with pytest.raises(expected_exception=AssertionError): + _stmt_for_test_should_show_checkboxes_for_selecting_templates_assertion_error( + client_request, user ) - checkboxes = page.select("input[name=templates_and_folders]") - assert len(checkboxes) == 4 - assert checkboxes[0]["value"] == TEMPLATE_ONE_ID - assert checkboxes[0]["id"] == "templates-or-folder-{}".format(TEMPLATE_ONE_ID) +def _stmt_for_test_should_show_checkboxes_for_selecting_templates_assertion_error( + client_request, user +): + client_request.login(user) - for index in (1, 2, 3): - assert checkboxes[index]["value"] != TEMPLATE_ONE_ID - assert TEMPLATE_ONE_ID not in checkboxes[index]["id"] + page = client_request.get( + "main.choose_template", + service_id=SERVICE_ONE_ID, + ) + checkboxes = page.select("input[name=templates_and_folders]") + + assert len(checkboxes) == 4 + + assert checkboxes[0]["value"] == TEMPLATE_ONE_ID + assert checkboxes[0]["id"] == "templates-or-folder-{}".format(TEMPLATE_ONE_ID) + + for index in (1, 2, 3): + assert checkboxes[index]["value"] != TEMPLATE_ONE_ID + assert TEMPLATE_ONE_ID not in checkboxes[index]["id"] @pytest.mark.parametrize( diff --git a/tests/conftest.py b/tests/conftest.py index 5a9ed3d0f..8b552a8fd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2423,7 +2423,7 @@ def _os_environ(): os.environ[k] = v -@pytest.fixture # noqa (C901 too complex) +@pytest.fixture() def client_request(logged_in_client, mocker, service_one): # noqa (C901 too complex) class ClientRequest: @staticmethod