Improve test coverage #74

Author: joelvdavies

test/unit/repositories/test_catalogue_category.py

@@ -529,13 +529,20 @@ def call_list(self, parent_id: Optional[str]) -> None:
             parent_id=parent_id, session=self.mock_session
         )
 
-    def check_list_success(self) -> None:
-        """Checks that a prior call to `call_list` worked as expected."""
+    def check_list_success(self, assert_find: bool = True) -> None:
+        """Checks that a prior call to `call_list` worked as expected.
+
+        :param assert_find: If `True` it asserts whether a `find_one` call was made, else it asserts that no call was
+                            made.
+        """
 
         self.mock_utils.list_query.assert_called_once_with(self._parent_id_filter, "catalogue categories")
-        self.catalogue_categories_collection.find.assert_called_once_with(
-            self.mock_utils.list_query.return_value, session=self.mock_session
-        )
+        if assert_find:
+            self.catalogue_categories_collection.find.assert_called_once_with(
+                self.mock_utils.list_query.return_value, session=self.mock_session
+            )
+        else:
+            self.catalogue_categories_collection.find.assert_not_called()
 
         assert self._obtained_catalogue_categories_out == self._expected_catalogue_categories_out
 
@@ -586,6 +593,14 @@ def test_list_with_parent_id_with_no_results(self):
         self.call_list(parent_id=str(ObjectId()))
         self.check_list_success()
 
+    def test_list_with_invalid_parent_id(self):
+        """Test listing all catalogue categories with an invalid `parent_id` filter returning no results."""
+
+        self.mock_list([])
+        self.mock_utils.list_query.side_effect = InvalidObjectIdError("Invalid ID")
+        self.call_list(parent_id="invalid-id")
+        self.check_list_success(assert_find=False)
+
 
 class UpdateDSL(CatalogueCategoryRepoDSL):
     """Base class for `update` tests."""

test/unit/repositories/test_catalogue_item.py

@@ -267,14 +267,21 @@ def call_list(self, catalogue_category_id: Optional[str]) -> None:
             catalogue_category_id=catalogue_category_id, session=self.mock_session
         )
 
-    def check_list_success(self) -> None:
-        """Checks that a prior call to `call_list` worked as expected."""
+    def check_list_success(self, assert_find: bool = True) -> None:
+        """Checks that a prior call to `call_list` worked as expected.
 
-        expected_query = {}
-        if self._catalogue_category_id_filter:
-            expected_query["catalogue_category_id"] = CustomObjectId(self._catalogue_category_id_filter)
+        :param assert_find: If `True` it asserts whether a `find_one` call was made, else it asserts that no call was
+                            made.
+        """
 
-        self.catalogue_items_collection.find.assert_called_once_with(expected_query, session=self.mock_session)
+        if assert_find:
+            expected_query = {}
+            if self._catalogue_category_id_filter:
+                expected_query["catalogue_category_id"] = CustomObjectId(self._catalogue_category_id_filter)
+
+            self.catalogue_items_collection.find.assert_called_once_with(expected_query, session=self.mock_session)
+        else:
+            self.catalogue_items_collection.find.assert_not_called()
 
         assert self._obtained_catalogue_items_out == self._expected_catalogue_items_out
 
@@ -313,6 +320,13 @@ def test_list_with_catalogue_category_id_with_no_results(self):
         self.call_list(catalogue_category_id=str(ObjectId()))
         self.check_list_success()
 
+    def test_list_with_invalid_catalogue_category_id_with_no_results(self):
+        """Test listing all catalogue categories with an invalid `catalogue_category_id` filter returning no results."""
+
+        self.mock_list([])
+        self.call_list(catalogue_category_id="invalid-id")
+        self.check_list_success(assert_find=False)
+
 
 class UpdateDSL(CatalogueItemRepoDSL):
     """Base class for `update` tests."""

test/unit/repositories/test_item.py

@@ -275,16 +275,23 @@ def call_list(self, system_id: Optional[str], catalogue_item_id: Optional[str])
             system_id=system_id, catalogue_item_id=catalogue_item_id, session=self.mock_session
         )
 
