From 1407952ba193d14538977703a062cc6ec51371f8 Mon Sep 17 00:00:00 2001 From: Zach Mullen Date: Wed, 16 Aug 2023 16:34:37 -0400 Subject: [PATCH] Update tests to use Django's test client --- isic/conftest.py | 5 ++++ isic/core/api/collection.py | 3 +- isic/core/serializers.py | 5 ++-- isic/core/tests/test_api_collection.py | 41 +++++++++++++------------- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/isic/conftest.py b/isic/conftest.py index cdb2a5db..b9e3a3be 100644 --- a/isic/conftest.py +++ b/isic/conftest.py @@ -44,6 +44,11 @@ def api_client() -> APIClient: return APIClient() +@pytest.fixture +def client() -> Client: + return Client() + + @pytest.fixture def authenticated_client(user): # Do not use the client fixture, to prevent mutating its state diff --git a/isic/core/api/collection.py b/isic/core/api/collection.py index 503d3059..795b00f5 100644 --- a/isic/core/api/collection.py +++ b/isic/core/api/collection.py @@ -42,7 +42,7 @@ def collection_list(request, pinned: bool | None = None) -> list[CollectionOut]: return queryset -@router.get("/{id}", response=CollectionOut) +@router.get("/{id}/", response=CollectionOut) def collection_detail(request, id) -> CollectionOut: qs = get_visible_objects(request.user, "core.view_collection", Collection.objects.all()) collection = get_object_or_404(qs, id=id) @@ -51,6 +51,7 @@ def collection_detail(request, id) -> CollectionOut: @router.post("/{id}/populate-from-search/", response={202: None, 403: dict, 409: dict}) def collection_populate_from_search(request, id, payload: SearchQueryIn): + breakpoint() collection = get_object_or_404(Collection, id=id) if not request.user.has_perm("core.add_images", collection): diff --git a/isic/core/serializers.py b/isic/core/serializers.py index 4d9b2e0b..604269aa 100644 --- a/isic/core/serializers.py +++ b/isic/core/serializers.py @@ -7,7 +7,7 @@ from django.shortcuts import get_object_or_404 from isic_metadata import FIELD_REGISTRY from ninja import Schema -from pydantic import validator +from pydantic import validator, field_validator from pyparsing.exceptions import ParseException from rest_framework import serializers from rest_framework.fields import Field @@ -89,7 +89,7 @@ def to_queryset(self, qs: Optional[QuerySet[Image]] = None) -> QuerySet[Image]: # Update this to use context for the user once django-ninja supports it class SearchQueryIn(Schema): query: str | None - collections: list[int] | None + collections: list[int] | None = None @validator("query") @classmethod @@ -100,6 +100,7 @@ def valid_search_query(cls, value: str): try: parse_query(value) except ParseException: + # TODO this doesn't do the right thing in ninja... raise ValueError("Couldn't parse search query.") return value diff --git a/isic/core/tests/test_api_collection.py b/isic/core/tests/test_api_collection.py index d35af674..61ff7085 100644 --- a/isic/core/tests/test_api_collection.py +++ b/isic/core/tests/test_api_collection.py @@ -8,15 +8,15 @@ def collections(public_collection, private_collection): @pytest.mark.django_db @pytest.mark.parametrize( - "client,colls,num_visible", + "client_,colls,num_visible", [ - [pytest.lazy_fixture("api_client"), pytest.lazy_fixture("collections"), 1], + [pytest.lazy_fixture("client"), pytest.lazy_fixture("collections"), 1], [ - pytest.lazy_fixture("authenticated_api_client"), + pytest.lazy_fixture("authenticated_client"), pytest.lazy_fixture("collections"), 1, ], - [pytest.lazy_fixture("staff_api_client"), pytest.lazy_fixture("collections"), 2], + [pytest.lazy_fixture("staff_client"), pytest.lazy_fixture("collections"), 2], ], ids=[ "guest", @@ -24,8 +24,8 @@ def collections(public_collection, private_collection): "staff", ], ) -def test_core_api_collection_list_permissions(client, colls, num_visible): - r = client.get("/api/v2/collections/") +def test_core_api_collection_list_permissions(client_, colls, num_visible): + r = client_.get("/api/v2/collections/") assert r.status_code == 200, r.json() assert r.json()["count"] == num_visible @@ -33,22 +33,22 @@ def test_core_api_collection_list_permissions(client, colls, num_visible): @pytest.mark.django_db @pytest.mark.parametrize( - "client,collection,visible", + "client_,collection,visible", [ - [pytest.lazy_fixture("api_client"), pytest.lazy_fixture("public_collection"), True], + [pytest.lazy_fixture("client"), pytest.lazy_fixture("public_collection"), True], [ - pytest.lazy_fixture("authenticated_api_client"), + pytest.lazy_fixture("authenticated_client"), pytest.lazy_fixture("public_collection"), True, ], - [pytest.lazy_fixture("staff_api_client"), pytest.lazy_fixture("public_collection"), True], - [pytest.lazy_fixture("api_client"), pytest.lazy_fixture("private_collection"), False], + [pytest.lazy_fixture("staff_client"), pytest.lazy_fixture("public_collection"), True], + [pytest.lazy_fixture("client"), pytest.lazy_fixture("private_collection"), False], [ - pytest.lazy_fixture("authenticated_api_client"), + pytest.lazy_fixture("authenticated_client"), pytest.lazy_fixture("private_collection"), False, ], - [pytest.lazy_fixture("staff_api_client"), pytest.lazy_fixture("private_collection"), True], + [pytest.lazy_fixture("staff_client"), pytest.lazy_fixture("private_collection"), True], ], ids=[ "guest-public", @@ -59,8 +59,8 @@ def test_core_api_collection_list_permissions(client, colls, num_visible): "staff-private", ], ) -def test_core_api_collection_detail_permissions(client, collection, visible): - r = client.get(f"/api/v2/collections/{collection.pk}/") +def test_core_api_collection_detail_permissions(client_, collection, visible): + r = client_.get(f"/api/v2/collections/{collection.pk}/") if visible: assert r.status_code == 200, r.json() @@ -84,7 +84,8 @@ def test_core_api_collection_populate_from_search( with django_capture_on_commit_callbacks(execute=True): r = authenticated_client.post( - f"/api/v2/collections/{collection.pk}/populate-from-search/", {"query": "sex:male"} + f"/api/v2/collections/{collection.pk}/populate-from-search/", {"query": "sex:male"}, + content_type='application/json' ) assert r.status_code == 202, r.json() @@ -111,7 +112,7 @@ def test_core_api_collection_modify_locked(endpoint, data, staff_client, collect @pytest.mark.django_db def test_core_api_collection_populate_from_list( - authenticated_api_client, collection_factory, image_factory, user + authenticated_client, collection_factory, image_factory, user ): collection = collection_factory(locked=False, creator=user, public=True) public_image = image_factory(accession__metadata={"sex": "male"}, public=True) @@ -122,7 +123,7 @@ def test_core_api_collection_populate_from_list( user, through_defaults={"creator": private_image_shared.accession.creator} ) - r = authenticated_api_client.post( + r = authenticated_client.post( f"/api/v2/collections/{collection.pk}/populate-from-list/", { "isic_ids": [ @@ -145,7 +146,7 @@ def test_core_api_collection_populate_from_list( @pytest.mark.django_db def test_core_api_collection_remove_from_list( - authenticated_api_client, collection_factory, image_factory, user + authenticated_client, collection_factory, image_factory, user ): collection = collection_factory(locked=False, creator=user, public=False) public_image = image_factory(accession__metadata={"sex": "male"}, public=True) @@ -158,7 +159,7 @@ def test_core_api_collection_remove_from_list( collection.images.add(public_image, private_image_shared, private_image_unshared) - r = authenticated_api_client.post( + r = authenticated_client.post( f"/api/v2/collections/{collection.pk}/remove-from-list/", { "isic_ids": [