From e2b3978fecea532545dd5a07be978f8b330d0a53 Mon Sep 17 00:00:00 2001 From: Lucas Jensen Date: Fri, 3 May 2024 18:19:20 -0700 Subject: [PATCH 1/2] finished controller unittests --- server/app/controllers/group.py | 38 ++++- server/app/controllers/users.py | 63 +++++++- server/app/db/users.py | 4 +- .../controllers/test_group_controller.py | 59 ++++++++ .../controllers/test_musician_controller.py | 14 +- .../tests/controllers/test_user_controller.py | 140 ++++++++++++++++++ 6 files changed, 300 insertions(+), 18 deletions(-) create mode 100644 server/tests/controllers/test_group_controller.py create mode 100644 server/tests/controllers/test_user_controller.py diff --git a/server/app/controllers/group.py b/server/app/controllers/group.py index fdc37ff..5ee2380 100644 --- a/server/app/controllers/group.py +++ b/server/app/controllers/group.py @@ -7,12 +7,31 @@ from app.models.group import Group class GroupController(BaseController): - def __init__(self) -> None: + """ + Handles all group-related operations and serves as an intermediate controller between + the main controller and the model layer. + Inherits from BaseController, which provides logging and other generic methods. + + The corresponding table contains only one row. + + Testing: pass a mocked GroupQueries object to the constructor. + """ + + def __init__(self, group_queries=group_queries) -> None: super().__init__() - self.db: GroupQueries = group_queries + self.group_queries: GroupQueries = group_queries def get_group(self) -> Group: - if (data := self.db.select_one_by_id()) is None: + """Retrieves the group from the database and returns it as a Group object. + + Raises: + HTTPException: If the group is not found (status code 404) + HTTPException: If any error occurs during the retrieval process (status code 500) + + Returns: + Group: A Group object which is suitable for a response body + """ + if (data := self.group_queries.select_one_by_id()) is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="Group not found" ) @@ -25,8 +44,19 @@ class GroupController(BaseController): ) def update_group_bio(self, bio: str) -> Group: + """Updates the group's bio in the database and returns the updated Group object. + + Args: + bio (str): The new bio for the group + + Raises: + HTTPException: If any error occurs during the update process (status code 500) + + Returns: + Group: The updated Group object which is suitable for a response body + """ try: - self.db.update_group_bio(bio) + self.group_queries.update_group_bio(bio) except Exception as e: raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, diff --git a/server/app/controllers/users.py b/server/app/controllers/users.py index efc8a17..6d19017 100644 --- a/server/app/controllers/users.py +++ b/server/app/controllers/users.py @@ -9,11 +9,28 @@ from app.models.user import User class UserController(BaseController): - def __init__(self) -> None: + """ + Handles all user-related operations and serves as an intermediate controller between + the main controller and the model layer. + + Inherits from BaseController, which provides logging and other generic methods. + + Testing: pass a mocked UserQueries object to the constructor. + """ + + def __init__(self, user_queries=user_queries) -> None: super().__init__() self.db: UserQueries = user_queries def get_users(self) -> list[User]: + """Retrieves all users from the database and returns them as a list of User objects. + + Raises: + HTTPException: If any error occurs during the retrieval process (status code 500) + + Returns: + list[User]: A list of User objects which are suitable for a response body + """ data = self.db.select_all_series() try: return [User(**e) for e in data] @@ -23,8 +40,20 @@ class UserController(BaseController): detail=f"Error creating user objects: {e}", ) - def get_user_by_id(self, id: int) -> User: - if (data := self.db.select_one_by_id(id)) is None: + def get_user_by_id(self, user_id: int) -> User: + """Retrieves a single user from the database and returns it as a User object. + + Args: + user_id (int): The ID of the user to retrieve + + Raises: + HTTPException: If the user is not found (status code 404) + HTTPException: If any error occurs during the retrieval process (status code 500) + + Returns: + User: A User object which is suitable for a response body + """ + if (data := self.db.select_one_by_id(user_id)) is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="User not found" ) @@ -37,7 +66,19 @@ class UserController(BaseController): ) def get_user_by_email(self, email: str) -> User: - if (data := self.db.get_one_by_email(email)) is None: + """Retrieves a single user from the database and returns it as a User object. + + Args: + email (str): The email of the user to retrieve + + Raises: + HTTPException: If the user is not found (status code 404) + HTTPException: If any error occurs during the retrieval process (status code 500) + + Returns: + User: A User object which is suitable for a response body + """ + if (data := self.db.select_one_by_email(email)) is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="User does not exist" ) @@ -50,7 +91,19 @@ class UserController(BaseController): ) def get_user_by_sub(self, sub: str) -> User: - if (data := self.db.get_one_by_sub(sub)) is None: + """Retrieves a single user from the database and returns it as a User object. + + Args: + sub (str): The sub of the user to retrieve + + Raises: + HTTPException: If the user is not found (status code 404) + HTTPException: If any error occurs during the retrieval process (status code 500) + + Returns: + User: A User object which is suitable for a response body + """ + if (data := self.db.select_one_by_sub(sub)) is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="User not found" ) diff --git a/server/app/db/users.py b/server/app/db/users.py index 2367ff3..1d59f88 100644 --- a/server/app/db/users.py +++ b/server/app/db/users.py @@ -7,7 +7,7 @@ class UserQueries(BaseQueries): super().__init__() self.table = USER_TABLE - def get_one_by_email(self, email: str) -> dict | None: + def select_one_by_email(self, email: str) -> dict | None: query = f"SELECT * FROM {self.table} WHERE email = %s" db = self.connect_db() cursor = db.cursor(dictionary=True) @@ -18,7 +18,7 @@ class UserQueries(BaseQueries): return data - def get_one_by_sub(self, sub: str) -> dict | None: + def select_one_by_sub(self, sub: str) -> dict | None: query = f"SELECT * FROM {self.table} WHERE sub = %s" db = self.connect_db() cursor = db.cursor(dictionary=True) diff --git a/server/tests/controllers/test_group_controller.py b/server/tests/controllers/test_group_controller.py new file mode 100644 index 0000000..b6d32ac --- /dev/null +++ b/server/tests/controllers/test_group_controller.py @@ -0,0 +1,59 @@ +from unittest.mock import Mock + +import pytest +from fastapi import HTTPException, status +from icecream import ic + +from app.controllers.group import GroupController +from app.models.group import Group + +mock_queries = Mock() +gc = GroupController(group_queries=mock_queries) + +valid_group_data = { + "id": 1, + "name": "Test Group", + "bio": "Test Bio", +} + +invalid_group_data = { + "id": 1, + "name": "Test Group", +} + + +def test_type(): + """Tests the type of the controller object.""" + assert isinstance(gc, GroupController) + + +def test_get_group(): + """Tests the retrieval of a group from the database with valid data.""" + mock_queries.select_one_by_id.return_value = valid_group_data + group = gc.get_group() + assert isinstance(group, Group) + assert group.id == 1 + assert group.name == "Test Group" + assert group.bio == "Test Bio" + + +def test_get_group_failure(): + """Tests a failure during the retrieval process or if the group is not found.""" + mock_queries.select_one_by_id.return_value = invalid_group_data + with pytest.raises(HTTPException) as e: + gc.get_group() + assert isinstance(e.value, HTTPException) + assert e.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + + +def test_update_group_bio(): + """This test does not test updating of the bio, but rather tests that the corresponding + method in the queries module is called with the correct arguments. + """ + new_bio = "New Bio" + mock_queries.update_group_bio = Mock() + mock_queries.select_one_by_id.return_value = valid_group_data + + group = gc.update_group_bio(new_bio) + Mock.assert_called_once_with(mock_queries.update_group_bio, new_bio) + assert isinstance(group, Group) diff --git a/server/tests/controllers/test_musician_controller.py b/server/tests/controllers/test_musician_controller.py index 3373001..f706dc7 100644 --- a/server/tests/controllers/test_musician_controller.py +++ b/server/tests/controllers/test_musician_controller.py @@ -1,4 +1,4 @@ -from unittest.mock import MagicMock, Mock +from unittest.mock import Mock import pytest from fastapi import HTTPException, UploadFile, status @@ -8,7 +8,7 @@ from app.controllers.musicians import MusicianController from app.models.musician import Musician mock_queries = Mock() -ec = MusicianController(musician_queries=mock_queries) +mc = MusicianController(musician_queries=mock_queries) sample_data = [ { @@ -55,7 +55,7 @@ mock_queries.select_one_by_id = mock_select_one_by_id def test_type(): - assert isinstance(ec, MusicianController) + assert isinstance(mc, MusicianController) """ @@ -68,7 +68,7 @@ TODO: write tests for following methods: def test_happy_get_musicians(): - musicians = ec.get_musicians() + musicians = mc.get_musicians() assert isinstance(musicians, list) assert len(musicians) == 2 for musician in musicians: @@ -87,14 +87,14 @@ def test_happy_get_musicians(): def test_sad_get_musicians(): mock_queries.select_all_series = mock_select_all_series_sad with pytest.raises(HTTPException) as e: - ec.get_musicians() + mc.get_musicians() mock_queries.select_all_series = mock_select_all_series assert isinstance(e.value, HTTPException) assert e.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR def test_happy_get_musician(): - musician = ec.get_musician(1) + musician = mc.get_musician(1) assert isinstance(musician, Musician) assert musician.id == 1 assert musician.name == "John Doe" @@ -104,7 +104,7 @@ def test_happy_get_musician(): def test_musician_not_found(): with pytest.raises(HTTPException) as e: - ec.get_musician(3) + mc.get_musician(3) assert isinstance(e.value, HTTPException) assert e.value.status_code == status.HTTP_404_NOT_FOUND assert e.value.detail == "Musician not found" diff --git a/server/tests/controllers/test_user_controller.py b/server/tests/controllers/test_user_controller.py new file mode 100644 index 0000000..df43858 --- /dev/null +++ b/server/tests/controllers/test_user_controller.py @@ -0,0 +1,140 @@ +from unittest.mock import Mock + +import pytest +from fastapi import HTTPException, status +from icecream import ic + +from app.controllers.users import UserController +from app.models.user import User + +mock_queries = Mock() +uc = UserController(user_queries=mock_queries) + +valid_user_data = [ + { + "id": 1, + "name": "John Doe", + "email": "john@doe.com", + }, + {"id": 2, "name": "Jane Doe", "email": "jane@doe.com", "sub": "1234567890"}, +] + +invalid_user_data = [ + { + "id": 1, + "name": "Jack Doe", + } +] + + +def mock_select_one_by_id(id: int): + for user in valid_user_data: + if user.get("id") == id: + return user + return None + + +def mock_select_one_by_email(email: str): + for user in valid_user_data: + if user.get("email") == email: + return user + return None + + +def mock_select_one_by_sub(sub: str): + for user in valid_user_data: + if user.get("sub") == sub: + return user + return None + + +mock_queries.select_one_by_id = mock_select_one_by_id +mock_queries.select_one_by_email = mock_select_one_by_email +mock_queries.select_one_by_sub = mock_select_one_by_sub + + +def test_type(): + """Tests the type of the controller object.""" + assert isinstance(uc, UserController) + + +def test_get_users(): + """Tests the retrieval of users from the database.""" + mock_queries.select_all_series.return_value = valid_user_data + users = uc.get_users() + assert isinstance(users, list) + assert len(users) == 2 + sub_found = False + none_sub_found = False + for user in users: + assert isinstance(user, User) + if user.sub: + sub_found = True + assert isinstance(user.sub, str) + else: + none_sub_found = True + u1, u2 = users + assert u1.id == 1 + assert u1.name == "John Doe" + assert isinstance(u1.email, str) + assert u2.id == 2 + assert u2.name == "Jane Doe" + assert isinstance(u2.email, str) + assert sub_found and none_sub_found + + +def test_get_users_sad(): + """Tests the retrieval of users from the database with invalid data.""" + mock_queries.select_all_series.return_value = invalid_user_data + with pytest.raises(HTTPException) as e: + uc.get_users() + assert isinstance(e.value, HTTPException) + assert e.value.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + + +def test_get_user_by_id(): + """Tests the retrieval of a user by id from the database.""" + + user = uc.get_user_by_id(1) + assert isinstance(user, User) + + +def test_get_user_by_invalid_id(): + """Tests the retrieval of a user by id from the database with invalid data.""" + + with pytest.raises(HTTPException) as e: + uc.get_user_by_id(10) + assert isinstance(e.value, HTTPException) + assert e.value.status_code == status.HTTP_404_NOT_FOUND + + +def test_get_user_by_email(): + """Tests the retrieval of a user by email from the database.""" + + user = uc.get_user_by_email(valid_user_data[0]["email"]) + assert isinstance(user, User) + + +def test_get_user_by_invalid_email(): + """Tests the retrieval of a user by email from the database with invalid data.""" + + with pytest.raises(HTTPException) as e: + uc.get_user_by_email("carol@cat.com") + assert isinstance(e.value, HTTPException) + assert e.value.status_code == status.HTTP_404_NOT_FOUND + + +def test_get_user_by_sub(): + """Tests the retrieval of a user by sub from the database.""" + + user = uc.get_user_by_sub(valid_user_data[1]["sub"]) + assert isinstance(user, User) + + +def test_get_user_by_invalid_sub(): + """Tests the retrieval of a user by sub from the database with invalid data.""" + + with pytest.raises(HTTPException) as e: + uc.get_user_by_sub("123abc") + assert isinstance(e.value, HTTPException) + assert e.value.status_code == status.HTTP_404_NOT_FOUND From 65e962e6488210c5a3829ea2b4909ee94de989a8 Mon Sep 17 00:00:00 2001 From: Lucas Jensen Date: Fri, 3 May 2024 18:25:10 -0700 Subject: [PATCH 2/2] switched from Mock to MagicMock --- server/tests/controllers/test_event_controller.py | 8 ++++---- server/tests/controllers/test_group_controller.py | 8 ++++---- server/tests/controllers/test_musician_controller.py | 4 ++-- server/tests/controllers/test_user_controller.py | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/server/tests/controllers/test_event_controller.py b/server/tests/controllers/test_event_controller.py index 2db327c..0419610 100644 --- a/server/tests/controllers/test_event_controller.py +++ b/server/tests/controllers/test_event_controller.py @@ -1,5 +1,5 @@ from datetime import datetime -from unittest.mock import Mock +from unittest.mock import MagicMock import pytest from icecream import ic @@ -8,7 +8,7 @@ from pydantic_core import Url from app.controllers.events import EventController from app.models.event import Event, EventSeries -mock_queries = Mock() +mock_queries = MagicMock() ec = EventController(event_queries=mock_queries) eventbrite_url = "https://www.eventbrite.com/e/the-grapefruits-duo-presents-works-for-horn-and-piano-tickets-1234567890" @@ -182,7 +182,7 @@ def test_all_series_with_many_series(): def test_all_series_with_error(): """Tests an error during the retrieval process.""" - mock_log_error = Mock() + mock_log_error = MagicMock() ec.log_error = mock_log_error def invalid_series() -> list[dict]: @@ -198,7 +198,7 @@ def test_all_series_with_error(): mock_queries.select_all_series = invalid_series with pytest.raises(Exception): ec.get_all_series() - Mock.assert_called_once(mock_log_error) + MagicMock.assert_called_once(mock_log_error) def test_one_series(): diff --git a/server/tests/controllers/test_group_controller.py b/server/tests/controllers/test_group_controller.py index b6d32ac..d34551f 100644 --- a/server/tests/controllers/test_group_controller.py +++ b/server/tests/controllers/test_group_controller.py @@ -1,4 +1,4 @@ -from unittest.mock import Mock +from unittest.mock import MagicMock import pytest from fastapi import HTTPException, status @@ -7,7 +7,7 @@ from icecream import ic from app.controllers.group import GroupController from app.models.group import Group -mock_queries = Mock() +mock_queries = MagicMock() gc = GroupController(group_queries=mock_queries) valid_group_data = { @@ -51,9 +51,9 @@ def test_update_group_bio(): method in the queries module is called with the correct arguments. """ new_bio = "New Bio" - mock_queries.update_group_bio = Mock() + mock_queries.update_group_bio = MagicMock() mock_queries.select_one_by_id.return_value = valid_group_data group = gc.update_group_bio(new_bio) - Mock.assert_called_once_with(mock_queries.update_group_bio, new_bio) + MagicMock.assert_called_once_with(mock_queries.update_group_bio, new_bio) assert isinstance(group, Group) diff --git a/server/tests/controllers/test_musician_controller.py b/server/tests/controllers/test_musician_controller.py index f706dc7..56bdd6a 100644 --- a/server/tests/controllers/test_musician_controller.py +++ b/server/tests/controllers/test_musician_controller.py @@ -1,4 +1,4 @@ -from unittest.mock import Mock +from unittest.mock import MagicMock import pytest from fastapi import HTTPException, UploadFile, status @@ -7,7 +7,7 @@ from icecream import ic from app.controllers.musicians import MusicianController from app.models.musician import Musician -mock_queries = Mock() +mock_queries = MagicMock() mc = MusicianController(musician_queries=mock_queries) sample_data = [ diff --git a/server/tests/controllers/test_user_controller.py b/server/tests/controllers/test_user_controller.py index df43858..895de12 100644 --- a/server/tests/controllers/test_user_controller.py +++ b/server/tests/controllers/test_user_controller.py @@ -1,4 +1,4 @@ -from unittest.mock import Mock +from unittest.mock import MagicMock import pytest from fastapi import HTTPException, status @@ -7,7 +7,7 @@ from icecream import ic from app.controllers.users import UserController from app.models.user import User -mock_queries = Mock() +mock_queries = MagicMock() uc = UserController(user_queries=mock_queries) valid_user_data = [