-    def check_list_success(self) -> None:
-        """Checks that a prior call to `call_list` worked as expected."""
+    def check_list_success(self, assert_find: bool = True) -> None:
+        """Checks that a prior call to `call_list` worked as expected.
 
-        expected_query = {}
-        if self._system_id_filter:
-            expected_query["system_id"] = CustomObjectId(self._system_id_filter)
-        if self._catalogue_item_id_filter:
-            expected_query["catalogue_item_id"] = CustomObjectId(self._catalogue_item_id_filter)
+        :param assert_find: If `True` it asserts whether a `find_one` call was made, else it asserts that no call was
+                            made.
+        """
 
-        self.items_collection.find.assert_called_once_with(expected_query, session=self.mock_session)
+        if assert_find:
+            expected_query = {}
+            if self._system_id_filter:
+                expected_query["system_id"] = CustomObjectId(self._system_id_filter)
+            if self._catalogue_item_id_filter:
+                expected_query["catalogue_item_id"] = CustomObjectId(self._catalogue_item_id_filter)
+
+            self.items_collection.find.assert_called_once_with(expected_query, session=self.mock_session)
+        else:
+            self.items_collection.find.assert_not_called()
 
         assert self._obtained_items_out == self._expected_items_out
 
@@ -306,13 +313,27 @@ def test_list_with_system_id_filter(self):
         self.call_list(system_id=str(ObjectId()), catalogue_item_id=None)
         self.check_list_success()
 
+    def test_list_with_invalid_system_id_filter(self):
+        """Test listing all items when giving an invalid `system_id`."""
+
+        self.mock_list([])
+        self.call_list(system_id="invalid-id", catalogue_item_id=None)
+        self.check_list_success(assert_find=False)
+
     def test_list_with_catalogue_category_id_filter(self):
         """Test listing all items with a given `catalogue_category_id`."""
 
         self.mock_list([ITEM_IN_DATA_REQUIRED_VALUES_ONLY, ITEM_IN_DATA_ALL_VALUES_NO_PROPERTIES])
         self.call_list(system_id=None, catalogue_item_id=str(ObjectId()))
         self.check_list_success()
 
+    def test_list_with_invalid_catalogue_category_id_filter(self):
+        """Test listing all items when given an invalid `catalogue_category_id`."""
+
+        self.mock_list([])
+        self.call_list(system_id=None, catalogue_item_id="invalid-id")
+        self.check_list_success(assert_find=False)
+
     def test_list_with_system_id_and_catalogue_category_id_with_no_results(self):
         """Test listing all items with a `system_id` and `catalogue_category_id` filter returning no results."""
 

test/unit/repositories/test_manufacturer.py

@@ -476,6 +476,8 @@ def test_update_name_to_duplicate(self):
             MANUFACTURER_IN_DATA_A,
             duplicate_manufacturer_in_data={**MANUFACTURER_IN_DATA_A, "name": duplicate_name},
         )
+        self.call_update_expecting_error(manufacturer_id, DuplicateRecordError)
+        self.check_update_failed_with_exception("Duplicate manufacturer found")
 
     def test_update_with_invalid_id(self):
         """Test updating a manufacturer with an invalid ID."""

test/unit/repositories/test_system.py

@@ -4,6 +4,7 @@
 
 # Expect some duplicate code inside tests as the tests for the different entities can be very similar
 # pylint: disable=duplicate-code
+# pylint: disable=too-many-lines
 
 from test.mock_data import ITEM_DATA_REQUIRED_VALUES_ONLY, SYSTEM_IN_DATA_NO_PARENT_A, SYSTEM_IN_DATA_NO_PARENT_B
 from test.unit.repositories.conftest import RepositoryTestHelpers
@@ -445,13 +446,16 @@ def call_list(self, parent_id: Optional[str]):
 
         self._obtained_systems_out = self.system_repository.list(parent_id=parent_id, session=self.mock_session)
 
-    def check_list_success(self):
+    def check_list_success(self, assert_find: bool = True):
         """Checks that a prior call to `call_list` worked as expected."""
 
         self.mock_utils.list_query.assert_called_once_with(self._parent_id_filter, "systems")
-        self.systems_collection.find.assert_called_once_with(
-            self.mock_utils.list_query.return_value, session=self.mock_session
-        )
+        if assert_find:
+            self.systems_collection.find.assert_called_once_with(
+                self.mock_utils.list_query.return_value, session=self.mock_session
+            )
+        else:
+            self.systems_collection.find.assert_not_called()
 
         assert self._obtained_systems_out == self._expected_systems_out
 
