From 4d5b4989aa92c6ccbb8988cd503101b06154eeef Mon Sep 17 00:00:00 2001 From: Joel Davies Date: Thu, 8 Aug 2024 14:56:38 +0000 Subject: [PATCH] Resolve TODO by moving some functions into a common base class #346 --- test/unit/services/conftest.py | 87 ++++++++++++++++++- test/unit/services/test_catalogue_category.py | 56 ++---------- test/unit/services/test_catalogue_item.py | 54 +----------- test/unit/services/test_item.py | 77 ++++------------ 4 files changed, 116 insertions(+), 158 deletions(-) diff --git a/test/unit/services/conftest.py b/test/unit/services/conftest.py index d42197ee..e7c59762 100644 --- a/test/unit/services/conftest.py +++ b/test/unit/services/conftest.py @@ -7,9 +7,10 @@ from unittest.mock import Mock, patch import pytest +from bson import ObjectId -from inventory_management_system_api.models.catalogue_category import CatalogueCategoryOut -from inventory_management_system_api.models.catalogue_item import CatalogueItemOut +from inventory_management_system_api.models.catalogue_category import CatalogueCategoryOut, CatalogueCategoryPropertyIn +from inventory_management_system_api.models.catalogue_item import CatalogueItemOut, PropertyIn from inventory_management_system_api.models.item import ItemOut from inventory_management_system_api.models.manufacturer import ManufacturerOut from inventory_management_system_api.models.system import SystemOut @@ -23,6 +24,8 @@ from inventory_management_system_api.repositories.unit import UnitRepo from inventory_management_system_api.repositories.usage_status import UsageStatusRepo from inventory_management_system_api.schemas.breadcrumbs import BreadcrumbsGetSchema +from inventory_management_system_api.schemas.catalogue_category import CatalogueCategoryPostPropertySchema +from inventory_management_system_api.schemas.catalogue_item import PropertyPostSchema from inventory_management_system_api.services.catalogue_category import CatalogueCategoryService from inventory_management_system_api.services.catalogue_category_property import CatalogueCategoryPropertyService from inventory_management_system_api.services.catalogue_item import CatalogueItemService @@ -321,6 +324,86 @@ def mock_update( repository_mock.update.return_value = repo_obj +class BaseCatalogueServiceDSL: + """Base class providing utilities to any catalogue related service tests.""" + + unit_value_id_dict: dict[str, str] + property_name_id_dict: dict[str, str] + + def construct_catalogue_category_properties_in_and_post_with_ids( + self, catalogue_category_properties_data: list[dict] + ) -> tuple[list[CatalogueCategoryPropertyIn], list[CatalogueCategoryPostPropertySchema]]: + """ + Returns a list of catalogue category property post schemas and expected property in models by adding + in unit IDs. It also assigns `unit_value_id_dict` for looking up these IDs. + + :param catalogue_category_properties_data: List of dictionaries containing the data for each property as would + be required for a `CatalogueCategoryPostPropertySchema` but without + any `unit_id`'s. + :returns: Tuple of lists. The first contains the expected `CatalogueCategoryPropertyIn` models and the second + the `CatalogueCategoryPostPropertySchema` schema's that should be posted in order to obtain them. + """ + + property_post_schemas = [] + expected_properties_in = [] + + self.unit_value_id_dict = {} + + for prop in catalogue_category_properties_data: + unit_id = None + prop_without_unit = prop.copy() + + # Give unit IDs and remove the unit value from the prop for the post schema + if "unit" in prop and prop["unit"]: + unit_id = str(ObjectId()) + self.unit_value_id_dict[prop["unit"]] = unit_id + del prop_without_unit["unit"] + + expected_properties_in.append(CatalogueCategoryPropertyIn(**prop, unit_id=unit_id)) + property_post_schemas.append(CatalogueCategoryPostPropertySchema(**prop_without_unit, unit_id=unit_id)) + + return expected_properties_in, property_post_schemas + + def construct_properties_in_and_post_with_ids( + self, + catalogue_category_properties_in: list[CatalogueCategoryPropertyIn], + properties_data: list[dict], + ) -> tuple[list[PropertyIn], list[PropertyPostSchema]]: + """ + Returns a list of property post schemas and expected property in models by adding + in unit IDs. It also assigns `unit_value_id_dict` for looking up these IDs. + + :param catalogue_category_properties_in: List of `CatalogueCategoryPropertyIn`'s as would be found in the + catalogue category. + :param properties_data: List of dictionaries containing the data for each property as would be required for a + `PropertyPostSchema` but without any `id`'s. + :returns: Tuple of lists. The first contains the expected `PropertyIn` models and the second the + `PropertyPostSchema` schema's that should be posted in order to obtain them. + """ + + property_post_schemas = [] + expected_properties_in = [] + + self.property_name_id_dict = {} + + for prop in properties_data: + prop_id = None + prop_without_name = prop.copy() + + # Find the corresponding catalogue category property with the same name + for found_prop in catalogue_category_properties_in: + if found_prop.name == prop["name"]: + prop_id = str(found_prop.id) + self.property_name_id_dict["name"] = prop_id + del prop_without_name["name"] + break + + expected_properties_in.append(PropertyIn(**prop, id=prop_id)) + property_post_schemas.append(PropertyPostSchema(**prop_without_name, id=prop_id)) + + return expected_properties_in, property_post_schemas + + # pylint:disable=fixme # TODO: Remove this once tests refactored - should be able to just use `ServiceTestHelpers.` @pytest.fixture(name="test_helpers") diff --git a/test/unit/services/test_catalogue_category.py b/test/unit/services/test_catalogue_category.py index 039db0ac..689ef7d7 100644 --- a/test/unit/services/test_catalogue_category.py +++ b/test/unit/services/test_catalogue_category.py @@ -16,7 +16,7 @@ CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT, UNIT_IN_DATA_MM, ) -from test.unit.services.conftest import ServiceTestHelpers +from test.unit.services.conftest import BaseCatalogueServiceDSL, ServiceTestHelpers from typing import Optional from unittest.mock import ANY, MagicMock, Mock, call, patch @@ -29,11 +29,7 @@ LeafCatalogueCategoryError, MissingRecordError, ) -from inventory_management_system_api.models.catalogue_category import ( - CatalogueCategoryIn, - CatalogueCategoryOut, - CatalogueCategoryPropertyIn, -) +from inventory_management_system_api.models.catalogue_category import CatalogueCategoryIn, CatalogueCategoryOut from inventory_management_system_api.models.unit import UnitIn, UnitOut from inventory_management_system_api.schemas.catalogue_category import ( CATALOGUE_CATEGORY_WITH_CHILD_NON_EDITABLE_FIELDS, @@ -45,7 +41,7 @@ from inventory_management_system_api.services.catalogue_category import CatalogueCategoryService -class CatalogueCategoryServiceDSL: +class CatalogueCategoryServiceDSL(BaseCatalogueServiceDSL): """Base class for `CatalogueCategoryService` unit tests.""" wrapped_utils: Mock @@ -53,8 +49,6 @@ class CatalogueCategoryServiceDSL: mock_unit_repository: Mock catalogue_category_service: CatalogueCategoryService - unit_value_id_dict: dict[str, str] - @pytest.fixture(autouse=True) def setup( self, @@ -75,40 +69,6 @@ def setup( self.wrapped_utils = wrapped_utils yield - def construct_properties_in_and_post_with_ids( - self, catalogue_category_properties_data: list[dict] - ) -> tuple[list[CatalogueCategoryPropertyIn], list[CatalogueCategoryPostPropertySchema]]: - """ - Returns a list of property post schemas and expected property in models by adding - in unit IDs. It also assigns `unit_value_id_dict` for looking up these IDs. - - :param catalogue_category_properties_data: List of dictionaries containing the data for each property as would - be required for a `CatalogueCategoryPostPropertySchema` but without - any `unit_id`'s. - :returns: Tuple of lists. The first contains the expected `CatalogueCategoryPropertyIn` models and the second - the `CatalogueCategoryPostPropertySchema` schema's that should be posted in order to obtain them. - """ - - property_post_schemas = [] - expected_properties_in = [] - - self.unit_value_id_dict = {} - - for prop in catalogue_category_properties_data: - unit_id = None - prop_without_unit = prop.copy() - - # Give unit IDs and remove the unit value from the prop for the post schema - if "unit" in prop and prop["unit"]: - unit_id = str(ObjectId()) - self.unit_value_id_dict[prop["unit"]] = unit_id - del prop_without_unit["unit"] - - expected_properties_in.append(CatalogueCategoryPropertyIn(**prop, unit_id=unit_id)) - property_post_schemas.append(CatalogueCategoryPostPropertySchema(**prop_without_unit, unit_id=unit_id)) - - return expected_properties_in, property_post_schemas - def mock_add_property_unit_values( self, units_in_data: list[Optional[dict]], unit_value_id_dict: dict[str, str] ) -> None: @@ -191,8 +151,8 @@ def mock_create( property_post_schemas = [] expected_properties_in = [] if "properties" in catalogue_category_data and catalogue_category_data["properties"]: - expected_properties_in, property_post_schemas = self.construct_properties_in_and_post_with_ids( - catalogue_category_data["properties"] + expected_properties_in, property_post_schemas = ( + self.construct_catalogue_category_properties_in_and_post_with_ids(catalogue_category_data["properties"]) ) self.mock_add_property_unit_values(units_in_data or [], self.unit_value_id_dict) @@ -542,8 +502,10 @@ def mock_update( # When properties are given need to mock any units and ensure the expected data inserts the unit IDs as well expected_properties_in = [] if "properties" in catalogue_category_update_data and catalogue_category_update_data["properties"]: - expected_properties_in, property_post_schemas = self.construct_properties_in_and_post_with_ids( - catalogue_category_update_data["properties"] + expected_properties_in, property_post_schemas = ( + self.construct_catalogue_category_properties_in_and_post_with_ids( + catalogue_category_update_data["properties"] + ) ) catalogue_category_update_data["properties"] = property_post_schemas diff --git a/test/unit/services/test_catalogue_item.py b/test/unit/services/test_catalogue_item.py index f26c94fe..b4530ca3 100644 --- a/test/unit/services/test_catalogue_item.py +++ b/test/unit/services/test_catalogue_item.py @@ -19,7 +19,7 @@ MANUFACTURER_IN_DATA_A, PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42, ) -from test.unit.services.conftest import ServiceTestHelpers +from test.unit.services.conftest import BaseCatalogueServiceDSL, ServiceTestHelpers from typing import Optional from unittest.mock import MagicMock, Mock, call, patch @@ -33,24 +33,19 @@ MissingRecordError, NonLeafCatalogueCategoryError, ) -from inventory_management_system_api.models.catalogue_category import ( - CatalogueCategoryIn, - CatalogueCategoryOut, - CatalogueCategoryPropertyIn, -) -from inventory_management_system_api.models.catalogue_item import CatalogueItemIn, CatalogueItemOut, PropertyIn +from inventory_management_system_api.models.catalogue_category import CatalogueCategoryIn, CatalogueCategoryOut +from inventory_management_system_api.models.catalogue_item import CatalogueItemIn, CatalogueItemOut from inventory_management_system_api.models.manufacturer import ManufacturerIn, ManufacturerOut from inventory_management_system_api.schemas.catalogue_item import ( CATALOGUE_ITEM_WITH_CHILD_NON_EDITABLE_FIELDS, CatalogueItemPatchSchema, CatalogueItemPostSchema, - PropertyPostSchema, ) from inventory_management_system_api.services import utils from inventory_management_system_api.services.catalogue_item import CatalogueItemService -class CatalogueItemServiceDSL: +class CatalogueItemServiceDSL(BaseCatalogueServiceDSL): """Base class for `CatalogueItemService` unit tests.""" wrapped_utils: Mock @@ -60,8 +55,6 @@ class CatalogueItemServiceDSL: mock_unit_repository: Mock catalogue_item_service: CatalogueItemService - property_name_id_dict: dict[str, str] - # pylint:disable=too-many-arguments @pytest.fixture(autouse=True) def setup( @@ -87,45 +80,6 @@ def setup( self.wrapped_utils = wrapped_utils yield - def construct_properties_in_and_post_with_ids( - self, - catalogue_category_properties_in: list[CatalogueCategoryPropertyIn], - properties_data: list[dict], - ) -> tuple[list[PropertyIn], list[PropertyPostSchema]]: - """ - Returns a list of property post schemas and expected property in models by adding - in unit IDs. It also assigns `unit_value_id_dict` for looking up these IDs. - - :param catalogue_category_properties_in: List of `CatalogueCategoryPropertyIn`'s as would be found in the - catalogue category. - :param properties_data: List of dictionaries containing the data for each property as would be required for a - `PropertyPostSchema` but without any `id`'s. - :returns: Tuple of lists. The first contains the expected `PropertyIn` models and the second the - `PropertyPostSchema` schema's that should be posted in order to obtain them. - """ - - property_post_schemas = [] - expected_properties_in = [] - - self.property_name_id_dict = {} - - for prop in properties_data: - prop_id = None - prop_without_name = prop.copy() - - # Find the corresponding catalogue category property with the same name - for found_prop in catalogue_category_properties_in: - if found_prop.name == prop["name"]: - prop_id = str(found_prop.id) - self.property_name_id_dict["name"] = prop_id - del prop_without_name["name"] - break - - expected_properties_in.append(PropertyIn(**prop, id=prop_id)) - property_post_schemas.append(PropertyPostSchema(**prop_without_name, id=prop_id)) - - return expected_properties_in, property_post_schemas - class CreateDSL(CatalogueItemServiceDSL): """Base class for `create` tests.""" diff --git a/test/unit/services/test_item.py b/test/unit/services/test_item.py index f915acbf..24f834ea 100644 --- a/test/unit/services/test_item.py +++ b/test/unit/services/test_item.py @@ -11,10 +11,14 @@ BASE_CATALOGUE_ITEM_DATA_WITH_PROPERTIES, CATALOGUE_CATEGORY_IN_DATA_LEAF_NO_PARENT_NO_PROPERTIES, CATALOGUE_ITEM_DATA_REQUIRED_VALUES_ONLY, - ITEM_DATA_ALL_VALUES_NO_PROPERTIES, ITEM_DATA_REQUIRED_VALUES_ONLY, - ITEM_DATA_WITH_ALL_PROPERTIES, ITEM_DATA_WITH_MANDATORY_PROPERTIES_ONLY, - SYSTEM_IN_DATA_NO_PARENT_A, USAGE_STATUS_IN_DATA_IN_USE) -from test.unit.services.conftest import ServiceTestHelpers + ITEM_DATA_ALL_VALUES_NO_PROPERTIES, + ITEM_DATA_REQUIRED_VALUES_ONLY, + ITEM_DATA_WITH_ALL_PROPERTIES, + ITEM_DATA_WITH_MANDATORY_PROPERTIES_ONLY, + SYSTEM_IN_DATA_NO_PARENT_A, + USAGE_STATUS_IN_DATA_IN_USE, +) +from test.unit.services.conftest import BaseCatalogueServiceDSL, ServiceTestHelpers from typing import List, Optional from unittest.mock import MagicMock, Mock, patch @@ -22,24 +26,22 @@ from bson import ObjectId from inventory_management_system_api.core.exceptions import ( - DatabaseIntegrityError, InvalidActionError, MissingRecordError) -from inventory_management_system_api.models.catalogue_category import ( - CatalogueCategoryIn, CatalogueCategoryOut, CatalogueCategoryPropertyIn) -from inventory_management_system_api.models.catalogue_item import ( - CatalogueItemIn, CatalogueItemOut, PropertyIn) + DatabaseIntegrityError, + InvalidActionError, + MissingRecordError, +) +from inventory_management_system_api.models.catalogue_category import CatalogueCategoryIn, CatalogueCategoryOut +from inventory_management_system_api.models.catalogue_item import CatalogueItemIn, CatalogueItemOut from inventory_management_system_api.models.item import ItemIn, ItemOut from inventory_management_system_api.models.system import SystemIn, SystemOut -from inventory_management_system_api.models.usage_status import ( - UsageStatusIn, UsageStatusOut) -from inventory_management_system_api.schemas.catalogue_item import \ - PropertyPostSchema -from inventory_management_system_api.schemas.item import (ItemPatchSchema, - ItemPostSchema) +from inventory_management_system_api.models.usage_status import UsageStatusIn, UsageStatusOut +from inventory_management_system_api.schemas.catalogue_item import PropertyPostSchema +from inventory_management_system_api.schemas.item import ItemPatchSchema, ItemPostSchema from inventory_management_system_api.services import utils from inventory_management_system_api.services.item import ItemService -class ItemServiceDSL: +class ItemServiceDSL(BaseCatalogueServiceDSL): """Base class for `ItemService` unit tests.""" # pylint:disable=too-many-instance-attributes @@ -51,8 +53,6 @@ class ItemServiceDSL: mock_usage_status_repository: Mock item_service: ItemService - property_name_id_dict: dict[str, str] - # pylint:disable=too-many-arguments @pytest.fixture(autouse=True) def setup( @@ -80,47 +80,6 @@ def setup( self.wrapped_utils = wrapped_utils yield - # pylint:disable=fixme - # TODO: Put this in a common place - its identical as the one catalogue items, and similar to catalogue categories - def construct_properties_in_and_post_with_ids( - self, - catalogue_category_properties_in: list[CatalogueCategoryPropertyIn], - properties_data: list[dict], - ) -> tuple[list[PropertyIn], list[PropertyPostSchema]]: - """ - Returns a list of property post schemas and expected property in models by adding - in unit IDs. It also assigns `unit_value_id_dict` for looking up these IDs. - - :param catalogue_category_properties_in: List of `CatalogueCategoryPropertyIn`'s as would be found in the - catalogue category. - :param properties_data: List of dictionaries containing the data for each property as would be required for a - `PropertyPostSchema` but without any `id`'s. - :returns: Tuple of lists. The first contains the expected `PropertyIn` models and the second the - `PropertyPostSchema` schema's that should be posted in order to obtain them. - """ - - property_post_schemas = [] - expected_properties_in = [] - - self.property_name_id_dict = {} - - for prop in properties_data: - prop_id = None - prop_without_name = prop.copy() - - # Find the corresponding catalogue category property with the same name - for found_prop in catalogue_category_properties_in: - if found_prop.name == prop["name"]: - prop_id = str(found_prop.id) - self.property_name_id_dict["name"] = prop_id - del prop_without_name["name"] - break - - expected_properties_in.append(PropertyIn(**prop, id=prop_id)) - property_post_schemas.append(PropertyPostSchema(**prop_without_name, id=prop_id)) - - return expected_properties_in, property_post_schemas - class CreateDSL(ItemServiceDSL): """Base class for `create` tests."""