From cf515b08d8ba8a9d1642d5a7b1c258bb16b721cb Mon Sep 17 00:00:00 2001 From: Cliff Hill Date: Wed, 1 Oct 2025 09:42:23 -0400 Subject: [PATCH] More fixes being done. Signed-off-by: Cliff Hill --- backend/tests/services/test_bookings.py | 23 +- backend/tests/services/test_rooms.py | 482 +++++++++++++----------- 2 files changed, 269 insertions(+), 236 deletions(-) diff --git a/backend/tests/services/test_bookings.py b/backend/tests/services/test_bookings.py index 685ef8b6..bff9de70 100644 --- a/backend/tests/services/test_bookings.py +++ b/backend/tests/services/test_bookings.py @@ -64,15 +64,6 @@ BOOKING_VALIDATION_WRONG_TYPE = ( "Invalid booking data: wrong type" # Validation error: wrong type ) -# Status code constants -BOOKING_CONFLICT_STATUS = 409 # HTTP 409 Conflict -BOOKING_SUCCESS_STATUS = 201 # HTTP 201 Created -BOOKING_NOT_FOUND_STATUS = 404 # HTTP 404 Not Found -BOOKING_DB_ERROR_STATUS = 500 # HTTP 500 Internal Server Error -BOOKING_GOOD_STATUS = 200 # HTTP 200 OK -BOOKING_VALIDATION_ERROR_STATUS = 400 # HTTP 400 Bad Request - -# Logger path for patching # Suppress AsyncMock coroutine warnings globally for this test file pytestmark = pytest.mark.filterwarnings( @@ -316,7 +307,7 @@ async def test_update_booking_success( with ( patch( "backend.services.bookings.get_room_service", - new=AsyncMock(return_value=lambda *_: sample_room), + new=AsyncMock(return_value=lambda *_: sample_room), # pyright: ignore ), patch( "backend.services.bookings.validate_no_overlap", @@ -1153,10 +1144,10 @@ async def test_new_booking_param( async_session, "scalars", AsyncMock(return_value=mock_scalars_result) ): if expected_exception: - with pytest.raises(expected_exception) as exc_info: + with pytest.raises(expected_exception) as exc_info: # pyright: ignore await new_booking(async_session, booking_create) if expected_error: - assert expected_error in str(exc_info.value) + assert expected_error in str(exc_info.value) # pyright: ignore async_session.rollback.assert_called() else: result = await new_booking(async_session, booking_create) @@ -1309,7 +1300,7 @@ async def test_update_booking_param( update_params = cast( BookingUpdateData, {k: v for k, v in update_params.items() if k != "id"} ) - with pytest.raises(expected_exception) as exc_info: + with pytest.raises(expected_exception) as exc_info: # pyright: ignore await update_booking( async_session, booking_id, @@ -1317,7 +1308,7 @@ async def test_update_booking_param( event_publisher=event_publisher, ) if expected_message: - assert expected_message in str(exc_info.value) + assert expected_message in str(exc_info.value) # pyright: ignore async_session.rollback.assert_called() else: update_params = cast( @@ -1397,10 +1388,10 @@ async def test_delete_booking_param( async_session.rollback = AsyncMock() # type: ignore[method-assign] if expected_exception: - with pytest.raises(expected_exception) as exc_info: + with pytest.raises(expected_exception) as exc_info: # pyright: ignore await delete_booking(async_session, booking_id) if expected_message: - assert expected_message in str(exc_info.value) + assert expected_message in str(exc_info.value) # pyright: ignore async_session.rollback.assert_called() else: await delete_booking(async_session, booking_id) diff --git a/backend/tests/services/test_rooms.py b/backend/tests/services/test_rooms.py index f714f90b..ac18453c 100644 --- a/backend/tests/services/test_rooms.py +++ b/backend/tests/services/test_rooms.py @@ -1,17 +1,36 @@ -"""Unit tests for the backend.services.rooms module.""" +"""Unit tests for backend.services.rooms. +This test module covers all CRUD service logic for Room objects, including: + - Retrieval of all rooms and single rooms + - Creation, update, and deletion of rooms + - Error handling for not found and database exceptions + +Style conventions: + - Google-style docstrings with "Asserts:" sections + - All test data provided via fixtures from conftest.py + - Constants used for error messages and status codes + - Parameterized tests for error scenarios and data variations + - Consistent blank lines and organized imports + - Explicit type annotations for all function signatures + +All test data is managed through fixtures in conftest.py for maintainability and reuse. +""" + +# Standard library imports from typing import Any +from typing import Callable from unittest.mock import AsyncMock from unittest.mock import MagicMock +# Third-party imports import pytest from sqlalchemy import delete from sqlalchemy import select from sqlalchemy import update -from sqlalchemy.exc import NoResultFound from sqlalchemy.exc import SQLAlchemyError from sqlalchemy.ext.asyncio import AsyncSession +# Local imports from backend.models import Room from backend.models import RoomList from backend.services.rooms import delete_room @@ -21,17 +40,27 @@ from backend.services.rooms import new_room from backend.services.rooms import update_room +# Error message constants +ROOM_NOT_FOUND_MSG = "Room not found" +ROOM_DB_ERROR_MSG = "Database error" + +# Not found ID constant for tests +ROOM_NOT_FOUND_ID = 999 + + @pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) -async def test_get_rooms_success( - async_session: AsyncSession, sample_rooms: RoomList, mock_logger: MagicMock +async def test_get_rooms_returns_all_rooms( + async_session: AsyncSession, sample_rooms: RoomList ) -> None: """Test successful retrieval of all rooms from the database. Args: async_session: The asynchronous database session. sample_rooms: The sample list of Room objects. - mock_logger: The mocked logger instance. + + Asserts: + - result is a list matching sample_rooms + - async_session.scalars called once with ``select(Room)`` """ mock_scalars_result = AsyncMock() mock_scalars_result.all = MagicMock(return_value=sample_rooms) @@ -48,15 +77,15 @@ async def test_get_rooms_success( @pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) -async def test_get_rooms_empty( - async_session: AsyncSession, mock_logger: MagicMock -) -> None: +async def test_get_rooms_returns_empty_list(async_session: AsyncSession) -> None: """Test retrieval of rooms when the database is empty. Args: async_session: The asynchronous database session. - mock_logger: The mocked logger instance. + + Asserts: + - result is an empty list + - async_session.scalars called once with ``select(Room)`` """ mock_scalars_result = AsyncMock() mock_scalars_result.all = MagicMock(return_value=[]) @@ -71,210 +100,249 @@ async def test_get_rooms_empty( assert async_session.scalars.call_args.args[0].compare(select(Room)) +# Parametrized test for get_room and delete_room error cases @pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) -async def test_get_rooms_database_error( - async_session: AsyncSession, mock_logger: MagicMock -) -> None: - """Test handling of database errors in get_rooms. - - Verifies that get_rooms raises an exception on database failure. - - Args: - async_session: The asynchronous database session. - mock_logger: The mocked logger instance. - """ - async_session.scalars = AsyncMock( # type: ignore [method-assign] - side_effect=SQLAlchemyError("Database error") - ) - - with pytest.raises(SQLAlchemyError): - await get_rooms(async_session) - async_session.scalars.assert_called_once() - - -@pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) @pytest.mark.parametrize( - "room_id, expected_name", + "service_func, session_attr, arg", [ - (1, "Room A"), - (2, "Room B"), + (get_room, "scalar", "sample_room"), + (delete_room, "execute", "sample_room"), ], - ids=["room1", "room2"], + ids=["get_room", "delete_room"], ) -async def test_get_room_success( +async def test_get_room_and_delete_room_database_error( async_session: AsyncSession, - room_id: int, - expected_name: str, - mock_logger: MagicMock, + service_func: Callable[[AsyncSession, int], Any], + session_attr: str, + arg: str, + request: pytest.FixtureRequest, ) -> None: - """Test successful retrieval of a room by ID. - - Verifies that get_room returns the correct room and constructs the correct query. + """Test handling of database errors for get_room and delete_room. Args: async_session: The asynchronous database session. - room_id: The ID of the room to retrieve. - expected_name: The expected name of the room. - mock_logger: The mocked logger instance. + service_func: The service function to test (get_room or delete_room). + session_attr: The session method to mock ("scalar" or "execute"). + arg: The fixture name for the room object. + request: The pytest request object for fixture access. + + Asserts: + - SQLAlchemyError is raised + - session method is called once + - rollback is called once for delete_room """ - room = Room( - id=room_id, - name=expected_name, - location="Building 1", - equipment="Projector", - capacity=10, + obj = request.getfixturevalue(arg) + room_id = obj.id + setattr( + async_session, + session_attr, + AsyncMock(side_effect=SQLAlchemyError(ROOM_DB_ERROR_MSG)), ) - async_session.scalar = AsyncMock(return_value=room) # type: ignore [method-assign] - - result: Room = await get_room(async_session, room_id) - - assert result.id == room_id - assert result.name == expected_name - async_session.scalar.assert_called_once() - assert async_session.scalar.call_args.args[0].compare( - select(Room).where(Room.id == room_id) - ) - - -@pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) -@pytest.mark.parametrize( - "room_id", - [999, -1], - ids=["nonexistent_id", "invalid_id"], -) -async def test_get_room_not_found( - async_session: AsyncSession, mock_logger: MagicMock, room_id: int -) -> None: - """Test handling of non-existent room in get_room. - - Verifies that get_room raises NoResultFound when the room is not found. - - Args: - async_session: The asynchronous database session. - mock_logger: The mocked logger instance. - room_id: The ID of the room to retrieve. - """ - async_session.scalar = AsyncMock(return_value=None) # type: ignore [method-assign] - - with pytest.raises(NoResultFound): - await get_room(async_session, room_id) - async_session.scalar.assert_called_once() - assert async_session.scalar.call_args.args[0].compare( - select(Room).where(Room.id == room_id) - ) - - -@pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) -async def test_get_room_database_error( - async_session: AsyncSession, mock_logger: MagicMock -) -> None: - """Test handling of database errors in get_room. - - Verifies that get_room raises an exception on database failure. - - Args: - async_session: The asynchronous database session. - mock_logger: The mocked logger instance. - """ - room_id: int = 1 - async_session.scalar = AsyncMock( # type: ignore [method-assign] - side_effect=SQLAlchemyError("Database error") - ) - + if service_func is delete_room: + async_session.rollback = AsyncMock() + async_session.commit = AsyncMock() with pytest.raises(SQLAlchemyError): - await get_room(async_session, room_id) - async_session.scalar.assert_called_once() - assert async_session.scalar.call_args.args[0].compare( - select(Room).where(Room.id == room_id) - ) + await service_func(async_session, room_id) + session_method = getattr(async_session, session_attr) + session_method.assert_called_once() + if service_func is delete_room: + # Assert rollback called + assert isinstance(async_session.rollback, AsyncMock) + async_session.rollback.assert_called_once() + # Assert the SQL statement is correct + assert session_method.call_args.args[0].compare( + delete(Room).where(Room.id == room_id) + ) + # Assert commit not called due to error + if isinstance(async_session.commit, AsyncMock): + assert async_session.commit.call_count == 0 +# Parametrized test for new_room error case @pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) -async def test_new_room_success( - async_session: AsyncSession, mock_logger: MagicMock +@pytest.mark.parametrize("sample_room", ["sample_room"], indirect=True) +async def test_new_room_database_error( + async_session: AsyncSession, sample_room: Room ) -> None: - """Test successful creation of a new room. - - Verifies that new_room adds the room, commits the session, and returns the room. + """Test handling of database errors for new_room using fixture. Args: async_session: The asynchronous database session. - mock_logger: The mocked logger instance. + sample_room: The sample Room object fixture. + + Asserts: + - SQLAlchemyError is raised + - async_session.add called once with ``room`` + - async_session.commit called once + - async_session.rollback called once """ - room = Room( - id=1, name="Room A", location="Building 1", equipment="Projector", capacity=10 - ) - async_session.add = MagicMock() # type: ignore [method-assign] - async_session.commit = AsyncMock() # type: ignore [method-assign] - - result: Room = await new_room(async_session, room) - - assert result == room - async_session.add.assert_called_once_with(room) + async_session.add = MagicMock() + async_session.commit = AsyncMock(side_effect=SQLAlchemyError(ROOM_DB_ERROR_MSG)) + async_session.rollback = AsyncMock() + with pytest.raises(SQLAlchemyError): + await new_room(async_session, sample_room) + async_session.add.assert_called_once_with(sample_room) async_session.commit.assert_called_once() + async_session.rollback.assert_called_once() + + +# Parametrized test for update_room error case +@pytest.mark.asyncio +@pytest.mark.parametrize( + "sample_room, room_update_data", + [("sample_room", "room_update_data")], + indirect=True, +) +async def test_update_room_database_error_param( + async_session: AsyncSession, sample_room: Room, room_update_data: dict[str, Any] +) -> None: + """Test handling of database errors for update_room using fixtures. + + Args: + async_session: The asynchronous database session. + sample_room: The sample Room object fixture. + room_update_data: The update data for the room. + + Asserts: + - SQLAlchemyError is raised + - async_session.execute called once + - async_session.rollback called once + """ + async_session.execute = AsyncMock(side_effect=SQLAlchemyError(ROOM_DB_ERROR_MSG)) + async_session.rollback = AsyncMock() + with pytest.raises(SQLAlchemyError): + await update_room(async_session, sample_room.id, **room_update_data) + async_session.execute.assert_called_once() + async_session.rollback.assert_called_once() + + +# Parametrized test for get_room_success (for all sample_rooms) +@pytest.mark.asyncio +@pytest.mark.parametrize( + "sample_room", + [ + pytest.param(room, id=f"room_{room.id}") + for room in [ + Room( + id=1, + name="Room 1", + location="Building A", + equipment="Projector", + capacity=10, + ), + Room( + id=2, + name="Room 2", + location="Building B", + equipment="Whiteboard", + capacity=15, + ), + ] + ], + indirect=True, +) +async def test_get_room_returns_room( + async_session: AsyncSession, sample_room: Room +) -> None: + """Test successful retrieval of a room by ID using fixture data. + + Args: + async_session: The asynchronous database session. + sample_room: The sample Room object fixture. + + Asserts: + - result matches sample_room + - async_session.scalar called once with + ``select(Room).where(Room.id == sample_room.id)`` + """ + async_session.scalar = AsyncMock(return_value=sample_room) + result: Room = await get_room(async_session, sample_room.id) + assert result == sample_room + async_session.scalar.assert_called_once() + assert async_session.scalar.call_args.args[0].compare( + select(Room).where(Room.id == sample_room.id) + ) @pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) -async def test_new_room_database_error( - async_session: AsyncSession, mock_logger: MagicMock +async def test_get_room_returns_room_fixture( + async_session: AsyncSession, sample_room: Room ) -> None: + """Test successful retrieval of a room by ID using fixture data. + + Args: + async_session: The asynchronous database session. + sample_room: The sample Room object fixture. + + Asserts: + - result matches sample_room + - async_session.scalar called once with + ``select(Room).where(Room.id == sample_room.id)`` + """ + async_session.scalar = AsyncMock(return_value=sample_room) # type: ignore [method-assign] + + result: Room = await get_room(async_session, sample_room.id) + + assert result == sample_room + async_session.scalar.assert_called_once() + assert async_session.scalar.call_args.args[0].compare( + select(Room).where(Room.id == sample_room.id) + ) """Test handling of database errors in new_room. Verifies that new_room rolls back the session on database failure. Args: async_session: The asynchronous database session. - mock_logger: The mocked logger instance. + + Asserts: + - SQLAlchemyError is raised + - async_session.add called once with room + - async_session.commit called once + - async_session.rollback called once """ - room = Room( - id=1, name="Room A", location="Building 1", equipment="Projector", capacity=10 - ) async_session.add = MagicMock() # type: ignore [method-assign] async_session.commit = AsyncMock( # type: ignore [method-assign] - side_effect=SQLAlchemyError("Database error") + side_effect=SQLAlchemyError(ROOM_DB_ERROR_MSG) ) async_session.rollback = AsyncMock() # type: ignore [method-assign] with pytest.raises(SQLAlchemyError): - await new_room(async_session, room) - async_session.add.assert_called_once_with(room) + await new_room(async_session, sample_room) + async_session.add.assert_called_once_with(sample_room) async_session.commit.assert_called_once() async_session.rollback.assert_called_once() @pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.routers.rooms"], indirect=True) -async def test_update_room_success( - async_session: AsyncSession, mock_logger: MagicMock +async def test_update_room_returns_updated_room( + async_session: AsyncSession, + sample_room: Room, + room_update_data: dict[str, Any], + updated_room: Room, ) -> None: """Test successful update of a room. - Verifies that update_room updates the room, commits the session, and returns - the updated room. - Args: async_session: The asynchronous database session. - mock_logger: The mocked logger instance. + sample_room: The sample Room object fixture. + room_update_data: The update data for the room. + updated_room: The expected updated Room object. + + Asserts: + - result matches updated_room + - async_session.execute called once with + ``update(Room).where(Room.id == room_id).values(**update_params)`` + - async_session.commit called once + - async_session.scalar called once with ``select(Room).where(Room.id == room_id)`` """ - room_id = 1 - update_params: dict[str, Any] = { - "name": "Updated Room", - "location": "Building 2", - "equipment": "Whiteboard", - "capacity": 15, - } + room_id = sample_room.id + update_params = room_update_data mock_execute_result = MagicMock(rowcount=1) async_session.execute = AsyncMock( # type: ignore [method-assign] return_value=mock_execute_result ) async_session.commit = AsyncMock() # type: ignore [method-assign] - updated_room = Room(id=room_id, **update_params) async_session.scalar = AsyncMock( # type: ignore [method-assign] return_value=updated_room ) @@ -294,19 +362,21 @@ async def test_update_room_success( @pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) -async def test_update_room_not_found( - async_session: AsyncSession, mock_logger: MagicMock -) -> None: +async def test_update_room_raises_not_found(async_session: AsyncSession) -> None: """Test handling of non-existent room in update_room. Verifies that update_room raises ValueError when the room is not found. Args: async_session: The asynchronous database session. - mock_logger: The mocked logger instance. + + Asserts: + - ValueError is raised + - async_session.execute called once with + ``update(Room).where(Room.id == room_id).values(**update_params)`` + - async_session.rollback called once """ - room_id = 999 + room_id = ROOM_NOT_FOUND_ID update_params: dict[str, Any] = { "name": "Updated Room", "location": "Building 2", @@ -329,27 +399,25 @@ async def test_update_room_not_found( @pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) async def test_update_room_database_error( - async_session: AsyncSession, mock_logger: MagicMock + async_session: AsyncSession, sample_room: Room, room_update_data: dict[str, Any] ) -> None: """Test handling of database errors in update_room. - Verifies that update_room rolls back the session on database failure. - Args: async_session: The asynchronous database session. - mock_logger: The mocked logger instance. + sample_room: The sample Room object fixture. + room_update_data: The update data for the room. + + Asserts: + - SQLAlchemyError is raised + - async_session.execute called once + - async_session.rollback called once """ - room_id = 1 - update_params: dict[str, Any] = { - "name": "Updated Room", - "location": "Building 2", - "equipment": "Whiteboard", - "capacity": 15, - } + room_id = sample_room.id + update_params = room_update_data async_session.execute = AsyncMock( # type: ignore [method-assign] - side_effect=SQLAlchemyError("Database error") + side_effect=SQLAlchemyError(ROOM_DB_ERROR_MSG) ) async_session.rollback = AsyncMock() # type: ignore [method-assign] @@ -363,19 +431,20 @@ async def test_update_room_database_error( @pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) -async def test_delete_room_success( - async_session: AsyncSession, mock_logger: MagicMock +async def test_delete_room_commits_success( + async_session: AsyncSession, sample_room: Room ) -> None: """Test successful deletion of a room. - Verifies that delete_room deletes the room and commits the session. - Args: async_session: The asynchronous database session. - mock_logger: The mocked logger instance. + sample_room: The sample Room object fixture. + + Asserts: + - async_session.execute called once with ``delete(Room).where(Room.id == room_id)`` + - async_session.commit called once """ - room_id = 1 + room_id = sample_room.id mock_execute_result = MagicMock(rowcount=1) async_session.execute = AsyncMock( # type: ignore [method-assign] return_value=mock_execute_result @@ -392,19 +461,20 @@ async def test_delete_room_success( @pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) -async def test_delete_room_not_found( - async_session: AsyncSession, mock_logger: MagicMock -) -> None: +async def test_delete_room_raises_not_found(async_session: AsyncSession) -> None: """Test handling of non-existent room in delete_room. Verifies that delete_room raises ValueError when the room is not found. Args: async_session: The asynchronous database session. - mock_logger: The mocked logger instance. + + Asserts: + - ValueError is raised + - async_session.execute called once with ``delete(Room).where(Room.id == room_id)`` + - async_session.rollback called once """ - room_id = 999 + room_id = ROOM_NOT_FOUND_ID mock_execute_result = MagicMock(rowcount=0) async_session.execute = AsyncMock( # type: ignore [method-assign] return_value=mock_execute_result @@ -418,31 +488,3 @@ async def test_delete_room_not_found( delete(Room).where(Room.id == room_id) ) async_session.rollback.assert_called_once() - - -@pytest.mark.asyncio -@pytest.mark.parametrize("mock_logger", ["backend.services.rooms"], indirect=True) -async def test_delete_room_database_error( - async_session: AsyncSession, mock_logger: MagicMock -) -> None: - """Test handling of database errors in delete_room. - - Verifies that delete_room rolls back the session on database failure. - - Args: - async_session: The asynchronous database session. - mock_logger: The mocked logger instance. - """ - room_id = 1 - async_session.execute = AsyncMock( # type: ignore [method-assign] - side_effect=SQLAlchemyError("Database error") - ) - async_session.rollback = AsyncMock() # type: ignore [method-assign] - - with pytest.raises(SQLAlchemyError): - await delete_room(async_session, room_id) - async_session.execute.assert_called_once() - assert async_session.execute.call_args.args[0].compare( - delete(Room).where(Room.id == room_id) - ) - async_session.rollback.assert_called_once()