Convert more fixtures to helpers

Author: medihack

adit/batch_transfer/tests/test_forms.py

@@ -4,32 +4,16 @@
 from django.core.files import File
 from pytest_mock import MockerFixture
 
-from adit.batch_transfer.utils.testing_helpers import create_batch_transfer_group
-from adit.core.factories import DicomServerFactory
+from adit.batch_transfer.utils.testing_helpers import (
+    create_batch_transfer_group,
+    create_data_dict,
+    create_file_dict,
+)
 from adit.core.utils.auth_utils import grant_access
 
 from ..forms import BatchTransferJobForm
 
 
-@pytest.fixture
-def data_dict():
-    return {
-        "source": DicomServerFactory(),
-        "destination": DicomServerFactory(),
-        "project_name": "Apollo project",
-        "project_description": "Fly to the moon",
-        "ethics_application_id": "12345",
-    }
-
-
-@pytest.fixture
-def file_dict(mocker: MockerFixture):
-    file = mocker.create_autospec(File, size=5242880)
-    file.name = "sample_sheet.xlsx"
-    file.read.return_value.decode.return_value = ""
-    return {"batch_file": file}
-
-
 @pytest.mark.django_db
 def test_field_labels(mocker: MockerFixture):
     # Arrange
@@ -53,17 +37,19 @@ def test_field_labels(mocker: MockerFixture):
 
 
 @pytest.mark.django_db
-def test_with_valid_data(mocker: MockerFixture, data_dict, file_dict):
+def test_with_valid_data(mocker: MockerFixture):
     # Arrange
     user = UserFactory.create()
     group = create_batch_transfer_group()
     add_user_to_group(user, group)
+    data_dict = create_data_dict()
     grant_access(group, data_dict["source"], source=True)
     grant_access(group, data_dict["destination"], destination=True)
     parse_mock = mocker.patch(
         "adit.batch_transfer.forms.BatchTransferFileParser.parse", autospec=True
     )
     parse_mock.return_value = []
+    file_dict = create_file_dict()
 
     # Act
     form = BatchTransferJobForm(data_dict, file_dict, user=user)
@@ -95,14 +81,16 @@ def test_with_missing_values(mocker: MockerFixture):
 
 
 @pytest.mark.django_db
-def test_disallow_too_large_file(mocker: MockerFixture, data_dict):
+def test_disallow_too_large_file(mocker: MockerFixture):
     # Arrange
     file = mocker.create_autospec(File, size=5242881)
     file.name = "sample_sheet.xlsx"
 
     user = UserFactory.create()
     mocker.patch.object(user, "has_perm", return_value=True)
 
+    data_dict = create_data_dict()
+
     # Act
     form = BatchTransferJobForm(data_dict, {"batch_file": file}, user=user)
 

adit/batch_transfer/tests/test_views.py

@@ -1,61 +1,29 @@
-from io import BytesIO
-
-import pandas as pd
 import pytest
 from adit_radis_shared.accounts.factories import UserFactory
 from adit_radis_shared.common.utils.auth_utils import add_user_to_group
-from django.core.files.uploadedfile import SimpleUploadedFile
+from django.conf import LazySettings
+from django.test import Client
 from django.urls import reverse
 from procrastinate.contrib.django.models import ProcrastinateJob
 from pytest_django.asserts import assertTemplateUsed
 
-from adit.batch_transfer.utils.testing_helpers import create_batch_transfer_group
-from adit.core.factories import DicomServerFactory
+from adit.batch_transfer.utils.testing_helpers import create_batch_transfer_group, create_form_data
 from adit.core.models import DicomServer
 from adit.core.utils.auth_utils import grant_access
 
 from ..models import BatchTransferJob
 
 
