diff --git a/inventory_management_system_api/repositories/manufacturer.py b/inventory_management_system_api/repositories/manufacturer.py index cfb1a27c..2555f41f 100644 --- a/inventory_management_system_api/repositories/manufacturer.py +++ b/inventory_management_system_api/repositories/manufacturer.py @@ -21,9 +21,7 @@ class ManufacturerRepo: - """ - Repository for managing manufacturers in a MongoDb database. - """ + """Repository for managing manufacturers in a MongoDb database.""" def __init__(self, database: DatabaseDep) -> None: """ @@ -89,8 +87,8 @@ def update( :param manufacturer_id: The ID of the manufacturer to update. :param manufacturer: The manufacturer containing the update data. :param session: PyMongo ClientSession to use for database operations. - :raises: DuplicateRecordError: if a duplicate manufacturer is found. - :return: the updated manufacturer. + :raises DuplicateRecordError: If a duplicate manufacturer is found. + :return: The updated manufacturer. """ manufacturer_id = CustomObjectId(manufacturer_id) @@ -111,13 +109,13 @@ def delete(self, manufacturer_id: str, session: ClientSession = None) -> None: """ Delete a manufacturer by its ID from a MongoDB database. - The method Checks if the manufacturer is part of a catalogue item, and raises a `PartOfCatalogueItemError` if it + The method checks if the manufacturer is part of a catalogue item, and raises a `PartOfCatalogueItemError` if it is. :param manufacturer_id: The ID of the manufacturer to delete. :param session: PyMongo ClientSession to use for database operations. - :raises PartOfCatalogueItemError: if the manufacturer is part of a catalogue item. - :raises MissingRecordError: if the manufacturer doesn't exist. + :raises PartOfCatalogueItemError: If the manufacturer is part of a catalogue item. + :raises MissingRecordError: If the manufacturer doesn't exist. """ manufacturer_id = CustomObjectId(manufacturer_id) if self._is_manufacturer_in_catalogue_item(str(manufacturer_id), session=session): @@ -137,7 +135,7 @@ def _is_duplicate_manufacturer( :param code: The code of the manufacturer to check for duplicates. :param manufacturer_id: The ID of the manufacturer to check if the duplicate manufacturer found is itself. :param session: PyMongo ClientSession to use for database operations. - :return `True` if a duplicate manufacturer is found, `False` otherwise. + :return: `True` if a duplicate manufacturer is found, `False` otherwise. """ logger.info("Checking if manufacturer with code '%s' already exists", code) manufacturer = self._manufacturers_collection.find_one( @@ -151,7 +149,7 @@ def _is_manufacturer_in_catalogue_item(self, manufacturer_id: str, session: Clie :param manufacturer_id: The ID of the manufacturer to check. :param session: PyMongo ClientSession to use for database operations. - :return: Returns `True` if the manufacturer is part of a catalogue item, `False` otherwise. + :return: `True` if the manufacturer is part of a catalogue item, `False` otherwise. """ manufacturer_id = CustomObjectId(manufacturer_id) return ( diff --git a/inventory_management_system_api/services/manufacturer.py b/inventory_management_system_api/services/manufacturer.py index c778272d..4b1be163 100644 --- a/inventory_management_system_api/services/manufacturer.py +++ b/inventory_management_system_api/services/manufacturer.py @@ -19,9 +19,7 @@ class ManufacturerService: - """ - Service for managing manufacturers. - """ + """Service for managing manufacturers.""" def __init__( self, diff --git a/test/e2e/test_manufacturer.py b/test/e2e/test_manufacturer.py index 1dccb612..a306bc77 100644 --- a/test/e2e/test_manufacturer.py +++ b/test/e2e/test_manufacturer.py @@ -1,6 +1,4 @@ -""" -End-to-End tests for the manufacturer router. -""" +"""End-to-End tests for the manufacturer router.""" # Expect some duplicate code inside tests as the tests for the different entities can be very similar # pylint: disable=duplicate-code @@ -36,7 +34,8 @@ def post_manufacturer(self, manufacturer_post_data: dict) -> Optional[str]: """ Posts a manufacturer with the given data, returns the ID of the created manufacturer if successful. - :param manufacturer_post_data: Dictionary containing the manufacturer data that should be posted. + :param manufacturer_post_data: Dictionary containing the manufacturer data as would be required for a + `ManufacturerPostSchema`. :return: ID of the created manufacturer (or `None` if not successful). """ self._post_response = self.test_client.post("/v1/manufacturers", json=manufacturer_post_data) @@ -46,8 +45,8 @@ def check_post_manufacturer_success(self, expected_manufacturer_get_data: dict) """ Checks that a prior call to `post_manufacturer` gave a successful response with the expected data returned. - :param expected_manufacturer_get_data: Dictionary containing the expected manufacturer data that should be - returned. + :param expected_manufacturer_get_data: Dictionary containing the expected manufacturer data as would be required + for a `ManufacturerSchema`. """ assert self._post_response.status_code == 201 assert self._post_response.json() == expected_manufacturer_get_data @@ -77,7 +76,7 @@ def check_post_manufacturer_failed_with_validation_message(self, status_code: in class TestCreate(CreateDSL): """Tests for creating a manufacturer.""" - def test_create_with_only_require_values_provided(self): + def test_create_with_only_required_values_provided(self): """Test creating a manufacturer with only required values provided.""" self.post_manufacturer(MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY) self.check_post_manufacturer_success(MANUFACTURER_GET_DATA_REQUIRED_VALUES_ONLY) @@ -111,8 +110,8 @@ def check_get_manufacturer_success(self, expected_manufacturer_get_data: dict) - """ Checks that a prior call to `get_manufacturer` gave a successful response with the expected data returned. - :param expected_manufacturer_get_data: Dictionary containing the expected manufacturer data that should be - returned. + :param expected_manufacturer_get_data: Dictionary containing the expected manufacturer data as would be required + for a `ManufacturerSchema`. """ assert self._get_response.status_code == 200 assert self._get_response.json() == expected_manufacturer_get_data @@ -159,8 +158,8 @@ def check_get_manufacturers_success(self, expected_manufacturers_get_data: list[ """ Checks that a prior call to `get_manufacturers` gave a successful response with the expected data returned. - :param expected_manufacturers_get_data: List of dictionaries containing the expected manufacturer data that - should be returned. + :param expected_manufacturers_get_data: List of dictionaries containing the expected manufacturer data as would + be required for a `ManufacturerSchema`. """ assert self._get_response.status_code == 200 assert self._get_response.json() == expected_manufacturers_get_data @@ -194,7 +193,8 @@ def patch_manufacturer(self, manufacturer_id: str, manufacturer_patch_data: dict Updates a manufacturer with the given ID. :param manufacturer_id: ID of the manufacturer to be updated. - :param manufacturer_patch_data: Dictionary containing the manufacturer patch data. + :param manufacturer_patch_data: Dictionary containing the manufacturer patch data as would be required for a + `ManufacturerPatchSchema`. """ self._patch_response = self.test_client.patch( f"/v1/manufacturers/{manufacturer_id}", json=manufacturer_patch_data @@ -204,8 +204,8 @@ def check_patch_manufacturer_success(self, expected_manufacturer_get_data: dict) """ Checks that a prior call to `patch_manufacturer` gave a successful response with the expected data returned. - :param expected_manufacturer_get_data: Dictionaries containing the expected manufacturer data that should be - returned. + :param expected_manufacturer_get_data: Dictionaries containing the expected manufacturer data as would be + required for a `ManufacturerSchema`. """ assert self._patch_response.status_code == 200 assert self._patch_response.json() == expected_manufacturer_get_data @@ -232,7 +232,6 @@ def test_partial_update_all_fields(self): def test_partial_update_name_to_duplicate(self): """Test updating the name of a manufacturer to conflict with a pre-existing one.""" - self.post_manufacturer(MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY) system_id = self.post_manufacturer(MANUFACTURER_POST_DATA_ALL_VALUES) self.patch_manufacturer(system_id, {"name": MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY["name"]}) @@ -251,7 +250,6 @@ def test_partial_update_name_capitalisation(self): def test_partial_update_with_non_existent_id(self): """Test updating a non-existent manufacturer.""" - self.patch_manufacturer(str(ObjectId()), {}) self.check_patch_manufacturer_failed_with_detail(404, "Manufacturer not found") @@ -280,7 +278,7 @@ def check_delete_manufacturer_success(self) -> None: def check_delete_manufacturer_failed_with_detail(self, status_code: int, detail: str) -> None: """ - Checks that a prior call to 'delete_manufacturer' gave a failed response with the expected code and detail. + Checks that a prior call to `delete_manufacturer` gave a failed response with the expected code and detail. :param status_code: Expected status code to be returned. :param detail: Expected detail to be returned. diff --git a/test/mock_data.py b/test/mock_data.py index d447029e..692c6893 100644 --- a/test/mock_data.py +++ b/test/mock_data.py @@ -18,41 +18,7 @@ # --------------------------------- MANUFACTURERS --------------------------------- -MANUFACTURER_POST_DATA_A = { - "name": "Manufacturer A", - "url": "http://testurl.co.uk/", - "address": { - "address_line": "1 Example Street", - "town": "Oxford", - "county": "Oxfordshire", - "country": "United Kingdom", - "postcode": "OX1 2AB", - }, - "telephone": "0932348348", -} - -MANUFACTURER_POST_DATA_B = { - "name": "Manufacturer B", - "url": "http://example.co.uk/", - "address": { - "address_line": "2 Example Street", - "town": "Oxford", - "county": "Oxfordshire", - "country": "United Kingdom", - "postcode": "OX1 2AB", - }, - "telephone": "073434394", -} - -MANUFACTURER_IN_DATA_A = { - **MANUFACTURER_POST_DATA_A, - "code": "manufacturer-a", -} - -MANUFACTURER_IN_DATA_B = { - **MANUFACTURER_POST_DATA_B, - "code": "manufacturer-b", -} +# Required values only MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY = { "name": "Manufacturer Test Required Values Only", @@ -73,9 +39,14 @@ "telephone": None, } +# All values + MANUFACTURER_POST_DATA_ALL_VALUES = { - **MANUFACTURER_POST_DATA_A, + **MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY, + "address": {**MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY["address"], "town": "Oxford", "county": "Oxfordshire"}, "name": "Manufacturer Test All Values", + "url": "http://testurl.co.uk/", + "telephone": "0932348348", } MANUFACTURER_GET_DATA_ALL_VALUES = { @@ -85,6 +56,29 @@ "code": "manufacturer-test-all-values", } +MANUFACTURER_POST_DATA_A = { + **MANUFACTURER_POST_DATA_ALL_VALUES, + "name": "Manufacturer A", +} + +MANUFACTURER_IN_DATA_A = { + **MANUFACTURER_POST_DATA_A, + "code": "manufacturer-a", +} + +MANUFACTURER_POST_DATA_B = { + **MANUFACTURER_POST_DATA_ALL_VALUES, + "address": {**MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY["address"], "address_line": "2 Example Street"}, + "name": "Manufacturer B", + "url": "http://example.co.uk/", + "telephone": "073434394", +} + +MANUFACTURER_IN_DATA_B = { + **MANUFACTURER_POST_DATA_B, + "code": "manufacturer-b", +} + # --------------------------------- SYSTEMS --------------------------------- SYSTEM_POST_DATA_NO_PARENT_A = { diff --git a/test/unit/repositories/test_manufacturer.py b/test/unit/repositories/test_manufacturer.py index 5dafd2f9..58092e67 100644 --- a/test/unit/repositories/test_manufacturer.py +++ b/test/unit/repositories/test_manufacturer.py @@ -1,6 +1,4 @@ -""" -Unit tests for the `ManufacturerRepo` repository. -""" +"""Unit tests for the `ManufacturerRepo` repository.""" # Expect some duplicate code inside tests as the tests for the different entities can be very similar # pylint: disable=duplicate-code @@ -26,9 +24,7 @@ class ManufacturerRepoDSL: - """ - Base class for `ManufacturerRepo` unit tests. - """ + """Base class for `ManufacturerRepo` unit tests.""" mock_database: Mock manufacturer_repository: ManufacturerRepo @@ -50,7 +46,7 @@ def setup(self, database_mock): def mock_is_duplicate_manufacturer(self, duplicate_manufacturer_in_data: Optional[dict] = None) -> None: """ - Mocks database methods appropriately for when the '_is_duplicate_manufacturer' repo method will be called. + Mocks database methods appropriately for when the `_is_duplicate_manufacturer` repo method will be called. :param duplicate_manufacturer_in_data: Either `None` or a dictionary containing manufacturer data for a duplicate manufacturer. @@ -71,16 +67,14 @@ def get_is_duplicate_manufacturer_expected_find_one_call( Returns the expected `find_one` calls that should occur when `_is_duplicate_manufacturer` is called. :param manufacturer_in: `ManufacturerIn` model containing the data about the manufacturer. - :param expected_manufacturer_id: Expected `manufacturer_id` provided to `_is_duplicate_manufacturer` + :param expected_manufacturer_id: Expected `manufacturer_id` provided to `_is_duplicate_manufacturer`. :return: Expected `find_one` calls. """ return call({"code": manufacturer_in.code, "_id": {"$ne": expected_manufacturer_id}}, session=self.mock_session) class CreateDSL(ManufacturerRepoDSL): - """ - Base class for create tests. - """ + """Base class for `create` tests.""" _manufacturer_in: ManufacturerIn _expected_manufacturer_out: ManufacturerOut @@ -92,7 +86,7 @@ def mock_create(self, manufacturer_in_data: dict, duplicate_manufacturer_in_data Mocks database methods appropriately to test the `create` repo method. :param manufacturer_in_data: Dictionary containing the manufacturer data as would be required for a - `ManufacturerIn` database model (i.e. no id or created and modified times required). + `ManufacturerIn` database model (i.e. no ID or created and modified times required). :param duplicate_manufacturer_in_data: Either `None` or a dictionary containing manufacturer data for a duplicate manufacturer. """ @@ -106,17 +100,15 @@ def mock_create(self, manufacturer_in_data: dict, duplicate_manufacturer_in_data ) # self.mock_is_duplicate_manufacturer(duplicate_manufacturer_in_data) - # Mock 'insert one' to return object for inserted manufacturer + # Mock `insert one` to return object for inserted manufacturer RepositoryTestHelpers.mock_insert_one(self.manufacturers_collection, inserted_manufacturer_id) - # Mock 'find_one' to return the inserted manufacturer document + # Mock `find_one` to return the inserted manufacturer document RepositoryTestHelpers.mock_find_one( self.manufacturers_collection, {**self._manufacturer_in.model_dump(), "_id": inserted_manufacturer_id} ) def call_create(self) -> None: - """ - Calls the `ManufacturerRepo` `create` method with the appropriate data from a prior call to `mock_create`. - """ + """Calls the `ManufacturerRepo` `create` method with the appropriate data from a prior call to `mock_create`.""" self._created_manufacturer = self.manufacturer_repository.create( self._manufacturer_in, session=self.mock_session ) @@ -133,9 +125,7 @@ def call_create_expecting_error(self, error_type: type[BaseException]) -> None: self._create_exception = exc def check_create_success(self) -> None: - """ - Checks that a prior call to `call_create` worked as expected. - """ + """Checks that a prior call to `call_create` worked as expected.""" manufacturer_in_data = self._manufacturer_in.model_dump() # Obtain a list of expected find_one calls @@ -157,38 +147,30 @@ def check_create_failed_with_exception(self, message: str) -> None: Checks that a prior call to `call_create_expecting_error` worked as expected, raising an exception with the correct message. - :param message: Message of the raised exception. + :param message: Expected message of the raised exception. """ self.manufacturers_collection.insert_one.assert_not_called() assert str(self._create_exception.value) == message class TestCreate(CreateDSL): - """ - Tests for creating a manufacturer. - """ + """Tests for creating a manufacturer.""" def test_create(self): - """ - Test creating a system. - """ + """Test creating a manufacturer.""" self.mock_create(MANUFACTURER_IN_DATA_A) self.call_create() self.check_create_success() def test_create_with_duplicate_name(self): - """ - Test creating a manufacturer with a duplicate manufacturer being found. - """ + """Test creating a manufacturer with a duplicate manufacturer being found.""" self.mock_create(MANUFACTURER_IN_DATA_A, duplicate_manufacturer_in_data=MANUFACTURER_IN_DATA_A) self.call_create_expecting_error(DuplicateRecordError) self.check_create_failed_with_exception("Duplicate manufacturer found") class GetDSL(ManufacturerRepoDSL): - """ - Base class for get tests. - """ + """Base class for `get` tests.""" _obtained_manufacturer_id: str _expected_manufacturer_out: Optional[ManufacturerOut] @@ -199,7 +181,7 @@ def mock_get(self, manufacturer_id: str, manufacturer_in_data: Optional[dict]) - """ Mocks database methods appropriately to test the `get` repo method. - :param manufacturer_id: ID of the manufacturer that will be obtained. + :param manufacturer_id: ID of the manufacturer to be obtained. :param manufacturer_in_data: Either `None` or a dictionary containing the manufacturer data as would be required for a `ManufacturerIn` database model (i.e. no ID or created and modified times required). """ @@ -236,9 +218,7 @@ def call_get_expecting_error(self, manufacturer_id: str, error_type: type[BaseEx self._get_exception = exc def check_get_success(self) -> None: - """ - Checks that a prior call to `call_get` worked as expected. - """ + """Checks that a prior call to `call_get` worked as expected.""" self.manufacturers_collection.find_one.assert_called_once_with( {"_id": CustomObjectId(self._obtained_manufacturer_id)}, session=self.mock_session ) @@ -249,21 +229,17 @@ def check_get_failed_with_exception(self, message: str) -> None: Checks that a prior call to `call_get_expecting_error` worked as expected, raising an exception with the correct message. - :param message: Message of the raised exception. + :param message: Expected message of the raised exception. """ self.manufacturers_collection.find_one.assert_not_called() assert str(self._get_exception.value) == message class TestGet(GetDSL): - """ - Tests for getting a manufacturer. - """ + """Tests for getting a manufacturer.""" def test_get(self): - """ - Test getting a system. - """ + """Test getting a system.""" manufacturer_id = str(ObjectId()) self.mock_get(manufacturer_id, MANUFACTURER_IN_DATA_A) @@ -271,9 +247,7 @@ def test_get(self): self.check_get_success() def test_get_with_non_existent_id(self): - """ - Testing getting a manufacturer with a non-existent ID. - """ + """Testing getting a manufacturer with a non-existent ID.""" manufacturer_id = str(ObjectId()) self.mock_get(manufacturer_id, None) @@ -281,9 +255,7 @@ def test_get_with_non_existent_id(self): self.check_get_success() def test_get_with_invalid_id(self): - """ - Test getting a manufacturer with an invalid ID. - """ + """Test getting a manufacturer with an invalid ID.""" manufacturer_id = "invalid-id" self.call_get_expecting_error(manufacturer_id, InvalidObjectIdError) @@ -291,9 +263,7 @@ def test_get_with_invalid_id(self): class ListDSL(ManufacturerRepoDSL): - """ - Base class for list tests. - """ + """Base class for `list` tests.""" _expected_manufacturers_out: list[ManufacturerOut] _obtained_manufacturers_out: list[ManufacturerOut] @@ -316,45 +286,33 @@ def mock_list(self, manufacturers_in_data: list[dict]) -> None: ) def call_list(self) -> None: - """ - Calls the `ManufacturerRepo` `list method` method. - """ + """Calls the `ManufacturerRepo` `list method` method.""" self._obtained_manufacturers_out = self.manufacturer_repository.list(session=self.mock_session) def check_list_success(self) -> None: - """ - Checks that a prior call to `call_list` worked as expected. - """ + """Checks that a prior call to `call_list` worked as expected.""" self.manufacturers_collection.find.assert_called_once_with(session=self.mock_session) assert self._obtained_manufacturers_out == self._expected_manufacturers_out class TestList(ListDSL): - """ - Tests for listing manufacturers. - """ + """Tests for listing manufacturers.""" def test_list(self): - """ - Test listing all manufacturers. - """ + """Test listing all manufacturers.""" self.mock_list([MANUFACTURER_IN_DATA_A, MANUFACTURER_IN_DATA_B]) self.call_list() self.check_list_success() def test_list_with_no_results(self): - """ - Test listing all manufacturers returning no results. - """ + """Test listing all manufacturers returning no results.""" self.mock_list([]) self.call_list() self.check_list_success() class UpdateDSL(ManufacturerRepoDSL): - """ - Base class for update tests. - """ + """Base class for `update` tests.""" _manufacturer_in: ManufacturerIn _stored_manufacturer_out: Optional[ManufacturerOut] @@ -366,6 +324,7 @@ class UpdateDSL(ManufacturerRepoDSL): def set_update_data(self, new_manufacturer_in_data: dict) -> None: """ Assigns the update data to use during a call to `call_update`. + :param new_manufacturer_in_data: New manufacturer data as would be required for a `ManufacturerIn` database model to supply to the `SystemRepo` `update` method. """ @@ -430,18 +389,16 @@ def call_update_expecting_error(self, manufacturer_id: str, error_type: type[Bas """ Calls the `ManufacturerRepo` `update` method with the appropriate data from a prior call to `mock_update` (or `set_update_data`) while expecting an error to be raised. + :param manufacturer_id: ID of the manufacturer to be updated. :param error_type: Expected exception to be raised. """ - with pytest.raises(error_type) as exc: self.manufacturer_repository.update(manufacturer_id, self._manufacturer_in) self._update_exception = exc def check_update_success(self) -> None: - """ - Checks that a prior call to `call_update` worked as expected. - """ + """Checks that a prior call to `call_update` worked as expected.""" # Obtain a list of expected `find_one` calls expected_find_one_calls = [ # Stored manufacturer @@ -472,21 +429,17 @@ def check_update_failed_with_exception(self, message: str) -> None: Checks that a prior call to `call_update_expecting_error` worked as expected, raising an exception with the correct message. - :param message: Message of the raised exception. + :param message: Expected message of the raised exception. """ self.manufacturers_collection.update_one.assert_not_called() assert str(self._update_exception.value) == message class TestUpdate(UpdateDSL): - """ - Tests for updating a manufacturer. - """ + """Tests for updating a manufacturer.""" def test_update(self): - """ - Test updating a manufacturer. - """ + """Test updating a manufacturer.""" manufacturer_id = str(ObjectId()) self.mock_update(manufacturer_id, MANUFACTURER_IN_DATA_A, MANUFACTURER_IN_DATA_B) @@ -494,9 +447,7 @@ def test_update(self): self.check_update_success() def test_update_name_capitalisation(self): - """ - Test updating the name capitalisation of a manufacturer. - """ + """Test updating the name capitalisation of a manufacturer.""" manufacturer_id = str(ObjectId()) new_name = "manufacturer a" @@ -505,9 +456,7 @@ def test_update_name_capitalisation(self): self.check_update_success() def test_update_name_to_duplicate(self): - """ - Test updating the name of a manufacturer to one that is a duplicate. - """ + """Test updating the name of a manufacturer to one that is a duplicate.""" manufacturer_id = str(ObjectId()) duplicate_name = "Duplicate name" @@ -519,9 +468,7 @@ def test_update_name_to_duplicate(self): ) def test_update_with_invalid_id(self): - """ - Test updating a manufacturer with an invalid ID. - """ + """Test updating a manufacturer with an invalid ID.""" manufacturer_id = "invalid-id" self.set_update_data(MANUFACTURER_IN_DATA_A) @@ -530,21 +477,18 @@ def test_update_with_invalid_id(self): class DeleteDSL(ManufacturerRepoDSL): - """ - Base class for delete tests. - """ + """Base class for `delete` tests.""" _delete_manufacturer_id: str _delete_exception: pytest.ExceptionInfo _mock_catalogue_item_data: Optional[dict] - def mock_delete(self, deleted_count: int, catalogue_item_data: Optional[dict] = None): + def mock_delete(self, deleted_count: int, catalogue_item_data: Optional[dict] = None) -> None: """ Mocks database methods appropriately to test the `delete` repo method. :param deleted_count: Number of documents deleted successfully. :param catalogue_item_data: Dictionary containing a catalogue item's data (or `None`). - :return: """ self.mock_is_manufacturer_in_catalogue_item(catalogue_item_data) RepositoryTestHelpers.mock_delete_one(self.manufacturers_collection, deleted_count) @@ -563,7 +507,7 @@ def call_delete_expecting_error(self, manufacturer_id: str, error_type: type[Bas Calls the `ManufacturerRepo` `delete` method with the appropriate data from a prior call to `mock_delete` while expecting an error to be raised. - :param manufacturer_id: ID of the manufacturer to be obtained. + :param manufacturer_id: ID of the manufacturer to be deleted. :param error_type: Expected exception to be raised. """ self._delete_manufacturer_id = manufacturer_id @@ -572,9 +516,7 @@ def call_delete_expecting_error(self, manufacturer_id: str, error_type: type[Bas self._delete_exception = exc def check_delete_success(self) -> None: - """ - Checks that a prior call to `call_delete` worked as expected. - """ + """Checks that a prior call to `call_delete` worked as expected.""" self.check_is_manufacturer_in_catalogue_item_performed_expected_calls(self._delete_manufacturer_id) self.manufacturers_collection.delete_one.assert_called_once_with( {"_id": CustomObjectId(self._delete_manufacturer_id)}, session=self.mock_session @@ -585,10 +527,9 @@ def check_delete_failed_with_exception(self, message: str, expecting_delete_one_ Checks that a prior call to `call_delete_expecting_error` worked as expected, raising an exception with the correct message. - :param message: Message of the raised exception. + :param message: Expected message of the raised exception. :param expecting_delete_one_called: Whether the `delete_one` method is expected to be called or not. """ - if not expecting_delete_one_called: self.manufacturers_collection.delete_one.assert_not_called() else: @@ -600,10 +541,10 @@ def check_delete_failed_with_exception(self, message: str, expecting_delete_one_ def mock_is_manufacturer_in_catalogue_item(self, catalogue_item_data: Optional[dict] = None) -> None: """ - Mocks database methods appropriately for when the '_is_manufacturer_in_catalogue_item' repo method will be + Mocks database methods appropriately for when the `_is_manufacturer_in_catalogue_item` repo method will be called. - :param catalogue_item_data: Dictionary containing a catalogue item's data (or `None`) + :param catalogue_item_data: Dictionary containing a catalogue item's data (or `None`). """ self._mock_catalogue_item_data = catalogue_item_data RepositoryTestHelpers.mock_find_one(self.catalogue_items_collection, catalogue_item_data) @@ -613,29 +554,22 @@ def check_is_manufacturer_in_catalogue_item_performed_expected_calls(self, expec :param expected_manufacturer_id: Expected manufacturer ID used in the database calls. """ - self.catalogue_items_collection.find_one.assert_called_once_with( {"manufacturer_id": CustomObjectId(expected_manufacturer_id)}, session=self.mock_session ) class TestDelete(DeleteDSL): - """ - Tests for deleting a manufacturer. - """ + """Tests for deleting a manufacturer.""" def test_delete(self): - """ - Test deleting a manufacturer. - """ + """Test deleting a manufacturer.""" self.mock_delete(deleted_count=1) self.call_delete(str(ObjectId())) self.check_delete_success() def test_delete_when_part_of_catalogue_item(self): - """ - Test deleting a manufacturer when it is part of a catalogue item. - """ + """Test deleting a manufacturer when it is part of a catalogue item.""" manufacturer_id = str(ObjectId()) # pylint:disable=fixme @@ -652,9 +586,7 @@ def test_delete_when_part_of_catalogue_item(self): self.check_delete_failed_with_exception(f"Manufacturer with ID '{manufacturer_id}' is part of a catalogue item") def test_delete_non_existent_id(self): - """ - Test deleting a manufacturer with a non-existent ID. - """ + """Test deleting a manufacturer with a non-existent ID.""" manufacturer_id = str(ObjectId()) self.mock_delete(deleted_count=0) @@ -664,9 +596,7 @@ def test_delete_non_existent_id(self): ) def test_delete_with_invalid_id(self): - """ - Test deleting a manufacturer with an invalid ID. - """ + """Test deleting a manufacturer with an invalid ID.""" manufacturer_id = "invalid-id" self.call_delete_expecting_error(manufacturer_id, InvalidObjectIdError) diff --git a/test/unit/services/test_manufacturer.py b/test/unit/services/test_manufacturer.py index 9ece0154..0133b879 100644 --- a/test/unit/services/test_manufacturer.py +++ b/test/unit/services/test_manufacturer.py @@ -1,6 +1,4 @@ -""" -Unit tests for the `ManufacturerService` service. -""" +"""Unit tests for the `ManufacturerService` service.""" # Expect some duplicate code inside tests as the tests for the different entities can be very similar # pylint: disable=duplicate-code @@ -23,9 +21,7 @@ class ManufacturerServiceDSL: - """ - Base class for `ManufacturerService` unit tests. - """ + """Base class for `ManufacturerService` unit tests.""" wrapped_utils: Mock mock_manufacturer_repository: Mock @@ -50,9 +46,7 @@ def setup( class CreateDSL(ManufacturerServiceDSL): - """ - Base class for create tests. - """ + """Base class for `create` tests.""" _manufacturer_post: ManufacturerPostSchema _expected_manufacturer_in: ManufacturerIn @@ -77,47 +71,36 @@ def mock_create(self, manufacturer_post_data: dict) -> None: ServiceTestHelpers.mock_create(self.mock_manufacturer_repository, self._expected_manufacturer_out) def call_create(self) -> None: - """ - Calls the `ManufacturerService` `create` method with the appropriate data from a prior call to `mock_create`. - """ + """Calls the `ManufacturerService` `create` method with the appropriate data from a prior call to + `mock_create`.""" self._created_manufacturer = self.manufacturer_service.create(self._manufacturer_post) def check_create_success(self) -> None: - """ - Checks that a prior call to `call_create` worked as expected. - """ + """Checks that a prior call to `call_create` worked as expected.""" self.wrapped_utils.generate_code.assert_called_once_with(self._expected_manufacturer_out.name, "manufacturer") self.mock_manufacturer_repository.create.assert_called_once_with(self._expected_manufacturer_in) assert self._created_manufacturer == self._expected_manufacturer_out class TestCreate(CreateDSL): - """ - Tests for creating a manufacturer. - """ + """Tests for creating a manufacturer.""" def test_create(self): - """ - Test creating a manufacturer. - """ + """Test creating a manufacturer.""" self.mock_create(MANUFACTURER_POST_DATA_A) self.call_create() self.check_create_success() class GetDSL(ManufacturerServiceDSL): - """ - Base class for get tests. - """ + """Base class for `get` tests.""" _obtained_manufacturer_id: str _expected_manufacturer: MagicMock _obtained_manufacturer: MagicMock def mock_get(self) -> None: - """ - Mocks repo methods appropriately to test the `get` service method. - """ + """Mocks repo methods appropriately to test the `get` service method.""" # Simply a return currently, so no need to use actual data self._expected_manufacturer = MagicMock() ServiceTestHelpers.mock_get(self.mock_manufacturer_repository, self._expected_manufacturer) @@ -126,81 +109,61 @@ def call_get(self, manufacturer_id: str) -> None: """ Calls the `ManufacturerService` `get` method. - :param manufacturer_id: ID of the manufacturer that will be obtained. + :param manufacturer_id: ID of the manufacturer to be obtained. """ self._obtained_manufacturer_id = manufacturer_id self._obtained_manufacturer = self.manufacturer_service.get(manufacturer_id) def check_get_success(self): - """ - Checks that a prior call to `call_get` worked as expected. - """ + """Checks that a prior call to `call_get` worked as expected.""" self.mock_manufacturer_repository.get.assert_called_once_with(self._obtained_manufacturer_id) assert self._obtained_manufacturer == self._expected_manufacturer class TestGet(GetDSL): - """ - Tests for getting a manufacturer. - """ + """Tests for getting a manufacturer.""" def test_get(self): - """ - Test getting a manufacturer. - """ + """Test getting a manufacturer.""" self.mock_get() self.call_get(str(ObjectId())) self.check_get_success() class ListDSL(ManufacturerServiceDSL): - """ - Base class for list tests. - """ + """Base class for `list` tests.""" _expected_manufacturers: MagicMock _obtained_manufacturers: MagicMock def mock_list(self) -> None: - """ - Mocks repo methods appropriately to test the `list` service method. - """ + """Mocks repo methods appropriately to test the `list` service method.""" # Simply a return currently, so no need to use actual data - self._expected_manufacturers = [MagicMock()] + self._expected_manufacturers = MagicMock() ServiceTestHelpers.mock_list(self.mock_manufacturer_repository, self._expected_manufacturers) def call_list(self) -> None: - """ - Calls the `ManufacturerService` `list` method. - """ + """Calls the `ManufacturerService` `list` method.""" self._obtained_manufacturers = self.manufacturer_service.list() def check_list_success(self) -> None: - """ - Checks that a prior call to `call_list` worked as expected. - """ + """Checks that a prior call to `call_list` worked as expected.""" self.mock_manufacturer_repository.list.assert_called_once() assert self._obtained_manufacturers == self._expected_manufacturers class TestList(ListDSL): - """ - Tests for listing manufacturers. - """ + """Tests for listing manufacturers.""" def test_list(self): - """ - Test listing manufacturers. - """ + """Test listing manufacturers.""" self.mock_list() self.call_list() self.check_list_success() class UpdateDSL(ManufacturerServiceDSL): - """ - Base class for update tests. - """ + """Base class for `update` tests.""" _stored_manufacturer: Optional[ManufacturerOut] _manufacturer_patch: ManufacturerPatchSchema @@ -272,9 +235,7 @@ def call_update_expecting_error(self, manufacturer_id: str, error_type: type[Bas self._update_exception = exc def check_update_success(self) -> None: - """ - Checks that a prior call to `call_update` worked as updated. - """ + """Checks that a prior call to `call_update` worked as updated.""" # Ensure obtained old manufacturer self.mock_manufacturer_repository.get.assert_called_once_with(self._updated_manufacturer_id) @@ -296,21 +257,17 @@ def check_update_failed_with_exception(self, message: str): Checks that a prior call to `call_update_expecting_error` worked as expected, raising an exception with the correct message. - :param message: Message of the raised exception. + :param message: Expected message of the raised exception. """ self.mock_manufacturer_repository.update.assert_not_called() assert str(self._update_exception.value) == message class TestUpdate(UpdateDSL): - """ - Tests for updating a manufacturer. - """ + """Tests for updating a manufacturer.""" def test_update(self): - """ - Test updating all fields of a manufacturer. - """ + """Test updating all fields of a manufacturer.""" manufacturer_id = str(ObjectId()) self.mock_update( @@ -322,9 +279,7 @@ def test_update(self): self.check_update_success() def test_update_address_only(self) -> None: - """ - Test updating manufacturer's address only (code should not need regenerating as name doesn't change). - """ + """Test updating manufacturer's address only (code should not need regenerating as name doesn't change).""" manufacturer_id = str(ObjectId()) self.mock_update( @@ -336,9 +291,7 @@ def test_update_address_only(self) -> None: self.check_update_success() def test_update_with_non_existent_id(self): - """ - Test updating a manufacturer with a non-existent ID. - """ + """Test updating a manufacturer with a non-existent ID.""" manufacturer_id = str(ObjectId()) self.mock_update( @@ -349,35 +302,28 @@ def test_update_with_non_existent_id(self): class DeleteDSL(ManufacturerServiceDSL): - """ - Base class for delete tests. - """ + """Base class for `delete` tests.""" _delete_manufacturer_id: str def call_delete(self, manufacturer_id: str) -> None: """ Calls the `ManufacturerService` `delete` method. + :param manufacturer_id: ID of the manufacturer to be deleted. """ self._delete_manufacturer_id = manufacturer_id self.manufacturer_service.delete(manufacturer_id) def check_delete_success(self) -> None: - """ - Checks that a prior call to `call_delete` worked as expected. - """ + """Checks that a prior call to `call_delete` worked as expected.""" self.mock_manufacturer_repository.delete.assert_called_once_with(self._delete_manufacturer_id) class TestDelete(DeleteDSL): - """ - Tests for deleting a manufacturer. - """ + """Tests for deleting a manufacturer.""" def test_delete(self): - """ - Test deleting a manufacturer. - """ + """Test deleting a manufacturer.""" self.call_delete(str(ObjectId())) self.check_delete_success()