From 05e17e2259b70075c791a601504009d6147f0a93 Mon Sep 17 00:00:00 2001 From: Stefan Kairinos Date: Thu, 18 Apr 2024 17:20:59 +0100 Subject: [PATCH] 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 | 4 ++- .vscode/launch.json | 3 ++ .vscode/settings.json | 2 ++ codecov.yml | 5 ++++ codeforlife/tests/model.py | 25 ++++++++++++---- codeforlife/user/models/auth_factor.py | 4 +-- codeforlife/user/models/auth_factor_test.py | 15 ++++++++++ .../user/models/otp_bypass_token_test.py | 7 ++--- codeforlife/user/models/session.py | 7 ++--- .../user/models/session_auth_factor_test.py | 18 ++++++++++++ codeforlife/user/models/session_test.py | 29 +++++++++++++++++++ codeforlife/user/models/student_test.py | 16 ++++++++++ codeforlife/user/models/user.py | 2 +- codeforlife/views/model.py | 20 ++++++------- pyproject.toml | 1 - 15 files changed, 127 insertions(+), 31 deletions(-) create mode 100644 codeforlife/user/models/auth_factor_test.py create mode 100644 codeforlife/user/models/session_auth_factor_test.py create mode 100644 codeforlife/user/models/session_test.py create mode 100644 codeforlife/user/models/student_test.py diff --git a/.devcontainer.json b/.devcontainer.json index a24389c..b3b3e08 100644 --- a/.devcontainer.json +++ b/.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" ] } }, diff --git a/.vscode/launch.json b/.vscode/launch.json index 014d2ef..9ff416d 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -9,6 +9,9 @@ "type": "debugpy" }, { + "env": { + "PYTEST_ADDOPTS": "--no-cov" + }, "justMyCode": false, "name": "Pytest", "presentation": { diff --git a/.vscode/settings.json b/.vscode/settings.json index c6a4edd..f37f2b8 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -63,6 +63,8 @@ "python.defaultInterpreterPath": ".venv/bin/python", "python.testing.pytestArgs": [ "-n=auto", + "--cov", + "--cov-report=html", "-c=pyproject.toml", "." ], diff --git a/codecov.yml b/codecov.yml index e5d28c1..069b0a7 100644 --- a/codecov.yml +++ b/codecov.yml @@ -6,5 +6,10 @@ coverage: patch: default: target: 90% + threshold: 1% + project: + default: + target: 90% + threshold: 1% comment: false diff --git a/codeforlife/tests/model.py b/codeforlife/tests/model.py index c1067d0..e04eaa0 100644 --- a/codeforlife/tests/model.py +++ b/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) diff --git a/codeforlife/user/models/auth_factor.py b/codeforlife/user/models/auth_factor.py index 8558d47..a85edfe 100644 --- a/codeforlife/user/models/auth_factor.py +++ b/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) diff --git a/codeforlife/user/models/auth_factor_test.py b/codeforlife/user/models/auth_factor_test.py new file mode 100644 index 0000000..ef3acaa --- /dev/null +++ b/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) diff --git a/codeforlife/user/models/otp_bypass_token_test.py b/codeforlife/user/models/otp_bypass_token_test.py index ec46201..8b7c766 100644 --- a/codeforlife/user/models/otp_bypass_token_test.py +++ b/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() diff --git a/codeforlife/user/models/session.py b/codeforlife/user/models/session.py index 299a525..25ca724 100644 --- a/codeforlife/user/models/session.py +++ b/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() ] ) diff --git a/codeforlife/user/models/session_auth_factor_test.py b/codeforlife/user/models/session_auth_factor_test.py new file mode 100644 index 0000000..6d85920 --- /dev/null +++ b/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) diff --git a/codeforlife/user/models/session_test.py b/codeforlife/user/models/session_test.py new file mode 100644 index 0000000..83f297b --- /dev/null +++ b/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() diff --git a/codeforlife/user/models/student_test.py b/codeforlife/user/models/student_test.py new file mode 100644 index 0000000..63a003d --- /dev/null +++ b/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) + ) diff --git a/codeforlife/user/models/user.py b/codeforlife/user/models/user.py index cad4231..2e6d0fd 100644 --- a/codeforlife/user/models/user.py +++ b/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 diff --git a/codeforlife/views/model.py b/codeforlife/views/model.py index 7059134..14ed52f 100644 --- a/codeforlife/views/model.py +++ b/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) diff --git a/pyproject.toml b/pyproject.toml index 1a3c55f..e4089ff 100644 --- a/pyproject.toml +++ b/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"]