-# Somehow the form data must be always generated from scratch (maybe cause of the
-# SimpleUploadedFile) otherwise tests fail.
-@pytest.fixture
-def form_data(db):
-    buffer = BytesIO()
-    data = pd.DataFrame(
-        [
-            ["1001", "1.2.840.113845.11.1000000001951524609.20200705182951.2689481", "WSOHMP4N"],
-            ["1002", "1.2.840.113845.11.1000000001951524609.20200705170836.2689469", "C2XJQ2AR"],
-            ["1003", "1.2.840.113845.11.1000000001951524609.20200705172608.2689471", "KRS8CZ3S"],
-        ],
-        columns=["PatientID", "StudyInstanceUID", "Pseudonym"],  # type: ignore
-    )
-    data.to_excel(buffer, index=False)  # type: ignore
-
-    return {
-        "source": DicomServerFactory.create().pk,
-        "destination": DicomServerFactory.create().pk,
-        "project_name": "Apollo project",
-        "project_description": "Fly to the moon",
-        "ethics_application_id": "12345",
-        "batch_file": SimpleUploadedFile(
-            name="sample_sheet.xlsx",
-            content=buffer.getvalue(),
-            content_type="application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
-        ),
-    }
-
-
 @pytest.mark.django_db
-def test_user_must_be_logged_in_to_access_view(client):
+def test_user_must_be_logged_in_to_access_view(client: Client):
     response = client.get(reverse("batch_transfer_job_create"))
     assert response.status_code == 302
     response = client.post(reverse("batch_transfer_job_create"))
     assert response.status_code == 302
 
 
 @pytest.mark.django_db
-def test_user_must_have_permission_to_access_view(client):
+def test_user_must_have_permission_to_access_view(client: Client):
     user = UserFactory.create()
     client.force_login(user)
     response = client.get(reverse("batch_transfer_job_create"))
@@ -65,7 +33,7 @@ def test_user_must_have_permission_to_access_view(client):
 
 
 @pytest.mark.django_db
-def test_logged_in_user_with_permission_can_access_form(client):
+def test_logged_in_user_with_permission_can_access_form(client: Client):
     user = UserFactory.create()
     group = create_batch_transfer_group()
     add_user_to_group(user, group)
@@ -75,13 +43,15 @@ def test_logged_in_user_with_permission_can_access_form(client):
     assertTemplateUsed(response, "batch_transfer/batch_transfer_job_form.html")
 
 
-def test_batch_job_created_and_enqueued_with_auto_verify(client, settings, form_data):
+@pytest.mark.django_db
+def test_batch_job_created_and_enqueued_with_auto_verify(client: Client, settings: LazySettings):
     settings.START_BATCH_TRANSFER_UNVERIFIED = True
 
     user = UserFactory.create()
     group = create_batch_transfer_group()
     add_user_to_group(user, group)
 
+    form_data = create_form_data()
     source_server = DicomServer.objects.get(pk=form_data["source"])
     destination_server = DicomServer.objects.get(pk=form_data["destination"])
     grant_access(group, source_server, source=True)
@@ -95,13 +65,17 @@ def test_batch_job_created_and_enqueued_with_auto_verify(client, settings, form_
     assert ProcrastinateJob.objects.count() == 3
 
 
-def test_batch_job_created_and_not_enqueued_without_auto_verify(client, settings, form_data):
+@pytest.mark.django_db
+def test_batch_job_created_and_not_enqueued_without_auto_verify(
+    client: Client, settings: LazySettings
+):
     settings.START_BATCH_TRANSFER_UNVERIFIED = False
 
     user = UserFactory.create()
     group = create_batch_transfer_group()
     add_user_to_group(user, group)
 
+    form_data = create_form_data()
     source_server = DicomServer.objects.get(pk=form_data["source"])
     destination_server = DicomServer.objects.get(pk=form_data["destination"])
     grant_access(group, source_server, source=True)
@@ -115,11 +89,13 @@ def test_batch_job_created_and_not_enqueued_without_auto_verify(client, settings
     assert ProcrastinateJob.objects.count() == 0
 
 
-def test_job_cant_be_created_with_missing_fields(client, form_data):
+@pytest.mark.django_db
+def test_job_cant_be_created_with_missing_fields(client: Client):
     user = UserFactory.create()
     group = create_batch_transfer_group()
     add_user_to_group(user, group)
     client.force_login(user)
+    form_data = create_form_data()
     for key_to_exclude in form_data:
         invalid_form_data = form_data.copy()
         del invalid_form_data[key_to_exclude]