Chore: Convert more testing to pytest style (#12678)

src/documents/tests/factories.py

@@ -5,6 +5,7 @@ Factory-boy factories for documents app models.
 from __future__ import annotations
 
 import factory
+from django.contrib.auth import get_user_model
 from factory.django import DjangoModelFactory
 
 from documents.models import Correspondent
@@ -15,6 +16,8 @@ from documents.models import PaperlessTask
 from documents.models import StoragePath
 from documents.models import Tag
 
+UserModelT = get_user_model()
+
 
 class CorrespondentFactory(DjangoModelFactory[Correspondent]):
     class Meta:
@@ -68,6 +71,20 @@ class DocumentFactory(DjangoModelFactory[Document]):
     storage_path = None
 
 
+class UserFactory(DjangoModelFactory[UserModelT]):
+    class Meta:
+        model = UserModelT
+
+    username = factory.Sequence(lambda n: f"user{n}")
+    is_staff = False
+    is_superuser = False
+    password = factory.django.Password("test")
+
+    class Params:
+        superuser = factory.Trait(is_staff=True, is_superuser=True)
+        staff = factory.Trait(is_staff=True)
+
+
 class PaperlessTaskFactory(DjangoModelFactory[PaperlessTask]):
     class Meta:
         model = PaperlessTask

src/documents/tests/test_admin.py

@@ -1,12 +1,13 @@
 import types
-from unittest.mock import patch
 
+import pytest
 import tantivy
 from django.contrib.admin.sites import AdminSite
 from django.contrib.auth.models import Permission
 from django.contrib.auth.models import User
+from django.test import Client
 from django.test import TestCase
-from django.utils import timezone
+from pytest_mock import MockerFixture
 from rest_framework import status
 
 from documents.admin import DocumentAdmin
@@ -15,10 +16,28 @@ from documents.models import Document
 from documents.models import Tag
 from documents.search import get_backend
 from documents.search import reset_backend
+from documents.tests.factories import DocumentFactory
+from documents.tests.factories import TagFactory
+from documents.tests.factories import UserFactory
 from documents.tests.utils import DirectoriesMixin
 from paperless.admin import PaperlessUserAdmin
 
 
+@pytest.fixture
+def tag_admin() -> TagAdmin:
+    return TagAdmin(model=Tag, admin_site=AdminSite())
+
+
+@pytest.fixture
+def user_admin() -> PaperlessUserAdmin:
+    return PaperlessUserAdmin(model=User, admin_site=AdminSite())
+
+
+@pytest.fixture
+def staff_user(db) -> User:
+    return UserFactory.create(username="staff", staff=True)
+
+
 class TestDocumentAdmin(DirectoriesMixin, TestCase):
     def get_document_from_index(self, doc):
         backend = get_backend()
@@ -41,160 +60,176 @@ class TestDocumentAdmin(DirectoriesMixin, TestCase):
         super().tearDown()
 
     def test_save_model(self) -> None:
-        doc = Document.objects.create(title="test")
+        doc = DocumentFactory.create(title="test")
 
         doc.title = "new title"
         self.doc_admin.save_model(None, doc, None, None)
-        self.assertEqual(Document.objects.get(id=doc.id).title, "new title")
+
         self.assertEqual(self.get_document_from_index(doc)["id"], [doc.id])
 
     def test_delete_model(self) -> None:
-        doc = Document.objects.create(title="test")
+        doc = DocumentFactory.create(title="test")
         get_backend().add_or_update(doc)
         self.assertIsNotNone(self.get_document_from_index(doc))
 
         self.doc_admin.delete_model(None, doc)
 
-        self.assertRaises(Document.DoesNotExist, Document.objects.get, id=doc.id)
         self.assertIsNone(self.get_document_from_index(doc))
 
     def test_delete_queryset(self) -> None:
-        docs = []
-        for i in range(42):
-            doc = Document.objects.create(
-                title="Many documents with the same title",
-                checksum=f"{i:02}",
-            )
-            docs.append(doc)
-            get_backend().add_or_update(doc)
-
-        self.assertEqual(Document.objects.count(), 42)
-
+        docs = DocumentFactory.create_batch(
+            2,
+            title="Many documents with the same title",
+        )
         for doc in docs:
+            get_backend().add_or_update(doc)
             self.assertIsNotNone(self.get_document_from_index(doc))
 
         self.doc_admin.delete_queryset(None, Document.objects.all())
 
-        self.assertEqual(Document.objects.count(), 0)
-
         for doc in docs:
             self.assertIsNone(self.get_document_from_index(doc))
 
-    def test_created(self) -> None:
-        doc = Document.objects.create(
-            title="test",
-            created=timezone.make_aware(timezone.datetime(2020, 4, 12)),
+
+@pytest.mark.django_db
+class TestTagAdmin:
+    def test_parent_tags_get_added(
+        self,
+        tag_admin: TagAdmin,
+        mocker: MockerFixture,
+    ) -> None:
+        mock_bulk_update = mocker.patch(
+            "documents.tasks.bulk_update_documents.apply_async",
         )
-        self.assertEqual(self.doc_admin.created_(doc), "2020-04-12")
-
-
-class TestTagAdmin(DirectoriesMixin, TestCase):
-    def setUp(self) -> None:
-        super().setUp()
-        self.tag_admin = TagAdmin(model=Tag, admin_site=AdminSite())
-
-    @patch("documents.tasks.bulk_update_documents")
-    def test_parent_tags_get_added(self, mock_bulk_update) -> None:
-        document = Document.objects.create(title="test")
-        parent = Tag.objects.create(name="parent")
-        child = Tag.objects.create(name="child")
+        document = DocumentFactory.create(title="test")
+        parent = TagFactory.create(name="parent")
+        child = TagFactory.create(name="child")
         document.tags.add(child)
 
         child.tn_parent = parent
-        self.tag_admin.save_model(None, child, None, change=True)
+        tag_admin.save_model(None, child, None, change=True)
+
         document.refresh_from_db()
-        self.assertIn(parent, document.tags.all())
-
-
-class TestPaperlessAdmin(DirectoriesMixin, TestCase):
-    def setUp(self) -> None:
-        super().setUp()
-        self.user_admin = PaperlessUserAdmin(model=User, admin_site=AdminSite())
-
-    def test_request_is_passed_to_form(self) -> None:
-        user = User.objects.create(username="test", is_superuser=False)
-        non_superuser = User.objects.create(username="requestuser")
-        request = types.SimpleNamespace(user=non_superuser)
-        formType = self.user_admin.get_form(request)
-        form = formType(data={}, instance=user)
-        self.assertEqual(form.request, request)
-
-    def test_only_superuser_can_change_superuser(self) -> None:
-        superuser = User.objects.create_superuser(username="superuser", password="test")
-        non_superuser = User.objects.create(username="requestuser")
-        user = User.objects.create(username="test", is_superuser=False)
-
-        data = {
-            "username": "test",
-            "is_superuser": True,
+        assert parent in document.tags.all()
+        mock_bulk_update.assert_called_once()
+        assert mock_bulk_update.call_args.kwargs["kwargs"] == {
+            "document_ids": [document.id],
         }
-        form = self.user_admin.form(data, instance=user)
+
+
+@pytest.mark.django_db
+class TestPaperlessAdmin:
+    def test_request_is_passed_to_form(
+        self,
+        user_admin: PaperlessUserAdmin,
+    ) -> None:
+        user = UserFactory.create()
+        non_superuser = UserFactory.create()
+        request = types.SimpleNamespace(user=non_superuser)
+        form_type = user_admin.get_form(request)
+        form = form_type(data={}, instance=user)
+        assert form.request == request
+
+    def test_non_superuser_cannot_change_superuser_status(
+        self,
+        user_admin: PaperlessUserAdmin,
+    ) -> None:
+        non_superuser = UserFactory.create()
+        user = UserFactory.create()
+
+        form = user_admin.form(
+            {"username": user.username, "is_superuser": True},
+            instance=user,
+        )
         form.request = types.SimpleNamespace(user=non_superuser)
-        self.assertFalse(form.is_valid())
-        self.assertEqual(
-            form.errors.get("__all__"),
-            ["Superuser status can only be changed by a superuser"],
+
+        assert not form.is_valid()
+        assert form.errors.get("__all__") == [
+            "Superuser status can only be changed by a superuser",
+        ]
+
+    def test_superuser_can_change_superuser_status(
+        self,
+        user_admin: PaperlessUserAdmin,
+        admin_user: User,
+    ) -> None:
+        user = UserFactory.create()
+
+        form = user_admin.form(
+            {"username": user.username, "is_superuser": True},
+            instance=user,
         )
+        form.request = types.SimpleNamespace(user=admin_user)
 
-        form = self.user_admin.form(data, instance=user)
-        form.request = types.SimpleNamespace(user=superuser)
-        self.assertTrue(form.is_valid())
-        self.assertEqual({}, form.errors)
+        assert form.is_valid()
+        assert form.errors == {}
 
-    def test_superuser_can_only_be_modified_by_superuser(self) -> None:
-        superuser = User.objects.create_superuser(username="superuser", password="test")
-        user = User.objects.create(
-            username="test",
-            is_superuser=False,
-            is_staff=True,
+    @pytest.mark.parametrize(
+        ("method", "perm_codename", "expected_message"),
+        [
+            pytest.param(
+                "patch",
+                "change_user",
+                "Superusers can only be modified by other superusers",
+                id="modify",
+            ),
+            pytest.param(
+                "delete",
+                "delete_user",
+                "Superusers can only be deleted by other superusers",
+                id="delete",
+            ),
+        ],
+    )
+    def test_non_superuser_cannot_mutate_superuser(
+        self,
+        client: Client,
+        admin_user: User,
+        staff_user: User,
+        method: str,
+        perm_codename: str,
+        expected_message: str,
+    ) -> None:
+        staff_user.user_permissions.add(
+            Permission.objects.get(codename=perm_codename),
         )
-        change_user_perm = Permission.objects.get(codename="change_user")
-        user.user_permissions.add(change_user_perm)
+        client.force_login(staff_user)
 
-        self.client.force_login(user)
-        response = self.client.patch(
-            f"/api/users/{superuser.pk}/",
+        response = getattr(client, method)(
+            f"/api/users/{admin_user.pk}/",
             {"first_name": "Updated"},
             content_type="application/json",
         )
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-        self.assertEqual(
-            response.content.decode(),
-            "Superusers can only be modified by other superusers",
-        )
 
-        self.client.logout()
-        self.client.force_login(superuser)
-        response = self.client.patch(
-            f"/api/users/{superuser.pk}/",
+        assert response.status_code == status.HTTP_403_FORBIDDEN
+        assert response.content.decode() == expected_message
+        assert User.objects.filter(pk=admin_user.pk).exists()
+
+    def test_superuser_can_modify_superuser(
+        self,
+        client: Client,
+        admin_user: User,
+    ) -> None:
+        client.force_login(admin_user)
+        response = client.patch(
+            f"/api/users/{admin_user.pk}/",
             {"first_name": "Updated"},
             content_type="application/json",
         )
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        superuser.refresh_from_db()
-        self.assertEqual(superuser.first_name, "Updated")
 
-    def test_superuser_can_only_be_deleted_by_superuser(self):
-        superuser = User.objects.create_superuser(username="superuser", password="test")
-        user = User.objects.create(
-            username="test",
-            is_superuser=False,
-            is_staff=True,
-        )
-        delete_user_perm = Permission.objects.get(codename="delete_user")
-        user.user_permissions.add(delete_user_perm)
+        assert response.status_code == status.HTTP_200_OK
+        admin_user.refresh_from_db()
+        assert admin_user.first_name == "Updated"
 
-        self.client.force_login(user)
-        response = self.client.delete(f"/api/users/{superuser.pk}/")
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
-        self.assertEqual(
-            response.content.decode(),
-            "Superusers can only be deleted by other superusers",
-        )
-        self.assertTrue(User.objects.filter(pk=superuser.pk).exists())
+    def test_superuser_can_delete_superuser(
+        self,
+        client: Client,
+        admin_user: User,
+    ) -> None:
+        target = UserFactory.create(superuser=True)
+        client.force_login(admin_user)
 
-        self.client.logout()
-        self.client.force_login(superuser)
-        response = self.client.delete(f"/api/users/{superuser.pk}/")
-        self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
-        self.assertFalse(User.objects.filter(pk=superuser.pk).exists())
+        response = client.delete(f"/api/users/{target.pk}/")
+
+        assert response.status_code == status.HTTP_204_NO_CONTENT
+        assert not User.objects.filter(pk=target.pk).exists()

src/documents/tests/test_checks.py

@@ -1,43 +1,61 @@
-from unittest import mock
-
+import pytest
 from django.core.checks import Error
 from django.core.checks import Warning
-from django.test import TestCase
-from django.test import override_settings
+from pytest_django.fixtures import SettingsWrapper
+from pytest_mock import MockerFixture
 
 from documents.checks import filename_format_check
 from documents.checks import parser_check
 
 
-class TestDocumentChecks(TestCase):
-    def test_parser_check(self) -> None:
-        self.assertEqual(parser_check(None), [])
+class TestParserCheck:
+    def test_returns_empty_when_parsers_present(self) -> None:
+        assert parser_check(None) == []
 
-        with mock.patch("documents.checks.get_parser_registry") as mock_registry_fn:
-            mock_registry = mock.MagicMock()
-            mock_registry.all_parsers.return_value = []
-            mock_registry_fn.return_value = mock_registry
+    def test_returns_error_when_no_parsers(self, mocker: MockerFixture) -> None:
+        mock_registry = mocker.patch(
+            "documents.checks.get_parser_registry",
+        ).return_value
+        mock_registry.all_parsers.return_value = []
 
-            self.assertEqual(
-                parser_check(None),
-                [
-                    Error(
-                        "No parsers found. This is a bug. The consumer won't be "
-                        "able to consume any documents without parsers.",
-                    ),
-                ],
-            )
+        assert parser_check(None) == [
+            Error(
+                "No parsers found. This is a bug. The consumer won't be "
+                "able to consume any documents without parsers.",
+            ),
+        ]
 
-    def test_filename_format_check(self) -> None:
-        self.assertEqual(filename_format_check(None), [])
 
-        with override_settings(FILENAME_FORMAT="{created}/{title}"):
-            self.assertEqual(
-                filename_format_check(None),
-                [
-                    Warning(
-                        "Filename format {created}/{title} is using the old style, please update to use double curly brackets",
-                        hint="{{ created }}/{{ title }}",
-                    ),
-                ],
-            )
+class TestFilenameFormatCheck:
+    def test_returns_empty_when_unset(self) -> None:
+        assert filename_format_check(None) == []
+
+    @pytest.mark.parametrize(
+        ("filename_format", "expected_hint"),
+        [
+            pytest.param(
+                "{created}/{title}",
+                "{{ created }}/{{ title }}",
+                id="created-and-title",
+            ),
+            pytest.param(
+                "{correspondent}",
+                "{{ correspondent }}",
+                id="correspondent",
+            ),
+        ],
+    )
+    def test_warns_on_old_style_format(
+        self,
+        settings: SettingsWrapper,
+        filename_format: str,
+        expected_hint: str,
+    ) -> None:
+        settings.FILENAME_FORMAT = filename_format
+
+        assert filename_format_check(None) == [
+            Warning(
+                f"Filename format {filename_format} is using the old style, please update to use double curly brackets",
+                hint=expected_hint,
+            ),
+        ]

src/documents/tests/test_models.py

@@ -1,4 +1,4 @@
-from django.test import TestCase
+import pytest
 
 from documents.models import Correspondent
 from documents.models import Document
@@ -6,25 +6,19 @@ from documents.tests.factories import CorrespondentFactory
 from documents.tests.factories import DocumentFactory
 
 
-class CorrespondentTestCase(TestCase):
-    def test___str__(self) -> None:
-        for s in ("test", "oχi", "test with fun_charÅc'\"terß"):
-            correspondent = CorrespondentFactory.create(name=s)
-            self.assertEqual(str(correspondent), s)
-
-
-class DocumentTestCase(TestCase):
+@pytest.mark.django_db
+class TestDocument:
     def test_correspondent_deletion_does_not_cascade(self) -> None:
-        self.assertEqual(Correspondent.objects.all().count(), 0)
+        assert Correspondent.objects.count() == 0
         correspondent = CorrespondentFactory.create()
-        self.assertEqual(Correspondent.objects.all().count(), 1)
+        assert Correspondent.objects.count() == 1
 
-        self.assertEqual(Document.objects.all().count(), 0)
+        assert Document.objects.count() == 0
         DocumentFactory.create(correspondent=correspondent)
-        self.assertEqual(Document.objects.all().count(), 1)
-        self.assertIsNotNone(Document.objects.all().first().correspondent)
+        assert Document.objects.count() == 1
+        assert Document.objects.first().correspondent is not None
 
         correspondent.delete()
-        self.assertEqual(Correspondent.objects.all().count(), 0)
-        self.assertEqual(Document.objects.all().count(), 1)
-        self.assertIsNone(Document.objects.all().first().correspondent)
+        assert Correspondent.objects.count() == 0
+        assert Document.objects.count() == 1
+        assert Document.objects.first().correspondent is None

src/documents/tests/test_parsers.py

@@ -1,5 +1,7 @@
-from django.test import TestCase
-from django.test import override_settings
+from collections.abc import Generator
+
+import pytest
+from pytest_django.fixtures import SettingsWrapper
 
 from documents.parsers import get_default_file_extension
 from documents.parsers import get_supported_file_extensions
@@ -11,103 +13,115 @@ from paperless.parsers.text import TextDocumentParser
 from paperless.parsers.tika import TikaDocumentParser
 
 
-class TestParserAvailability(TestCase):
-    def test_tesseract_parser(self) -> None:
+@pytest.fixture()
+def _tika_registry(settings: SettingsWrapper) -> Generator[None, None, None]:
+    """
+    Rebuild the parser registry with Tika enabled for the duration of the
+    test, then reset on exit so other tests see the default (Tika-disabled)
+    registry.
+    """
+    settings.TIKA_ENABLED = True
+    reset_parser_registry()
+    yield
+    reset_parser_registry()
+
+
+@pytest.mark.django_db
+class TestParserAvailability:
+    @pytest.mark.parametrize(
+        ("mime_type", "ext"),
+        [
+            pytest.param("application/pdf", ".pdf", id="pdf"),
+            pytest.param("image/png", ".png", id="png"),
+            pytest.param("image/jpeg", ".jpg", id="jpeg"),
+            pytest.param("image/tiff", ".tif", id="tiff"),
+            pytest.param("image/webp", ".webp", id="webp"),
+        ],
+    )
+    def test_tesseract_parser(self, mime_type: str, ext: str) -> None:
         """
         GIVEN:
             - Various mime types
         WHEN:
             - The parser class is instantiated
         THEN:
-            - The Tesseract based parser is return
+            - The Tesseract based parser is returned
         """
-        supported_mimes_and_exts = [
-            ("application/pdf", ".pdf"),
-            ("image/png", ".png"),
-            ("image/jpeg", ".jpg"),
-            ("image/tiff", ".tif"),
-            ("image/webp", ".webp"),
-        ]
+        assert ext in get_supported_file_extensions()
+        assert get_default_file_extension(mime_type) == ext
+        assert isinstance(
+            get_parser_registry().get_parser_for_file(mime_type, "")(),
+            RasterisedDocumentParser,
+        )
 
-        supported_exts = get_supported_file_extensions()
-
-        for mime_type, ext in supported_mimes_and_exts:
-            self.assertIn(ext, supported_exts)
-            self.assertEqual(get_default_file_extension(mime_type), ext)
-            self.assertIsInstance(
-                get_parser_registry().get_parser_for_file(mime_type, "")(),
-                RasterisedDocumentParser,
-            )
-
-    def test_text_parser(self) -> None:
+    @pytest.mark.parametrize(
+        ("mime_type", "ext"),
+        [
+            pytest.param("text/plain", ".txt", id="plain"),
+            pytest.param("text/csv", ".csv", id="csv"),
+        ],
+    )
+    def test_text_parser(self, mime_type: str, ext: str) -> None:
         """
         GIVEN:
             - Various mime types of a text form
         WHEN:
             - The parser class is instantiated
         THEN:
-            - The text based parser is return
+            - The text based parser is returned
         """
-        supported_mimes_and_exts = [
-            ("text/plain", ".txt"),
-            ("text/csv", ".csv"),
-        ]
+        assert ext in get_supported_file_extensions()
+        assert get_default_file_extension(mime_type) == ext
+        assert isinstance(
+            get_parser_registry().get_parser_for_file(mime_type, "")(),
+            TextDocumentParser,
+        )
 
-        supported_exts = get_supported_file_extensions()
-
-        for mime_type, ext in supported_mimes_and_exts:
-            self.assertIn(ext, supported_exts)
-            self.assertEqual(get_default_file_extension(mime_type), ext)
-            self.assertIsInstance(
-                get_parser_registry().get_parser_for_file(mime_type, "")(),
-                TextDocumentParser,
-            )
-
-    def test_tika_parser(self) -> None:
+    @pytest.mark.usefixtures("_tika_registry")
+    @pytest.mark.parametrize(
+        ("mime_type", "ext"),
+        [
+            pytest.param(
+                "application/vnd.oasis.opendocument.text",
+                ".odt",
+                id="odt",
+            ),
+            pytest.param("text/rtf", ".rtf", id="rtf"),
+            pytest.param("application/msword", ".doc", id="doc"),
+            pytest.param(
+                "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
+                ".docx",
+                id="docx",
+            ),
+        ],
+    )
+    def test_tika_parser(self, mime_type: str, ext: str) -> None:
         """
         GIVEN:
-            - Various mime types of a office document form
+            - Various mime types of an office document form
         WHEN:
             - The parser class is instantiated
         THEN:
-            - The Tika/Gotenberg based parser is return
+            - The Tika/Gotenberg based parser is returned
         """
-        supported_mimes_and_exts = [
-            ("application/vnd.oasis.opendocument.text", ".odt"),
-            ("text/rtf", ".rtf"),
-            ("application/msword", ".doc"),
-            (
-                "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
-                ".docx",
-            ),
-        ]
-
-        self.addCleanup(reset_parser_registry)
-
-        # Reset and rebuild the registry with Tika enabled.
-        with override_settings(TIKA_ENABLED=True):
-            reset_parser_registry()
-            supported_exts = get_supported_file_extensions()
-
-            for mime_type, ext in supported_mimes_and_exts:
-                self.assertIn(ext, supported_exts)
-                self.assertEqual(get_default_file_extension(mime_type), ext)
-                self.assertIsInstance(
-                    get_parser_registry().get_parser_for_file(mime_type, "")(),
-                    TikaDocumentParser,
-                )
+        assert ext in get_supported_file_extensions()
+        assert get_default_file_extension(mime_type) == ext
+        assert isinstance(
+            get_parser_registry().get_parser_for_file(mime_type, "")(),
+            TikaDocumentParser,
+        )
 
     def test_no_parser_for_mime(self) -> None:
-        self.assertIsNone(get_parser_registry().get_parser_for_file("text/sdgsdf", ""))
+        assert get_parser_registry().get_parser_for_file("text/sdgsdf", "") is None
 
     def test_default_extension(self) -> None:
-        # Test no parser declared still returns a an extension
-        self.assertEqual(get_default_file_extension("application/zip"), ".zip")
+        # Test no parser declared still returns an extension
+        assert get_default_file_extension("application/zip") == ".zip"
 
         # Test invalid mimetype returns no extension
-        self.assertEqual(get_default_file_extension("aasdasd/dgfgf"), "")
+        assert get_default_file_extension("aasdasd/dgfgf") == ""
 
     def test_file_extension_support(self) -> None:
-        self.assertTrue(is_file_ext_supported(".pdf"))
-        self.assertFalse(is_file_ext_supported(".hsdfh"))
-        self.assertFalse(is_file_ext_supported(""))
+        assert is_file_ext_supported(".pdf")
+        assert not is_file_ext_supported(".hsdfh")
+        assert not is_file_ext_supported("")