From ca17acc557bdebde8af08d88db08e687be2524e7 Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Tue, 8 Oct 2024 16:09:07 -0400 Subject: [PATCH 01/12] test: include tests in coverage measurement As a method for confirming that every line of the tests code is executed. Signed-off-by: Mike Fiedler --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 1b2a17dfc50e..3777e7ec27f5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [tool.coverage.run] branch = true dynamic_context = "test_function" -source = ["warehouse"] +source = ["warehouse", "tests"] omit = [ # We don't want to get coverage information for our migrations. "warehouse/migrations/*", From 4982f80f7f446bf0fe442f5e0dc95d7aa00eb6af Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Fri, 1 Aug 2025 16:10:52 -0400 Subject: [PATCH 02/12] test: convert to using `exclude_also` Added in coverage 7.2.0, allowing extension to the defaults. Refs: https://coverage.readthedocs.io/en/7.10.1/config.html#config-report-exclude-also Signed-off-by: Mike Fiedler --- pyproject.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3777e7ec27f5..801fa2f6dcc3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,8 +22,7 @@ omit = [ parallel = true [tool.coverage.report] -exclude_lines = [ - "pragma: no cover", +exclude_also = [ "class \\w+\\(Interface\\):", "if (typing\\.)?TYPE_CHECKING:", ] From 99f0fd72b21d50b91721f19dd1b3b09a99b8084f Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Tue, 26 Nov 2024 18:02:53 -0500 Subject: [PATCH 03/12] test: exclude test failure conditions from coverage `pytest.fail` will fail a test case, with conditions expected never to be reached. Signed-off-by: Mike Fiedler --- pyproject.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 801fa2f6dcc3..3106abee96af 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,6 +25,9 @@ parallel = true exclude_also = [ "class \\w+\\(Interface\\):", "if (typing\\.)?TYPE_CHECKING:", + "pytest.fail\\(", # Use for test branches that should never be reached. + # Remove once resolved: https://github.com/nedbat/coveragepy/issues/1563 + "case _:\\n\\s*pytest\\.fail\\(", ] [tool.djlint] From 1d393a9ce480e451a99ccdbabb9f2387bcb64635 Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Mon, 5 May 2025 14:56:47 -0400 Subject: [PATCH 04/12] test: explicitly fail if impossible condition met Signed-off-by: Mike Fiedler --- tests/unit/accounts/test_views.py | 2 +- tests/unit/admin/views/test_organizations.py | 2 +- tests/unit/email/test_init.py | 2 +- tests/unit/legacy/api/xmlrpc/test_xmlrpc.py | 4 ++-- tests/unit/manage/test_init.py | 2 +- tests/unit/packaging/test_tasks.py | 2 +- tests/unit/search/test_tasks.py | 2 +- tests/unit/test_csrf.py | 4 ++-- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/unit/accounts/test_views.py b/tests/unit/accounts/test_views.py index f380a418d837..61fb635c91ca 100644 --- a/tests/unit/accounts/test_views.py +++ b/tests/unit/accounts/test_views.py @@ -3505,7 +3505,7 @@ def find_service(iface, name=None, context=None): return metrics if iface is IProjectService: return project_service - return pretend.stub() + pytest.fail(f"Unexpected service requested: {iface}") request = pretend.stub( find_service=pretend.call_recorder(find_service), diff --git a/tests/unit/admin/views/test_organizations.py b/tests/unit/admin/views/test_organizations.py index 25f759467445..190f6be8c765 100644 --- a/tests/unit/admin/views/test_organizations.py +++ b/tests/unit/admin/views/test_organizations.py @@ -974,7 +974,7 @@ def _organization_application_routes( elif route_name == "admin.dashboard": return "/admin/" else: - raise ValueError("No dummy route found") + pytest.fail(f"No dummy route found for {route_name}") class TestOrganizationApplicationActions: diff --git a/tests/unit/email/test_init.py b/tests/unit/email/test_init.py index 12025485708c..e871b26440ac 100644 --- a/tests/unit/email/test_init.py +++ b/tests/unit/email/test_init.py @@ -491,7 +491,7 @@ class Task: @staticmethod @pretend.call_recorder def retry(exc): - raise celery.exceptions.Retry + pytest.fail("retry should not be called") sender, task = FakeMailSender(), Task() request = pretend.stub(find_service=lambda *a, **kw: sender) diff --git a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py index d885d881981f..774a0f360b4a 100644 --- a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py +++ b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py @@ -43,7 +43,7 @@ def view(context, request): def test_ratelimiting_block(self, pyramid_services, pyramid_request, metrics): def view(context, request): - return None + pytest.fail("view should not be called") ratelimited_view = xmlrpc.ratelimit()(view) context = pretend.stub() @@ -77,7 +77,7 @@ def test_ratelimiting_block_with_hint( self, pyramid_services, pyramid_request, metrics, resets_in_delta, expected ): def view(context, request): - return None + pytest.fail("view should not be called") ratelimited_view = xmlrpc.ratelimit()(view) context = pretend.stub() diff --git a/tests/unit/manage/test_init.py b/tests/unit/manage/test_init.py index 0355de119817..96e33a8c2708 100644 --- a/tests/unit/manage/test_init.py +++ b/tests/unit/manage/test_init.py @@ -74,7 +74,7 @@ def mock_form(*args, **kwargs): @pretend.call_recorder def view(context, request): - return response + pytest.fail("view should not be called") info = pretend.stub(options={}, exception_only=False) info.options["require_reauth"] = require_reauth diff --git a/tests/unit/packaging/test_tasks.py b/tests/unit/packaging/test_tasks.py index 961a4d05c1cd..be6b17fd8a72 100644 --- a/tests/unit/packaging/test_tasks.py +++ b/tests/unit/packaging/test_tasks.py @@ -541,7 +541,7 @@ def test_insert_new_row( def find_service(name=None): if name == "gcloud.bigquery": return bigquery - raise LookupError + pytest.fail(f"Unexpected service name: {name}") db_request.find_service = find_service db_request.registry.settings = { diff --git a/tests/unit/search/test_tasks.py b/tests/unit/search/test_tasks.py index 6d9d0f9b0ce6..23c7fb66d243 100644 --- a/tests/unit/search/test_tasks.py +++ b/tests/unit/search/test_tasks.py @@ -184,7 +184,7 @@ def update_aliases(self, *, body): elif action == "remove": self.remove_alias(values["alias"], values["index"]) else: - raise ValueError(f"Unknown action: {action!r}.") + pytest.fail(f"Unknown action: {action!r}.") class FakeESClient: diff --git a/tests/unit/test_csrf.py b/tests/unit/test_csrf.py index 3d9af47d58ed..8abbbef22a85 100644 --- a/tests/unit/test_csrf.py +++ b/tests/unit/test_csrf.py @@ -37,7 +37,7 @@ def view(context, request): def test_disallows_unsafe_by_default(self, method): @pretend.call_recorder def view(context, request): - pass + pytest.fail("view should not be called") info = pretend.stub(options={}) wrapped_view = csrf.require_method_view(view, info) @@ -85,7 +85,7 @@ def view(context, request): def test_explicit_controls_exception_views(self): @pretend.call_recorder def view(context, request): - pass + pytest.fail("view should not be called") info = pretend.stub(options={"require_methods": ["POST"]}) wrapped_view = csrf.require_method_view(view, info) From b9ce9a417b162206b134437cc1b73a47ab365338 Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Tue, 26 Nov 2024 13:22:09 -0500 Subject: [PATCH 05/12] test: replace inline logic with intentional change Since the in-test logic relies on how Faker generates strings, there is zero chance that both branches will ever be true in the context of a single test run. Replace the conditionals with `str.swapcase()`. Signed-off-by: Mike Fiedler --- tests/unit/accounts/test_views.py | 8 ++---- tests/unit/legacy/api/test_json.py | 36 +++++++++----------------- tests/unit/organizations/test_views.py | 7 +---- tests/unit/packaging/test_views.py | 12 ++------- 4 files changed, 17 insertions(+), 46 deletions(-) diff --git a/tests/unit/accounts/test_views.py b/tests/unit/accounts/test_views.py index 61fb635c91ca..d1c89a35789f 100644 --- a/tests/unit/accounts/test_views.py +++ b/tests/unit/accounts/test_views.py @@ -121,15 +121,11 @@ class TestUserProfile: def test_user_redirects_username(self, db_request): user = UserFactory.create() - if user.username.upper() != user.username: - username = user.username.upper() - else: - username = user.username.lower() - db_request.current_route_path = pretend.call_recorder( lambda username: "/user/the-redirect/" ) - db_request.matchdict = {"username": username} + # Intentionally swap the case of the username to trigger the redirect + db_request.matchdict = {"username": user.username.swapcase()} result = views.profile(user, db_request) diff --git a/tests/unit/legacy/api/test_json.py b/tests/unit/legacy/api/test_json.py index 063afb1dae97..f976aadb67ef 100644 --- a/tests/unit/legacy/api/test_json.py +++ b/tests/unit/legacy/api/test_json.py @@ -126,11 +126,7 @@ def test_normalizing_redirects(self, db_request): project = ProjectFactory.create() release = ReleaseFactory.create(project=project, version="1.0") - name = project.name.lower() - if name == project.normalized_name: - name = project.name.upper() - - db_request.matchdict = {"name": name} + db_request.matchdict = {"name": project.name.swapcase()} db_request.current_route_path = pretend.call_recorder( lambda name: "/project/the-redirect/" ) @@ -367,11 +363,7 @@ def test_normalizing_redirects(self, db_request): project = ProjectFactory.create() release = ReleaseFactory.create(project=project, version="1.0") - name = project.name.lower() - if name == project.normalized_name: - name = project.name.upper() - - db_request.matchdict = {"name": name} + db_request.matchdict = {"name": project.name.swapcase()} db_request.current_route_path = pretend.call_recorder( lambda name: "/project/the-redirect/" ) @@ -446,14 +438,12 @@ def test_lookup_release( class TestJSONRelease: def test_normalizing_redirects(self, db_request): - project = ProjectFactory.create() - release = ReleaseFactory.create(project=project, version="3.0") + release = ReleaseFactory.create(version="3.0") - name = release.project.name.lower() - if name == release.project.normalized_name: - name = release.project.name.upper() - - db_request.matchdict = {"name": name, "version": "3.0"} + db_request.matchdict = { + "name": release.project.name.swapcase(), + "version": "3.0", + } db_request.current_route_path = pretend.call_recorder( lambda name: "/project/the-redirect/3.0/" ) @@ -744,14 +734,12 @@ def test_vulnerabilities_renders(self, pyramid_config, db_request, withdrawn): class TestJSONReleaseSlash: def test_normalizing_redirects(self, db_request): - project = ProjectFactory.create() - release = ReleaseFactory.create(project=project, version="3.0") - - name = release.project.name.lower() - if name == release.project.normalized_name: - name = release.project.name.upper() + release = ReleaseFactory.create(version="3.0") - db_request.matchdict = {"name": name, "version": "3.0"} + db_request.matchdict = { + "name": release.project.name.swapcase(), + "version": "3.0", + } db_request.current_route_path = pretend.call_recorder( lambda name: "/project/the-redirect/3.0/" ) diff --git a/tests/unit/organizations/test_views.py b/tests/unit/organizations/test_views.py index ef51463e26e9..18e0c6009bba 100644 --- a/tests/unit/organizations/test_views.py +++ b/tests/unit/organizations/test_views.py @@ -14,15 +14,10 @@ class TestOrganizationProfile: def test_redirects_name(self, db_request): org = OrganizationFactory.create() - if org.name.upper() != org.name: - organization_name = org.name.upper() - else: - organization_name = org.name.lower() - db_request.current_route_path = pretend.call_recorder( lambda organization: "/user/the-redirect/" ) - db_request.matchdict = {"organization": organization_name} + db_request.matchdict = {"organization": org.name.swapcase()} result = views.profile(org, db_request) diff --git a/tests/unit/packaging/test_views.py b/tests/unit/packaging/test_views.py index bd05eaa71da4..cb59d8f8d811 100644 --- a/tests/unit/packaging/test_views.py +++ b/tests/unit/packaging/test_views.py @@ -23,11 +23,7 @@ class TestProjectDetail: def test_normalizing_redirects(self, db_request): project = ProjectFactory.create() - name = project.name.lower() - if name == project.name: - name = project.name.upper() - - db_request.matchdict = {"name": name} + db_request.matchdict = {"name": project.name.swapcase()} db_request.current_route_path = pretend.call_recorder( lambda name: "/project/the-redirect/" ) @@ -131,11 +127,7 @@ def test_normalizing_name_redirects(self, db_request): project = ProjectFactory.create() release = ReleaseFactory.create(project=project, version="3.0") - name = release.project.name.lower() - if name == release.project.name: - name = release.project.name.upper() - - db_request.matchdict = {"name": name} + db_request.matchdict = {"name": project.name.swapcase()} db_request.current_route_path = pretend.call_recorder( lambda name: "/project/the-redirect/3.0/" ) From 596223d3bda62d2c6bc8009d899aecd1a99b3340 Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Mon, 5 May 2025 15:10:38 -0400 Subject: [PATCH 06/12] test: ensure query recorder is cleared Signed-off-by: Mike Fiedler --- tests/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/conftest.py b/tests/conftest.py index 71b6074fad20..943804a6aa1b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -597,6 +597,7 @@ def query_recorder(app_config): yield recorder finally: event.remove(engine, "before_cursor_execute", recorder.record) + recorder.clear() @pytest.fixture From 6c7c7a21e51b3a9ba6ec5c9639c0ff7999f8c558 Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Mon, 5 May 2025 14:51:15 -0400 Subject: [PATCH 07/12] test: ensure all branches get tested Signed-off-by: Mike Fiedler --- tests/unit/forklift/test_legacy.py | 6 +++++- tests/unit/test_db.py | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/unit/forklift/test_legacy.py b/tests/unit/forklift/test_legacy.py index 1ae09aeb065f..b867252f3b28 100644 --- a/tests/unit/forklift/test_legacy.py +++ b/tests/unit/forklift/test_legacy.py @@ -145,7 +145,11 @@ def test_exc_with_missing_message(self, monkeypatch): def test_construct_dependencies(): - types = {"requires": DependencyKind.requires, "provides": DependencyKind.provides} + types = { + "requires": DependencyKind.requires, + "provides": DependencyKind.provides, + "requires_dist": DependencyKind.requires_dist, + } meta = metadata.Metadata.from_raw( { diff --git a/tests/unit/test_db.py b/tests/unit/test_db.py index a1449f466532..342dd54e4824 100644 --- a/tests/unit/test_db.py +++ b/tests/unit/test_db.py @@ -59,6 +59,9 @@ def test_listens_for(monkeypatch): def handler(config): pass + # Ensure the function is called + handler(None) + assert venusian_attach.calls == [ pretend.call(handler, mock.ANY, category="warehouse") ] From fdbec215c1779495061a779cfc2b35f6cb12606d Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Tue, 26 Nov 2024 14:37:31 -0500 Subject: [PATCH 08/12] test: remove unused test code Signed-off-by: Mike Fiedler --- tests/common/db/integrations.py | 2 -- tests/unit/cli/test_cli.py | 5 ----- tests/unit/forklift/test_legacy.py | 16 -------------- tests/unit/i18n/test_extensions.py | 15 +++---------- tests/unit/manage/views/test_organizations.py | 21 ------------------- tests/unit/packaging/test_tasks.py | 3 +-- tests/unit/search/test_tasks.py | 6 ------ 7 files changed, 4 insertions(+), 64 deletions(-) diff --git a/tests/common/db/integrations.py b/tests/common/db/integrations.py index 046c8dd5f4c7..6e85a02809f4 100644 --- a/tests/common/db/integrations.py +++ b/tests/common/db/integrations.py @@ -15,7 +15,5 @@ class Meta: id = factory.Faker("word") source = factory.Faker("word") link = factory.Faker("uri") - aliases = factory.Sequence(lambda n: "alias" + str(n)) releases = factory.SubFactory(ReleaseFactory) details = factory.Faker("word") - fixed_in = factory.Sequence(lambda n: str(n) + ".0") diff --git a/tests/unit/cli/test_cli.py b/tests/unit/cli/test_cli.py index 8da7d1e6765a..6a6f11d9616a 100644 --- a/tests/unit/cli/test_cli.py +++ b/tests/unit/cli/test_cli.py @@ -42,11 +42,6 @@ def test_cli_help(monkeypatch, cli): configure = pretend.call_recorder(lambda: config) monkeypatch.setattr(warehouse.cli, "LazyConfig", configure) - @warehouse.cli.warehouse.command() - @click.pass_obj - def cli_test_command(obj): - assert obj is config - result = cli.invoke(warehouse.cli.warehouse, ["db", "-h"]) assert result.exit_code == 0 diff --git a/tests/unit/forklift/test_legacy.py b/tests/unit/forklift/test_legacy.py index b867252f3b28..1c6f34d4ce6f 100644 --- a/tests/unit/forklift/test_legacy.py +++ b/tests/unit/forklift/test_legacy.py @@ -5479,22 +5479,6 @@ def test_upload_for_company_organization_owned_project_fails_without_subscriptio name=project.normalized_name.replace("-", "_"), version=version ) - @pretend.call_recorder - def storage_service_store(path, file_path, *, meta): - with open(file_path, "rb") as fp: - if file_path.endswith(".metadata"): - assert fp.read() == b"Fake metadata" - else: - assert fp.read() == filebody - - storage_service = pretend.stub(store=storage_service_store) - - db_request.find_service = pretend.call_recorder( - lambda svc, name=None, context=None: { - IFileStorage: storage_service, - }.get(svc) - ) - monkeypatch.setattr( legacy, "_is_valid_dist_file", lambda *a, **kw: (True, None) ) diff --git a/tests/unit/i18n/test_extensions.py b/tests/unit/i18n/test_extensions.py index 6b4894a8ba96..a78ed64c243c 100644 --- a/tests/unit/i18n/test_extensions.py +++ b/tests/unit/i18n/test_extensions.py @@ -43,12 +43,6 @@ def __getattribute__(self, _: str): ) -def _get_with_context(value, ctx=None): - if isinstance(value, dict): - return value.get(ctx, value) - return value - - class TestFallbackInternationalizationExtension: @pytest.mark.parametrize( ( @@ -139,8 +133,7 @@ def test_gettext_fallback(self, translation, expected): @pass_context def gettext(context, string): language = context.get("LANGUAGE", "en") - value = languages.get(language, {}).get(string, string) - return _get_with_context(value) + return languages.get(language, {}).get(string, string) env = Environment( loader=DictLoader(templates), @@ -214,10 +207,8 @@ def test_ngettext_fallback( def ngettext(context, s, p, n): language = context.get("LANGUAGE", "en") if n != 1: - value = languages.get(language, {}).get(p, p) - return _get_with_context(value) - value = languages.get(language, {}).get(s, s) - return _get_with_context(value) + return languages.get(language, {}).get(p, p) + return languages.get(language, {}).get(s, s) env = Environment( loader=DictLoader(templates), diff --git a/tests/unit/manage/views/test_organizations.py b/tests/unit/manage/views/test_organizations.py index e9af21168c6d..48d8e3fbced3 100644 --- a/tests/unit/manage/views/test_organizations.py +++ b/tests/unit/manage/views/test_organizations.py @@ -949,7 +949,6 @@ def test_disable_save_organization_name( self, db_request, pyramid_user, - organization_service, user_service, monkeypatch, ): @@ -964,15 +963,6 @@ def test_disable_save_organization_name( ) ) - def rename_organization(organization_id, organization_name): - organization.name = organization_name - - monkeypatch.setattr( - organization_service, - "rename_organization", - pretend.call_recorder(rename_organization), - ) - admin = None monkeypatch.setattr( user_service, @@ -1005,7 +995,6 @@ def rename_organization(organization_id, organization_name): assert result.headers["Location"] == ( f"/manage/organization/{organization.normalized_name}/settings/" ) - assert organization_service.rename_organization.calls == [] assert send_email.calls == [] # When support for renaming orgs is re-introduced @@ -1852,7 +1841,6 @@ def test_add_organization_project_existing_project_invalid( self, db_request, pyramid_user, - organization_service, monkeypatch, ): organization = OrganizationFactory.create() @@ -1879,15 +1867,6 @@ def test_add_organization_project_existing_project_invalid( org_views, "AddOrganizationProjectForm", add_organization_project_cls ) - def add_organization_project(*args, **kwargs): - OrganizationProjectFactory.create( - organization=organization, project=project - ) - - monkeypatch.setattr( - organization_service, "add_organization_project", add_organization_project - ) - view = org_views.ManageOrganizationProjectsViews(organization, db_request) result = view.add_organization_project() diff --git a/tests/unit/packaging/test_tasks.py b/tests/unit/packaging/test_tasks.py index be6b17fd8a72..b13e6984fe91 100644 --- a/tests/unit/packaging/test_tasks.py +++ b/tests/unit/packaging/test_tasks.py @@ -440,8 +440,7 @@ def test_update_release_description(db_request): class TestUpdateBigQueryMetadata: class ListField(Field): - def process_formdata(self, valuelist): - self.data = [v.strip() for v in valuelist if v.strip()] + pass input_parameters = [ ( diff --git a/tests/unit/search/test_tasks.py b/tests/unit/search/test_tasks.py index 23c7fb66d243..9abcd64b9f03 100644 --- a/tests/unit/search/test_tasks.py +++ b/tests/unit/search/test_tasks.py @@ -202,12 +202,6 @@ def __enter__(self): def __exit__(self, exc_type, exc_value, traceback): pass - def acquire(self): - return True - - def release(self): - return True - class TestSearchLock: def test_is_subclass_of_redis_lock(self, mockredis): From e1f1b0838478298bad76a06a9f025492ad964ada Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Tue, 26 Nov 2024 17:14:07 -0500 Subject: [PATCH 09/12] test: remove unused input to test function No tests use the `branch_id` field, so the condition is never met. Signed-off-by: Mike Fiedler --- tests/unit/oidc/models/test_activestate.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/unit/oidc/models/test_activestate.py b/tests/unit/oidc/models/test_activestate.py index 41a62ccfc890..a1d178735566 100644 --- a/tests/unit/oidc/models/test_activestate.py +++ b/tests/unit/oidc/models/test_activestate.py @@ -37,7 +37,6 @@ def new_signed_claims( project_id: str = "fakeprojectid", project_path: str = "fakeorg/fakeproject", project_visibility: str = "public", - branch_id: str | None = None, ) -> SignedClaims: claims = SignedClaims( { @@ -54,8 +53,6 @@ def new_signed_claims( "builder": "pypi-publisher", } ) - if branch_id: - claims["branch_id"] = branch_id return claims From 116636ea3abf405bee33bb0c46ac2ddc2b768d04 Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Wed, 27 Nov 2024 09:40:49 -0500 Subject: [PATCH 10/12] test: remove unused conditional branches Signed-off-by: Mike Fiedler --- tests/unit/forklift/test_legacy.py | 5 +-- tests/unit/legacy/api/xmlrpc/test_xmlrpc.py | 3 +- tests/unit/manage/test_views.py | 8 ++-- tests/unit/oidc/test_views.py | 12 ++---- tests/unit/organizations/test_models.py | 26 ++----------- tests/unit/packaging/test_models.py | 42 ++------------------- tests/unit/packaging/test_tasks.py | 3 +- tests/unit/search/test_tasks.py | 2 - 8 files changed, 18 insertions(+), 83 deletions(-) diff --git a/tests/unit/forklift/test_legacy.py b/tests/unit/forklift/test_legacy.py index 1c6f34d4ce6f..c357c07a230e 100644 --- a/tests/unit/forklift/test_legacy.py +++ b/tests/unit/forklift/test_legacy.py @@ -3068,10 +3068,7 @@ def test_upload_succeeds_pep625_normalized_filename( @pretend.call_recorder def storage_service_store(path, file_path, *, meta): with open(file_path, "rb") as fp: - if file_path.endswith(".metadata"): - assert fp.read() == b"Fake metadata" - else: - assert fp.read() == filebody + assert fp.read() == filebody storage_service = pretend.stub(store=storage_service_store) diff --git a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py index 774a0f360b4a..c70f962c0c5d 100644 --- a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py +++ b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py @@ -145,8 +145,7 @@ def test_list_packages_with_serial(db_request): expected.setdefault(project.name, 0) entries = JournalEntryFactory.create_batch(10, name=project.name) for entry in entries: - if entry.id > expected[project.name]: - expected[project.name] = entry.id + expected[project.name] = entry.id assert xmlrpc.list_packages_with_serial(db_request) == expected diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index 9a2c2569e353..34ac24b282af 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -6348,8 +6348,8 @@ def test_manage_project_oidc_publishers_prefill( # The form data does not contain the provider, so we'll remove it from # the prefilled data before comparing them - if "provider" in prefilled_data: - del prefilled_data["provider"] + del prefilled_data["provider"] + form = getattr(view, form_name) assert form.data == prefilled_data @@ -6431,8 +6431,8 @@ def test_manage_project_oidc_publishers_prefill_partial( # The form data does not contain the provider, so we'll remove it from # the prefilled data before comparing them - if "provider" in prefilled_data: - del prefilled_data["provider"] + del prefilled_data["provider"] + missing_data = {k: None for k in missing_fields} # The expected form data is the prefilled data plus the missing fields # (set to None) minus the extra fields diff --git a/tests/unit/oidc/test_views.py b/tests/unit/oidc/test_views.py index afa75318537e..9e959820356a 100644 --- a/tests/unit/oidc/test_views.py +++ b/tests/unit/oidc/test_views.py @@ -610,9 +610,7 @@ def _find_publisher(claims, pending=False): ) def find_service(iface, **kw): - if iface == IOIDCPublisherService: - return oidc_service - elif iface == IMacaroonService: + if iface == IMacaroonService: return macaroon_service else: pytest.fail(iface) @@ -712,9 +710,7 @@ def _find_publisher(claims, pending=False): ) def find_service(iface, **kw): - if iface == IOIDCPublisherService: - return oidc_service - elif iface == IMacaroonService: + if iface == IMacaroonService: return macaroon_service else: pytest.fail(iface) @@ -923,9 +919,7 @@ def _find_publisher(claims, pending=False): ) def find_service(iface, **kw): - if iface == IOIDCPublisherService: - return oidc_service - elif iface == IMacaroonService: + if iface == IMacaroonService: return macaroon_service elif iface == IMetricsService: return metrics diff --git a/tests/unit/organizations/test_models.py b/tests/unit/organizations/test_models.py index ee44a5e48aed..fd11c35691b9 100644 --- a/tests/unit/organizations/test_models.py +++ b/tests/unit/organizations/test_models.py @@ -123,17 +123,9 @@ def test_acl(self, db_session): organization=organization, role_name=OrganizationRoleType.Member ) - acls = [] - for location in lineage(organization): - try: - acl = location.__acl__ - except AttributeError: - continue - - if acl and callable(acl): - acl = acl() - - acls.extend(acl) + acls = [ + item for location in lineage(organization) for item in location.__acl__() + ] assert acls == [ ( @@ -341,17 +333,7 @@ def test_acl(self, db_session): organization=organization, role_name=OrganizationRoleType.Member ) - acls = [] - for location in lineage(team): - try: - acl = location.__acl__ - except AttributeError: - continue - - if acl and callable(acl): - acl = acl() - - acls.extend(acl) + acls = [item for location in lineage(team) for item in location.__acl__()] assert acls == [ ( diff --git a/tests/unit/packaging/test_models.py b/tests/unit/packaging/test_models.py index 29a4bcc9b877..83770d77734b 100644 --- a/tests/unit/packaging/test_models.py +++ b/tests/unit/packaging/test_models.py @@ -145,17 +145,7 @@ def test_acl(self, db_session): publisher = GitHubPublisherFactory.create(projects=[project]) - acls = [] - for location in lineage(project): - try: - acl = location.__acl__ - except AttributeError: - continue - - if acl and callable(acl): - acl = acl() - - acls.extend(acl) + acls = [item for location in lineage(project) for item in location.__acl__()] assert acls == [ ( @@ -279,17 +269,7 @@ def test_acl_for_quarantined_project(self, db_session): publisher = GitHubPublisherFactory.create(projects=[project]) - acls = [] - for location in lineage(project): - try: - acl = location.__acl__ - except AttributeError: - continue - - if acl and callable(acl): - acl = acl() - - acls.extend(acl) + acls = [item for location in lineage(project) for item in location.__acl__()] _perms_read_and_upload = [ Permissions.ProjectsRead, @@ -381,17 +361,7 @@ def test_acl_for_archived_project(self, db_session): # permissions, and archived projects don't allow upload GitHubPublisherFactory.create(projects=[project]) - acls = [] - for location in lineage(project): - try: - acl = location.__acl__ - except AttributeError: - continue - - if acl and callable(acl): - acl = acl() - - acls.extend(acl) + acls = [item for location in lineage(project) for item in location.__acl__()] _perms_read_and_write = [ Permissions.ProjectsRead, @@ -931,11 +901,7 @@ def test_acl(self, db_session): acl = location.__acl__ except AttributeError: continue - - if acl and callable(acl): - acl = acl() - - acls.extend(acl) + acls.extend(acl()) assert acls == [ ( diff --git a/tests/unit/packaging/test_tasks.py b/tests/unit/packaging/test_tasks.py index b13e6984fe91..fb69ecb5bc51 100644 --- a/tests/unit/packaging/test_tasks.py +++ b/tests/unit/packaging/test_tasks.py @@ -527,8 +527,7 @@ def test_insert_new_row( # Process the mocked wtform fields for key, value in form_factory.items(): - if isinstance(value, StringField) or isinstance(value, self.ListField): - value.process(None) + value.process(None) get_table = pretend.stub(schema=bq_schema) bigquery = pretend.stub( diff --git a/tests/unit/search/test_tasks.py b/tests/unit/search/test_tasks.py index 9abcd64b9f03..ccfb30de7f3c 100644 --- a/tests/unit/search/test_tasks.py +++ b/tests/unit/search/test_tasks.py @@ -173,8 +173,6 @@ def put_alias(self, name, index): def remove_alias(self, name, alias): self.aliases[name] = [n for n in self.aliases[name] if n != alias] - if not self.aliases[name]: - del self.aliases[name] def update_aliases(self, *, body): for items in body["actions"]: From 221e5e11e24650a17b1b408c5fda54a0cac26511 Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Fri, 11 Jul 2025 14:14:38 -0400 Subject: [PATCH 11/12] test: ignore unreachable test code Signed-off-by: Mike Fiedler --- tests/conftest.py | 8 ++++---- tests/unit/cli/test_cli.py | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 943804a6aa1b..1c5c53be8e24 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -89,7 +89,7 @@ def _event( tags=None, hostname=None, ): - return None + return None # pragma: no cover @pytest.fixture @@ -739,7 +739,7 @@ class _MockRedis: def __init__(self, cache=None): self.cache = cache - if not self.cache: + if not self.cache: # pragma: no cover self.cache = dict() def __enter__(self): @@ -770,7 +770,7 @@ def hget(self, hash_, key): return None def hset(self, hash_, key, value, *_args, **_kwargs): - if hash_ not in self.cache: + if hash_ not in self.cache: # pragma: no cover self.cache[hash_] = dict() self.cache[hash_][key] = value @@ -781,7 +781,7 @@ def pipeline(self): return self def register_script(self, script): - return script + return script # pragma: no cover def scan_iter(self, search, count): del count # unused diff --git a/tests/unit/cli/test_cli.py b/tests/unit/cli/test_cli.py index 6a6f11d9616a..d11dfe818a10 100644 --- a/tests/unit/cli/test_cli.py +++ b/tests/unit/cli/test_cli.py @@ -21,6 +21,8 @@ def test_lazy_config_delays(monkeypatch): assert configure.calls == [pretend.call("thing", settings={"lol": "wat"})] +# TODO: This test doesn't actually test anything, as the command is not registered. +# The test output is effectively "command not found". def test_cli_no_settings(monkeypatch, cli): config = pretend.stub() configure = pretend.call_recorder(lambda: config) @@ -28,7 +30,7 @@ def test_cli_no_settings(monkeypatch, cli): @warehouse.cli.warehouse.command() @click.pass_obj - def cli_test_command(obj): + def cli_test_command(obj): # pragma: no cover assert obj is config result = cli.invoke(warehouse.cli.warehouse, ["cli-test-command"]) From f427f393483ed131ceedd2a96cb7ffa74193c0a1 Mon Sep 17 00:00:00 2001 From: Mike Fiedler Date: Fri, 1 Aug 2025 16:09:53 -0400 Subject: [PATCH 12/12] test: ignore unreachable test branches The test branch is being executed, but coverage isn't detecting it. Refs: https://github.com/nedbat/coveragepy/issues/2015 Signed-off-by: Mike Fiedler --- tests/unit/forklift/test_legacy.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/forklift/test_legacy.py b/tests/unit/forklift/test_legacy.py index c357c07a230e..496023bab443 100644 --- a/tests/unit/forklift/test_legacy.py +++ b/tests/unit/forklift/test_legacy.py @@ -5154,13 +5154,13 @@ def test_upload_succeeds_creates_release_metadata_2_4( ) digest = _TAR_GZ_PKG_MD5 data = _TAR_GZ_PKG_TESTDATA - elif mimetype == "application/zip": + elif mimetype == "application/zip": # pragma: no branch filename = "{}-{}.zip".format( project.normalized_name.replace("-", "_"), "1.0" ) digest = _ZIP_PKG_MD5 data = _ZIP_PKG_TESTDATA - elif filetype == "bdist_wheel": + elif filetype == "bdist_wheel": # pragma: no branch filename = "{}-{}-py3-none-any.whl".format( project.normalized_name.replace("-", "_"), "1.0" ) @@ -5272,14 +5272,14 @@ def test_upload_fails_missing_license_file_metadata_2_4( ) digest = _TAR_GZ_PKG_MD5 data = _TAR_GZ_PKG_TESTDATA - elif mimetype == "application/zip": + elif mimetype == "application/zip": # pragma: no branch filename = "{}-{}.zip".format( project.normalized_name.replace("-", "_"), "1.0" ) digest = _ZIP_PKG_MD5 data = _ZIP_PKG_TESTDATA license_filename = "fake_package-1.0/LICENSE" - elif filetype == "bdist_wheel": + elif filetype == "bdist_wheel": # pragma: no branch filename = "{}-{}-py3-none-any.whl".format( project.normalized_name.replace("-", "_"), "1.0",