fix: polish tests (#113)

* fix: add test coverage

* use xml

* rename codecov yaml

* add yml path

* ignore codeforlife test helpers

* reset pipeline

* reset pipeline

* use updated workflow

* polish_test

* add todo

* Merge branch 'main' into polish_tests

* fix: test job

* use new test workflow

* add some small tests and don't cover some lines

* no cov when debugging tests

* test get queryset

* update config

* merge from main

* use main

.devcontainer.json

@@ -18,7 +18,9 @@
         "qwtel.sqlite-viewer",
         "njpwerner.autodocstring",
         "tamasfe.even-better-toml",
-        "github.vscode-github-actions"
+        "github.vscode-github-actions",
+        "codecov.codecov",
+        "ritwickdey.liveserver"
       ]
     }
   },

.vscode/launch.json

@@ -9,6 +9,9 @@
       "type": "debugpy"
     },
     {
+      "env": {
+        "PYTEST_ADDOPTS": "--no-cov"
+      },
       "justMyCode": false,
       "name": "Pytest",
       "presentation": {

.vscode/settings.json

@@ -63,6 +63,8 @@
   "python.defaultInterpreterPath": ".venv/bin/python",
   "python.testing.pytestArgs": [
     "-n=auto",
+    "--cov",
+    "--cov-report=html",
     "-c=pyproject.toml",
     "."
   ],

codecov.yml

@@ -6,5 +6,10 @@ coverage:
     patch:
       default:
         target: 90%
+        threshold: 1%
+    project:
+      default:
+        target: 90%
+        threshold: 1%
 
 comment: false

codeforlife/tests/model.py

@@ -25,7 +25,6 @@ def get_model_class(cls) -> t.Type[AnyModel]:
         Returns:
             The model's class.
         """
-
         # pylint: disable-next=no-member
         return t.get_args(cls.__orig_bases__[0])[  # type: ignore[attr-defined]
             0
@@ -37,7 +36,6 @@ def assert_raises_integrity_error(self, *args, **kwargs):
         Returns:
             Error catcher that will assert if an integrity error is raised.
         """
-
         return self.assertRaises(IntegrityError, *args, **kwargs)
 
     def assert_check_constraint(self, name: str, *args, **kwargs):
@@ -75,11 +73,26 @@ def assert_does_not_exist(self, model_or_pk: t.Union[AnyModel, t.Any]):
         Args:
             model_or_pk: The model itself or its primary key.
         """
-
-        model_class = self.get_model_class()
         with self.assertRaises(ObjectDoesNotExist):
             if isinstance(model_or_pk, Model):
                 model_or_pk.refresh_from_db()
             else:
-                objects = model_class.objects  # type: ignore[attr-defined]
-                objects.get(pk=model_or_pk)
+                (
+                    self.get_model_class().objects  # type: ignore[attr-defined]
+                ).get(pk=model_or_pk)
+
+    def assert_get_queryset(
+        self, values: t.Collection[AnyModel], ordered: bool = True
+    ):
+        """Assert that the expected queryset is returned.
+
+        Args:
+            values: The values we expect the queryset to contain.
+            ordered: Whether the queryset provides an implicit ordering.
+        """
+        queryset = (
+            self.get_model_class().objects  # type: ignore[attr-defined]
+        ).get_queryset()
+        if ordered and not queryset.ordered:
+            queryset = queryset.order_by("pk")
+        self.assertQuerysetEqual(queryset, values, ordered=ordered)

codeforlife/user/models/auth_factor.py

@@ -11,7 +11,7 @@
 
 from .user import User
 
-if t.TYPE_CHECKING:
+if t.TYPE_CHECKING:  # pragma: no cover
     from .session_auth_factor import SessionAuthFactor
 
 
@@ -37,4 +37,4 @@ class Meta:
         unique_together = ["user", "type"]
 
     def __str__(self):
-        return self.type
+        return str(self.type)

codeforlife/user/models/auth_factor_test.py

@@ -0,0 +1,15 @@
+"""
+© Ocado Group
+Created on 16/04/2024 at 14:29:21(+01:00).
+"""
+
+from ...tests import ModelTestCase
+from .auth_factor import AuthFactor
+
+
+# pylint: disable-next=missing-class-docstring
+class TestAuthFactor(ModelTestCase[AuthFactor]):
+    def test_str(self):
+        """String representation is as expected."""
+        auth_factor_type = AuthFactor.Type.OTP
+        assert str(AuthFactor(type=auth_factor_type)) == str(auth_factor_type)

codeforlife/user/models/otp_bypass_token_test.py

@@ -19,17 +19,16 @@ def setUp(self):
         assert user
         self.user = user
 
-    def test_bulk_create(self):
+    def test_objects__bulk_create(self):
         """Can bulk create a new set of tokens."""
         original_otp_bypass_tokens = list(self.user.otp_bypass_tokens.all())
 
         otp_bypass_tokens = OtpBypassToken.objects.bulk_create(self.user)
 
-        assert len(otp_bypass_tokens) == OtpBypassToken.max_count
-
         for otp_bypass_token in original_otp_bypass_tokens:
             self.assert_does_not_exist(otp_bypass_token)
 
+        assert len(otp_bypass_tokens) == OtpBypassToken.max_count
         assert len(otp_bypass_tokens) == self.user.otp_bypass_tokens.count()
 
         for otp_bypass_token in otp_bypass_tokens:
@@ -42,7 +41,7 @@ def test_bulk_create(self):
             )
             assert check_password(raw_token, otp_bypass_token.token)
 
-    def test_create(self):
+    def test_save(self):
         """Cannot create or update a single instance."""
         with self.assert_raises_integrity_error():
             OtpBypassToken().save()

codeforlife/user/models/session.py

@@ -14,7 +14,7 @@
 
 from .user import User
 
-if t.TYPE_CHECKING:
+if t.TYPE_CHECKING:  # pragma: no cover
     from .session_auth_factor import SessionAuthFactor
 
 
@@ -81,10 +81,7 @@ def create_model_instance(self, data):
             session.user = User.objects.get(id=user_id)
             SessionAuthFactor.objects.bulk_create(
                 [
-                    SessionAuthFactor(
-                        session=session,
-                        auth_factor=auth_factor,
-                    )
+                    SessionAuthFactor(session=session, auth_factor=auth_factor)
                     for auth_factor in session.user.auth_factors.all()
                 ]
             )

codeforlife/user/models/session_auth_factor_test.py

@@ -0,0 +1,18 @@
+"""
+© Ocado Group
+Created on 16/04/2024 at 14:36:42(+01:00).
+"""
+
+from ...tests import ModelTestCase
+from .auth_factor import AuthFactor
+from .session_auth_factor import SessionAuthFactor
+
+
+# pylint: disable-next=missing-class-docstring
+class TestSessionAuthFactor(ModelTestCase[SessionAuthFactor]):
+    def test_str(self):
+        """String representation is as expected."""
+        auth_factor_type = AuthFactor.Type.OTP
+        assert str(
+            SessionAuthFactor(auth_factor=AuthFactor(type=auth_factor_type))
+        ) == str(auth_factor_type)

codeforlife/user/models/session_test.py

@@ -0,0 +1,29 @@
+"""
+© Ocado Group
+Created on 16/04/2024 at 14:40:11(+01:00).
+"""
+
+from datetime import timedelta
+from unittest.mock import patch
+
+from django.utils import timezone
+
+from ...tests import ModelTestCase
+from .session import Session
+
+
+# pylint: disable-next=missing-class-docstring
+class TestSession(ModelTestCase[Session]):
+    def test_is_expired(self):
+        """Can check if a session is expired."""
+        now = timezone.now()
+
+        session = Session(expire_date=now - timedelta(hours=1))
+        with patch.object(timezone, "now", return_value=now) as timezone_now:
+            assert session.is_expired
+            timezone_now.assert_called_once()
+
+        session = Session(expire_date=now + timedelta(hours=1))
+        with patch.object(timezone, "now", return_value=now) as timezone_now:
+            assert not session.is_expired
+            timezone_now.assert_called_once()

codeforlife/user/models/student_test.py

@@ -0,0 +1,16 @@
+"""
+© Ocado Group
+Created on 16/04/2024 at 14:54:18(+01:00).
+"""
+
+from ...tests import ModelTestCase
+from .student import Independent
+
+
+# pylint: disable-next=missing-class-docstring
+class TestIndependent(ModelTestCase[Independent]):
+    def test_objects__get_queryset(self):
+        """Check if only get independent students."""
+        self.assert_get_queryset(
+            values=Independent.objects.filter(class_field__isnull=True)
+        )

codeforlife/user/models/user.py

@@ -22,7 +22,7 @@
 from .klass import Class
 from .school import School
 
-if t.TYPE_CHECKING:
+if t.TYPE_CHECKING:  # pragma: no cover
     from .auth_factor import AuthFactor
     from .otp_bypass_token import OtpBypassToken
     from .session import Session

codeforlife/views/model.py

@@ -20,13 +20,11 @@
 from .api import APIView
 from .decorators import action
 
-if t.TYPE_CHECKING:
-    from ..serializers import ModelListSerializer, ModelSerializer
-
-
 AnyModel = t.TypeVar("AnyModel", bound=Model)
 
-if t.TYPE_CHECKING:
+if t.TYPE_CHECKING:  # pragma: no cover
+    from ..serializers import ModelListSerializer, ModelSerializer
+
     # NOTE: This raises an error during runtime.
     # pylint: disable-next=too-few-public-methods
     class _ModelViewSet(DrfModelViewSet[AnyModel], t.Generic[AnyModel]):
@@ -117,32 +115,32 @@ class _ModelListSerializer(
 
     # pylint: disable=useless-parent-delegation
 
-    def destroy(  # type: ignore[override]
+    def destroy(  # type: ignore[override] # pragma: no cover
         self, request: Request[RequestUser], *args, **kwargs
     ):
         return super().destroy(request, *args, **kwargs)
 
-    def create(  # type: ignore[override]
+    def create(  # type: ignore[override] # pragma: no cover
         self, request: Request[RequestUser], *args, **kwargs
     ):
         return super().create(request, *args, **kwargs)
 
-    def list(  # type: ignore[override]
+    def list(  # type: ignore[override] # pragma: no cover
         self, request: Request[RequestUser], *args, **kwargs
     ):
         return super().list(request, *args, **kwargs)
 
-    def retrieve(  # type: ignore[override]
+    def retrieve(  # type: ignore[override] # pragma: no cover
         self, request: Request[RequestUser], *args, **kwargs
     ):
         return super().retrieve(request, *args, **kwargs)
 
-    def update(  # type: ignore[override]
+    def update(  # type: ignore[override] # pragma: no cover
         self, request: Request[RequestUser], *args, **kwargs
     ):
         return super().update(request, *args, **kwargs)
 
-    def partial_update(  # type: ignore[override]
+    def partial_update(  # type: ignore[override] # pragma: no cover
         self, request: Request[RequestUser], *args, **kwargs
     ):
         return super().partial_update(request, *args, **kwargs)

pyproject.toml

@@ -11,7 +11,6 @@ extend-exclude = ".*/migrations/.*py"
 
 [tool.pytest.ini_options]
 env = ["DJANGO_SETTINGS_MODULE=manage"]
-addopts = "--cov --cov-report=xml"
 
 [tool.mypy]
 plugins = ["mypy_django_plugin.main", "mypy_drf_plugin.main"]