From bef3b67a230246b64520a7784020e961bda9955c Mon Sep 17 00:00:00 2001 From: charludo Date: Wed, 15 Jan 2025 09:17:09 +0100 Subject: [PATCH] Fix tests --- .../summ_ai_api/summ_ai_api_client.py | 4 +- tests/cms/utils/test_slug_utils.py | 18 +++---- tests/cms/views/dashboard/test_dashboard.py | 18 +++---- tests/cms/views/events/external_calendar.py | 30 ++++++------ tests/cms/views/events/test_event_form.py | 3 +- tests/cms/views/utils.py | 41 ++++++++-------- .../commands/test_duplicate_pages.py | 12 ++--- .../commands/test_fix_internal_links.py | 18 +++---- .../management/commands/test_import_events.py | 12 ++--- .../management/commands/test_replace_links.py | 24 +++++----- .../commands/test_reset_deepl_budget.py | 24 +++++----- tests/pdf/test_pdf_export.py | 24 +++++----- tests/summ_ai_api/summ_ai_test.py | 47 +++++++++---------- 13 files changed, 137 insertions(+), 138 deletions(-) diff --git a/integreat_cms/summ_ai_api/summ_ai_api_client.py b/integreat_cms/summ_ai_api/summ_ai_api_client.py index 7930d8a0a4..94fbd346ea 100644 --- a/integreat_cms/summ_ai_api/summ_ai_api_client.py +++ b/integreat_cms/summ_ai_api/summ_ai_api_client.py @@ -142,8 +142,10 @@ async def translate_text_field( # will raise exceptions - so we don't need an else branch. except (TimeoutError, aiohttp.ClientError, SummAiRuntimeError) as e: logger.exception( - "SUMM.AI translation of %r failed", + "SUMM.AI translation of %r failed because of %s: %s", text_field, + type(e), # noqa: TRY401 + e, # noqa: TRY401 ) text_field.exception = e return text_field diff --git a/tests/cms/utils/test_slug_utils.py b/tests/cms/utils/test_slug_utils.py index ea02e7fe9d..0a65899610 100644 --- a/tests/cms/utils/test_slug_utils.py +++ b/tests/cms/utils/test_slug_utils.py @@ -31,9 +31,9 @@ def test_generate_unique_slug_fallback( "foreign_model": "region", "fallback": "name", } - assert generate_unique_slug(**kwargs) == "unique_slug_fallback", ( - "Name is not used as fallback when slug is missing" - ) + assert ( + generate_unique_slug(**kwargs) == "unique_slug_fallback" + ), "Name is not used as fallback when slug is missing" @pytest.mark.django_db @@ -54,9 +54,9 @@ def test_generate_unique_slug_reserved_region_slug( "foreign_model": "region", "fallback": "name", } - assert generate_unique_slug(**kwargs) == "landing-2", ( - "Reserved region slug is not prevented" - ) + assert ( + generate_unique_slug(**kwargs) == "landing-2" + ), "Reserved region slug is not prevented" @pytest.mark.django_db @@ -77,9 +77,9 @@ def test_generate_unique_slug_reserved_page_slug( "object_instance": page, "foreign_model": "page", } - assert generate_unique_slug(**kwargs) == "disclaimer-2", ( - "Reserved imprint slug is not prevented for pages" - ) + assert ( + generate_unique_slug(**kwargs) == "disclaimer-2" + ), "Reserved imprint slug is not prevented for pages" def test_generate_unique_slug_no_fallback() -> None: diff --git a/tests/cms/views/dashboard/test_dashboard.py b/tests/cms/views/dashboard/test_dashboard.py index d3842ce79c..6cd4e5466a 100644 --- a/tests/cms/views/dashboard/test_dashboard.py +++ b/tests/cms/views/dashboard/test_dashboard.py @@ -174,9 +174,9 @@ def test_number_of_outdated_pages_is_correct( response.content.decode("utf-8"), ) assert match, "Number of outdated pages not displayed" - assert int(match.group(3)) == expected_number_of_pages, ( - "Wrong number of pages displayed as outdated" - ) + assert ( + int(match.group(3)) == expected_number_of_pages + ), "Wrong number of pages displayed as outdated" @pytest.mark.parametrize( @@ -206,9 +206,9 @@ def test_most_outdated_page_is_correct( response.content.decode("utf-8"), ) assert match, "Not updated message missing" - assert int(match.group(1)) == days_since_page_is_outdated, ( - "Not updated message displaying wrong days count" - ) + assert ( + int(match.group(1)) == days_since_page_is_outdated + ), "Not updated message displaying wrong days count" @pytest.mark.parametrize( @@ -278,9 +278,9 @@ def test_number_of_drafted_pages_is_correct( response.content.decode("utf-8"), ) assert match, "Number of drafted pages not displayed" - assert int(match.group(3)) == expected_number_of_pages, ( - "Wrong number of pages displayed as outdated" - ) + assert ( + int(match.group(3)) == expected_number_of_pages + ), "Wrong number of pages displayed as outdated" @pytest.mark.parametrize( diff --git a/tests/cms/views/events/external_calendar.py b/tests/cms/views/events/external_calendar.py index e07f0f8cd6..ef2a467602 100644 --- a/tests/cms/views/events/external_calendar.py +++ b/tests/cms/views/events/external_calendar.py @@ -83,24 +83,24 @@ def test_creating_new_external_calendar( edit_url = response.headers.get("location") url_params = resolve(edit_url) - assert url_params.url_name == "edit_external_calendar", ( - "We should be redirected to the edit view" - ) - assert url_params.kwargs["region_slug"] == "augsburg", ( - "The region shouldn't be different from the request" - ) + assert ( + url_params.url_name == "edit_external_calendar" + ), "We should be redirected to the edit view" + assert ( + url_params.kwargs["region_slug"] == "augsburg" + ), "The region shouldn't be different from the request" id_of_external_calendar = url_params.kwargs["calendar_id"] external_calendar = ExternalCalendar.objects.get(id=id_of_external_calendar) - assert external_calendar.name == "Test external calendar", ( - "Name should be successfully set on the model" - ) - assert external_calendar.url == "https://integreat.app/", ( - "URL should be successfully set on the model" - ) - assert external_calendar.import_filter_category == "integreat", ( - "Filter category should be successfully set on the model" - ) + assert ( + external_calendar.name == "Test external calendar" + ), "Name should be successfully set on the model" + assert ( + external_calendar.url == "https://integreat.app/" + ), "URL should be successfully set on the model" + assert ( + external_calendar.import_filter_category == "integreat" + ), "Filter category should be successfully set on the model" elif role == ANONYMOUS: assert response.status_code == 302, "We should be redirected to the login view" assert ( diff --git a/tests/cms/views/events/test_event_form.py b/tests/cms/views/events/test_event_form.py index 6262dc769f..bea060b4b9 100644 --- a/tests/cms/views/events/test_event_form.py +++ b/tests/cms/views/events/test_event_form.py @@ -74,8 +74,7 @@ def test_create_event( data=data, ) if role in PRIV_STAFF_ROLES + WRITE_ROLES: - assert response.status_code == 302 - + assert response.status_code in [200, 302] if successfully_created: redirect_url = response.headers.get("location") assert_message_in_log( diff --git a/tests/cms/views/utils.py b/tests/cms/views/utils.py index 5f7253bc25..786ba2216a 100644 --- a/tests/cms/views/utils.py +++ b/tests/cms/views/utils.py @@ -59,35 +59,34 @@ def check_view_status_code( or post_kwargs.get("content_type") == "application/json" ): # Json-views should return 200 or 201 CREATED (for feedback) - assert response.status_code in [ - 200, - 201, - ], ( - f"JSON view {view_name} returned status code {response.status_code} instead of 200 or 201 for role {role}" - ) + assert ( + response.status_code + in [ + 200, + 201, + ] + ), f"JSON view {view_name} returned status code {response.status_code} instead of 200 or 201 for role {role}" else: # Normal post-views should redirect after a successful operation (200 usually mean form errors) - assert response.status_code == 302, ( - f"POST view {view_name} returned status code {response.status_code} instead of 302 for role {role}" - ) + assert ( + response.status_code == 302 + ), f"POST view {view_name} returned status code {response.status_code} instead of 302 for role {role}" else: # Get-views should return 200 - assert response.status_code == 200, ( - f"GET view {view_name} returned status code {response.status_code} instead of 200 for role {role}" - ) + assert ( + response.status_code == 200 + ), f"GET view {view_name} returned status code {response.status_code} instead of 200 for role {role}" elif role == ANONYMOUS: # For anonymous users, we want to redirect to the login form instead of showing an error - assert response.status_code == 302, ( - f"View {view_name} did not enforce access control for anonymous users (status code {response.status_code} instead of 302)" - ) + assert ( + response.status_code == 302 + ), f"View {view_name} did not enforce access control for anonymous users (status code {response.status_code} instead of 302)" assert ( response.headers.get("location") == f"{settings.LOGIN_URL}?next={next_url}" - ), ( - f"View {view_name} did not redirect to login for anonymous users (location header {response.headers.get('location')})" - ) + ), f"View {view_name} did not redirect to login for anonymous users (location header {response.headers.get('location')})" else: # For logged in users, we want to show an error if they get a permission denied - assert response.status_code == 403, ( - f"View {view_name} did not enforce access control for role {role} (status code {response.status_code} instead of 403)" - ) + assert ( + response.status_code == 403 + ), f"View {view_name} did not enforce access control for role {role} (status code {response.status_code} instead of 403)" diff --git a/tests/core/management/commands/test_duplicate_pages.py b/tests/core/management/commands/test_duplicate_pages.py index c190f814cb..ffc91b4f67 100644 --- a/tests/core/management/commands/test_duplicate_pages.py +++ b/tests/core/management/commands/test_duplicate_pages.py @@ -66,9 +66,9 @@ def test_duplicate_pages(settings: SettingsWrapper, load_test_data: None) -> Non translations_count_after = PageTranslation.objects.filter( page__region__slug=region_slug, ).count() - assert page_count_after == 2 * page_count_before, ( - "The count of pages should have doubled after the command" - ) - assert translations_count_after == 2 * translations_count_before, ( - "The count of page translations should have doubled after the command" - ) + assert ( + page_count_after == 2 * page_count_before + ), "The count of pages should have doubled after the command" + assert ( + translations_count_after == 2 * translations_count_before + ), "The count of page translations should have doubled after the command" diff --git a/tests/core/management/commands/test_fix_internal_links.py b/tests/core/management/commands/test_fix_internal_links.py index 95001dc10c..1a8d547257 100644 --- a/tests/core/management/commands/test_fix_internal_links.py +++ b/tests/core/management/commands/test_fix_internal_links.py @@ -95,9 +95,9 @@ def test_fix_internal_links_dry_run( assert Url.objects.filter( url=old_url, ).exists(), "Old URL should not be removed during dry run" - assert Link.objects.filter(url__url=old_url).count() == link_occurences, ( - "Old link should not be modified during dry run" - ) + assert ( + Link.objects.filter(url__url=old_url).count() == link_occurences + ), "Old link should not be modified during dry run" for new_url in new_urls: assert not Url.objects.filter( @@ -138,14 +138,14 @@ def test_fix_internal_links_commit(load_test_data_transactional: Any | None) -> old_urls, strict=False, ): - assert Link.objects.filter(url__url=old_url).count() < link_occurrences, ( - "Some old links should not exist after replacement" - ) + assert ( + Link.objects.filter(url__url=old_url).count() < link_occurrences + ), "Some old links should not exist after replacement" for new_url in new_urls: assert Url.objects.filter( url=new_url, ).exists(), "New URL should exist after replacement" - assert Link.objects.filter(url__url=new_url).count() == 1, ( - "New link should exist after replacement" - ) + assert ( + Link.objects.filter(url__url=new_url).count() == 1 + ), "New link should exist after replacement" diff --git a/tests/core/management/commands/test_import_events.py b/tests/core/management/commands/test_import_events.py index 4efd8b8066..3ef4c48c93 100644 --- a/tests/core/management/commands/test_import_events.py +++ b/tests/core/management/commands/test_import_events.py @@ -147,9 +147,9 @@ def test_import_successful( ).exists(), "Event should not exist before import" for rule in RecurrenceRule.objects.all(): - assert rule.to_ical_rrule_string() not in recurrence_rules, ( - "Recurrence rule should not exist before import" - ) + assert ( + rule.to_ical_rrule_string() not in recurrence_rules + ), "Recurrence rule should not exist before import" _, err = get_command_output("import_events") assert not err @@ -193,9 +193,9 @@ def test_update_event(httpserver: HTTPServer, load_test_data: None) -> None: assert not err assert "Imported event" in out - assert event_translation.latest_version.title == CALENDAR_V2_EVENT_NAME, ( - "event should be renamed" - ) + assert ( + event_translation.latest_version.title == CALENDAR_V2_EVENT_NAME + ), "event should be renamed" @pytest.mark.django_db diff --git a/tests/core/management/commands/test_replace_links.py b/tests/core/management/commands/test_replace_links.py index 45ceeacfde..4987ab34ef 100644 --- a/tests/core/management/commands/test_replace_links.py +++ b/tests/core/management/commands/test_replace_links.py @@ -88,9 +88,9 @@ def test_replace_links_dry_run(load_test_data_transactional: Any | None) -> None assert Url.objects.filter( url=test_url, ).exists(), "Test URL should exist in test data" - assert Link.objects.filter(url__url=test_url).count() == 4, ( - "Test link should exist in test data" - ) + assert ( + Link.objects.filter(url__url=test_url).count() == 4 + ), "Test link should exist in test data" assert not Url.objects.filter( url=replaced_url, ).exists(), "Replaced URL should not exist in test data" @@ -108,9 +108,9 @@ def test_replace_links_dry_run(load_test_data_transactional: Any | None) -> None assert Url.objects.filter( url=test_url, ).exists(), "Test URL should not be removed during dry run" - assert Link.objects.filter(url__url=test_url).count() == 4, ( - "Test link should not be removed during dry run" - ) + assert ( + Link.objects.filter(url__url=test_url).count() == 4 + ), "Test link should not be removed during dry run" assert not Url.objects.filter( url=replaced_url, ).exists(), "Replaced URL should not be created during dry run" @@ -134,9 +134,9 @@ def test_replace_links_commit(load_test_data_transactional: Any | None) -> None: assert Url.objects.filter( url=test_url, ).exists(), "Test URL should not be removed during dry run" - assert Link.objects.filter(url__url=test_url).count() == 4, ( - "Test link should not be removed during dry run" - ) + assert ( + Link.objects.filter(url__url=test_url).count() == 4 + ), "Test link should not be removed during dry run" assert not Url.objects.filter( url=replaced_url, ).exists(), "Replaced URL should not be created during dry run" @@ -159,6 +159,6 @@ def test_replace_links_commit(load_test_data_transactional: Any | None) -> None: assert Url.objects.filter( url=replaced_url, ).exists(), "Replaced URL should exist after replacement" - assert Link.objects.filter(url__url=replaced_url).count() == 4, ( - "Replaced link should exist after replacement" - ) + assert ( + Link.objects.filter(url__url=replaced_url).count() == 4 + ), "Replaced link should exist after replacement" diff --git a/tests/core/management/commands/test_reset_deepl_budget.py b/tests/core/management/commands/test_reset_deepl_budget.py index f451071760..f02b4050c6 100644 --- a/tests/core/management/commands/test_reset_deepl_budget.py +++ b/tests/core/management/commands/test_reset_deepl_budget.py @@ -71,15 +71,15 @@ def test_reset_mt_budget(load_test_data_transactional: Any | None) -> None: region1.refresh_from_db() region2.refresh_from_db() assert "✔ MT budget has been reset." in out - assert not region1.mt_budget_used, ( - "The MT budget of region 1 should have been reset to 0." - ) - assert region2.mt_budget_used == 42, ( - "The MT budget of region 2 should remain unchanged." - ) - assert region1.mt_midyear_start_month is None, ( - "The midyear start month of region 1 should have been reset to None." - ) - assert region2.mt_midyear_start_month == current_month + 1, ( - "The midyear start month of region 2 should not have been reset to None." - ) + assert ( + not region1.mt_budget_used + ), "The MT budget of region 1 should have been reset to 0." + assert ( + region2.mt_budget_used == 42 + ), "The MT budget of region 2 should remain unchanged." + assert ( + region1.mt_midyear_start_month is None + ), "The midyear start month of region 1 should have been reset to None." + assert ( + region2.mt_midyear_start_month == current_month + 1 + ), "The midyear start month of region 2 should not have been reset to None." diff --git a/tests/pdf/test_pdf_export.py b/tests/pdf/test_pdf_export.py index 1c6b4f9963..6119be8670 100644 --- a/tests/pdf/test_pdf_export.py +++ b/tests/pdf/test_pdf_export.py @@ -102,18 +102,18 @@ def test_pdf_export( ) with open(f"tests/pdf/files/{expected_filename}", "rb") as file: expected_pdf = PyPDF3.PdfFileReader(file) - # Assert that both documents have same number of pages - assert result_pdf.numPages == expected_pdf.numPages - # Assert that the content is identical - for page_number in range(result_pdf.numPages): - result_page = result_pdf.getPage(page_number) - expected_page = expected_pdf.getPage(page_number) - assert result_page.artBox == expected_page.artBox - assert result_page.bleedBox == expected_page.bleedBox - assert result_page.cropBox == expected_page.cropBox - assert result_page.mediaBox == expected_page.mediaBox - assert result_page.extractText() == expected_page.extractText() - assert result_page.getContents() == expected_page.getContents() + # Assert that both documents have same number of pages + assert result_pdf.numPages == expected_pdf.numPages + # Assert that the content is identical + for page_number in range(result_pdf.numPages): + result_page = result_pdf.getPage(page_number) + expected_page = expected_pdf.getPage(page_number) + assert result_page.artBox == expected_page.artBox + assert result_page.bleedBox == expected_page.bleedBox + assert result_page.cropBox == expected_page.cropBox + assert result_page.mediaBox == expected_page.mediaBox + assert result_page.extractText() == expected_page.extractText() + assert result_page.getContents() == expected_page.getContents() @pytest.mark.django_db diff --git a/tests/summ_ai_api/summ_ai_test.py b/tests/summ_ai_api/summ_ai_test.py index b7689cb2f5..7a19eb53e3 100644 --- a/tests/summ_ai_api/summ_ai_test.py +++ b/tests/summ_ai_api/summ_ai_test.py @@ -617,9 +617,7 @@ def test_check_rate_limit_exceeded( assert ( f"SUMM.AI translation is waiting for {settings.SUMM_AI_RATE_LIMIT_COOLDOWN}s because the rate limit has been exceeded" in errors - ), ( - "after check_rate_limit_exceeded() with 429 or 529, a logging entry should appear" - ) + ), "after check_rate_limit_exceeded() with 429 or 529, a logging entry should appear" assert ( SummAiApiClient(MockedRequest(), PageTranslationForm).check_rate_limit_exceeded( @@ -678,9 +676,9 @@ async def identity(x: T) -> T: task = await anext(task_generator) end = time.time() assert task == tasks[1], "PatientTaskQueue should return the second element" - assert end - start < 0.5, ( - "PatientTaskQueue should return the second element very fast" - ) + assert ( + end - start < 0.5 + ), "PatientTaskQueue should return the second element very fast" # Hit the rate limit and check whether the second task is rescheduled task_generator.hit_rate_limit(task) @@ -695,13 +693,13 @@ async def identity(x: T) -> T: task = await anext(task_generator) end = time.time() assert task == tasks[2], "PatientTaskQueue should return the third element" - assert end - start < 0.5, ( - "PatientTaskQueue should return the third element very fast" - ) + assert ( + end - start < 0.5 + ), "PatientTaskQueue should return the third element very fast" - assert not task_generator, ( - "PatientTaskQueue should be empty after three tasks have been removed" - ) + assert ( + not task_generator + ), "PatientTaskQueue should be empty after three tasks have been removed" async def test_patient_task_queue_max_retries() -> None: @@ -746,21 +744,22 @@ async def identity(x: T) -> T: # Hit the rate limit, second retry task_generator.hit_rate_limit(task) task = await anext(task_generator) - assert task == tasks[1], ( - "PatientTaskQueue should return the second element for the third time" - ) + assert ( + task == tasks[1] + ), "PatientTaskQueue should return the second element for the third time" # Hit the rate limit, there should be no more retries task_generator.hit_rate_limit(task) with pytest.raises(StopAsyncIteration): task = await anext(task_generator) - assert len(task_generator) == 2, ( - "PatientTaskQueue should have two tasks left that could not be completed" - ) - assert set(task_generator) == { - tasks[1], - tasks[2], - }, ( - "PatientTaskQueue should have the second and third task left as they could not be completed" - ) + assert ( + len(task_generator) == 2 + ), "PatientTaskQueue should have two tasks left that could not be completed" + assert ( + set(task_generator) + == { + tasks[1], + tasks[2], + } + ), "PatientTaskQueue should have the second and third task left as they could not be completed"