@@ -487,6 +491,14 @@ def test_list_with_parent_id_with_no_results(self):
         self.call_list(parent_id=str(ObjectId()))
         self.check_list_success()
 
+    def test_list_with_invalid_parent_id(self):
+        """Test listing all systems with an invalid `parent_id` filter returning no results."""
+
+        self.mock_list([])
+        self.mock_utils.list_query.side_effect = InvalidObjectIdError("Invalid ID")
+        self.call_list(parent_id="invalid-id")
+        self.check_list_success(assert_find=False)
+
 
 class UpdateDSL(SystemRepoDSL):
     """Base class for `update` tests."""

test/unit/services/test_catalogue_item.py

@@ -231,7 +231,7 @@ def check_create_success(self) -> None:
         # This is the get for the obsolete replacement catalogue item
         if self._catalogue_item_post.obsolete_replacement_catalogue_item_id:
             self.mock_catalogue_item_repository.get.assert_called_once_with(
-                self._catalogue_item_post.obsolete_replacement_catalogue_item_id, entity_type_modifier="specified"
+                self._catalogue_item_post.obsolete_replacement_catalogue_item_id, entity_type_modifier="replacement"
             )
 
         self.wrapped_utils.process_properties.assert_called_once_with(
@@ -273,7 +273,7 @@ def test_create_with_all_properties(self):
         catalogue item."""
 
         self.mock_create(
-            CATALOGUE_ITEM_DATA_WITH_ALL_PROPERTIES,
+            {**CATALOGUE_ITEM_DATA_WITH_ALL_PROPERTIES, "obsolete_replacement_catalogue_item_id": str(ObjectId())},
             catalogue_category_in_data=BASE_CATALOGUE_CATEGORY_IN_DATA_WITH_PROPERTIES_MM,
             manufacturer_in_data=MANUFACTURER_IN_DATA_A,
         )

test/unit/services/test_system.py

@@ -14,7 +14,7 @@
 from bson import ObjectId
 
 from inventory_management_system_api.core.custom_object_id import CustomObjectId
-from inventory_management_system_api.core.exceptions import ChildElementsExistError
+from inventory_management_system_api.core.exceptions import ChildElementsExistError, InvalidObjectIdError
 from inventory_management_system_api.models.system import SystemIn, SystemOut
 from inventory_management_system_api.schemas.system import SystemPatchSchema, SystemPostSchema
 from inventory_management_system_api.services import utils
@@ -275,10 +275,13 @@ def mock_update(self, system_id: str, system_patch_data: dict, stored_system_pos
 
         # Construct the expected input for the repository
         merged_system_data = {**(stored_system_post_data or {}), **system_patch_data}
-        self._expected_system_in = SystemIn(
-            **merged_system_data,
-            code=utils.generate_code(merged_system_data["name"], "system"),
-        )
+        try:
+            self._expected_system_in = SystemIn(
+                **merged_system_data,
+                code=utils.generate_code(merged_system_data["name"], "system"),
+            )
+        except InvalidObjectIdError:
+            pass
 
     def call_update(self, system_id: str) -> None:
         """
@@ -349,6 +352,32 @@ def test_update_all_fields_except_parent_id(self):
         self.call_update(system_id)
         self.check_update_success()
 
+    def test_update_parent_id(self):
+        """Test updating the parent ID of a system."""
+
+        system_id = str(ObjectId())
+
+        self.mock_update(
+            system_id,
+            system_patch_data={"parent_id": str(ObjectId())},
+            stored_system_post_data=SYSTEM_POST_DATA_NO_PARENT_A,
+        )
+        self.call_update(system_id)
+        self.check_update_success()
+
+    def test_update_invalid_parent_id(self):
+        """Test updating the parent ID of a system to an invalid ID."""
+
+        system_id = str(ObjectId())
+
+        self.mock_update(
+            system_id,
+            system_patch_data={"parent_id": "invalid-id"},
+            stored_system_post_data=SYSTEM_POST_DATA_NO_PARENT_A,
+        )
+        self.call_update_expecting_error(system_id, InvalidObjectIdError)
+        self.check_update_failed_with_exception("Invalid ObjectId value 'invalid-id'")
+
     def test_update_description_only(self):
         """Test updating system's description field only (code should not need regenerating as name doesn't change)."""