Fix reamining tests, linting, TODOs and clean up #347

ral-facilities · Aug 9, 2024 · 733934e · 733934e
1 parent a186bc2
commit 733934e
Show file tree

Hide file tree

Showing 10 changed files with 72 additions and 134 deletions.
diff --git a/test/e2e/mock_schemas.py b/test/e2e/mock_schemas.py
@@ -4,67 +4,13 @@
 
 # Leaf with allowed values in properties
 
-from unittest.mock import ANY
-
-CREATED_MODIFIED_VALUES_EXPECTED = {"created_time": ANY, "modified_time": ANY}
 
-# pylint: disable=duplicate-code
-CATALOGUE_CATEGORY_POST_ALLOWED_VALUES = {
-    "name": "Category Allowed Values",
-    "is_leaf": True,
-    "properties": [
-        {
-            "name": "Property A",
-            "type": "number",
-            "unit": "mm",
-            "mandatory": False,
-            "allowed_values": {"type": "list", "values": [2, 4, 6]},
-        },
-        {
-            "name": "Property B",
-            "type": "string",
-            "unit": None,
-            "mandatory": True,
-            "allowed_values": {"type": "list", "values": ["red", "green"]},
-        },
-    ],
-}
-
-# To be posted on CATALOGUE_CATEGORY_POST_ALLOWED_VALUES
-CATALOGUE_ITEM_POST_ALLOWED_VALUES = {
-    "name": "Catalogue Item D",
-    "description": "This is Catalogue Item D",
-    "cost_gbp": 300.00,
-    "cost_to_rework_gbp": 120.99,
-    "days_to_replace": 1.5,
-    "days_to_rework": 3.0,
-    "drawing_number": "789xyz",
-    "is_obsolete": False,
-    "properties": [{"name": "Property A", "value": 4}, {"name": "Property B", "value": "red"}],
-}
+# pylint: disable=fixme
+# TODO: Remove this file - replace by mock_data.py
 
-ITEM_POST_ALLOWED_VALUES = {
-    "is_defective": False,
-    "warranty_end_date": "2015-11-15T23:59:59Z",
-    "serial_number": "xyz123",
-    "delivered_date": "2012-12-05T12:00:00Z",
-    "notes": "Test notes",
-    "properties": [{"name": "Property A", "value": 6}, {"name": "Property B", "value": "green"}],
-}
+from unittest.mock import ANY
 
-ITEM_POST_ALLOWED_VALUES_EXPECTED = {
-    **ITEM_POST_ALLOWED_VALUES,
-    **CREATED_MODIFIED_VALUES_EXPECTED,
-    "id": ANY,
-    "purchase_order_number": None,
-    "usage_status": "New",
-    "usage_status_id": ANY,
-    "asset_number": None,
-    "properties": [
-        {"name": "Property A", "unit": "mm", "value": 6},
-        {"name": "Property B", "value": "green", "unit": None},
-    ],
-}
+CREATED_MODIFIED_VALUES_EXPECTED = {"created_time": ANY, "modified_time": ANY}
 
 SYSTEM_POST_A = {
     "name": "System A",

diff --git a/test/e2e/test_catalogue_category.py b/test/e2e/test_catalogue_category.py
@@ -1,9 +1,9 @@
-# pylint: disable=too-many-lines
 """
 End-to-End tests for the catalogue category router.
 """
 
 # Expect some duplicate code inside tests as the tests for the different entities can be very similar
+# pylint: disable=too-many-lines
 # pylint: disable=duplicate-code
 # pylint: disable=too-many-public-methods
 
@@ -24,16 +24,17 @@
     CATALOGUE_CATEGORY_PROPERTY_DATA_BOOLEAN_MANDATORY,
     CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT,
     CATALOGUE_ITEM_DATA_REQUIRED_VALUES_ONLY,
-    MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY, UNIT_POST_DATA_MM)
+    MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY,
+    UNIT_POST_DATA_MM,
+)
 from typing import Optional
 
 import pytest
 from bson import ObjectId
 from fastapi.testclient import TestClient
 from httpx import Response
 
-from inventory_management_system_api.core.consts import \
-    BREADCRUMBS_TRAIL_MAX_LENGTH
+from inventory_management_system_api.core.consts import BREADCRUMBS_TRAIL_MAX_LENGTH
 
 
 class CreateDSL:

diff --git a/test/e2e/test_catalogue_item.py b/test/e2e/test_catalogue_item.py
@@ -1,16 +1,15 @@
-# pylint: disable=too-many-lines
 """
 End-to-End tests for the catalogue item router.
 """
 
 # Expect some duplicate code inside tests as the tests for the different entities can be very similar
+# pylint: disable=too-many-lines
 # pylint: disable=duplicate-code
 # pylint: disable=too-many-public-methods
 
 from test.e2e.conftest import E2ETestHelpers
 from test.e2e.mock_schemas import SYSTEM_POST_A, USAGE_STATUS_POST_A
-from test.e2e.test_catalogue_category import \
-    CreateDSL as CatalogueCategoryCreateDSL
+from test.e2e.test_catalogue_category import CreateDSL as CatalogueCategoryCreateDSL
 from test.e2e.test_manufacturer import CreateDSL as ManufacturerCreateDSL
 from test.mock_data import (
     BASE_CATALOGUE_CATEGORY_DATA_WITH_PROPERTIES_MM,
@@ -44,7 +43,8 @@
     PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_1,
     PROPERTY_GET_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT_42,
     PROPERTY_GET_DATA_STRING_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST_VALUE1,
-    UNIT_POST_DATA_MM)
+    UNIT_POST_DATA_MM,
+)
 from typing import Any, Optional
 
 import pytest
@@ -269,9 +269,8 @@ def check_post_catalogue_item_success(self, expected_catalogue_item_get_data: di
         returned.
 
         :param expected_catalogue_item_get_data: Dictionary containing the expected catalogue item data returned as
-                                                 would be required for a `CatalogueItemSchema`. Does not need mandatory
-                                                 IDs (e.g. `manufacturer_id`) as they will be to check they are as
-                                                 expected.
+                                would be required for a `CatalogueItemSchema`. Does not need mandatory IDs (e.g.
+                                `manufacturer_id`) as they will be added automatically to check they are as expected.
         """
 
         assert self._post_response_catalogue_item.status_code == 201

diff --git a/test/e2e/test_item.py b/test/e2e/test_item.py
@@ -1,30 +1,17 @@
-# pylint: disable=too-many-lines
 """
-End-to-End tests for the catalogue item router.
+End-to-End tests for the item router.
 """
 
 # Expect some duplicate code inside tests as the tests for the different entities can be very similar
+# pylint: disable=too-many-lines
 # pylint: disable=duplicate-code
 # pylint: disable=too-many-public-methods
 # pylint: disable=too-many-ancestors
 
-from test.conftest import add_ids_to_properties
-from test.e2e.conftest import E2ETestHelpers, replace_unit_values_with_ids_in_properties
-from test.e2e.mock_schemas import (
-    CATALOGUE_CATEGORY_POST_ALLOWED_VALUES,
-    CATALOGUE_ITEM_POST_ALLOWED_VALUES,
-    CREATED_MODIFIED_VALUES_EXPECTED,
-    ITEM_POST_ALLOWED_VALUES,
-    ITEM_POST_ALLOWED_VALUES_EXPECTED,
-    SYSTEM_POST_A,
-    SYSTEM_POST_B,
-    USAGE_STATUS_POST_A,
-)
+from test.e2e.conftest import E2ETestHelpers
 from test.e2e.test_catalogue_item import CreateDSL as CatalogueItemCreateDSL
 from test.e2e.test_system import CreateDSL as SystemCreateDSL
-from test.e2e.test_unit import UNIT_POST_A, UNIT_POST_B
 from test.mock_data import (
-    CATALOGUE_CATEGORY_POST_DATA_LEAF_NO_PARENT_NO_PROPERTIES,
     CATALOGUE_CATEGORY_PROPERTY_DATA_BOOLEAN_MANDATORY,
     CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_ALLOWED_VALUES_LIST,
     CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT,
@@ -54,7 +41,6 @@
     USAGE_STATUS_DATA_NEW,
 )
 from typing import Any, Optional
-from unittest.mock import ANY
 
 import pytest
 from bson import ObjectId
@@ -78,13 +64,39 @@ def setup_item_create_dsl(self):
         self.system_id = None
         self.usage_status_value_id_dict = {}
 
+    def merge_properties_in_expected_item_get_data(self, expected_item_get_data: dict) -> dict:
+        """
+        Merges any existing properties in an already posted catalogue item into expected get data returned for an item.
+
+        Assumes the last catalogue item posted was the one the properties should be merged with.
+
+        :param expected_item_get_data: Dictionary containing the expected item data returned as would be required for a
+                                       `ItemSchema`. Does not need mandatory IDs (e.g. `system_id`).
+        :return: Dictionary containing the same `expected_item_get_data` but with any missing properties found inside
+                 the last posted catalogue item merged in.
+        """
+
+        if "properties" in expected_item_get_data:
+            catalogue_item_data = self._post_response_catalogue_item.json()
+            expected_merged_properties = []
+            supplied_properties_dict = {
+                supplied_property["name"]: supplied_property
+                for supplied_property in expected_item_get_data["properties"]
+            }
+            for prop in catalogue_item_data["properties"]:
+                supplied_property = supplied_properties_dict.get(prop["name"])
+                expected_merged_properties.append(supplied_property if supplied_property else prop)
+
+            return {**expected_item_get_data, "properties": expected_merged_properties}
+        return expected_item_get_data
+
     def add_ids_to_expected_item_get_data(self, expected_item_get_data) -> dict:
         """
         Adds required IDs to some expected item get data based on what has already been posted.
 
-        :param expected_item_get_data: Dictionary containing the expected catalogue item data returned as would be
-                                       required for an `ItemSchema`. Does not need mandatory IDs (e.g. `system_id`) as
-                                       they will be added here.
+        :param expected_item_get_data: Dictionary containing the expected item data returned as would be required for an
+                                       `ItemSchema`. Does not need mandatory IDs (e.g. `system_id`) as they will be
+                                       added here.
         """
         # Where there are properties add the property ID, unit ID and unit value
         expected_item_get_data = E2ETestHelpers.add_property_ids_to_properties(
@@ -154,10 +166,10 @@ def post_item(self, item_data: dict) -> Optional[str]:
         """
         Posts an item with the given data and returns the ID of the created item if successful.
 
-        :param item_data: Dictionary containing the basic catalogue item data as would be required for a
-                          `ItemPostSchema` but with mandatory IDs missing and any `id`'s replaced by the `name` value
-                          in its properties as the IDs will be added automatically.
-        :return: ID of the created catalogue item (or `None` if not successful).
+        :param item_data: Dictionary containing the basic item data as would be required for a `ItemPostSchema` but with
+                          mandatory IDs missing and any `id`'s replaced by the `name` value in its properties as the IDs
+                          will be added automatically.
+        :return: ID of the created item (or `None` if not successful).
         """
 
         # Replace any unit values with unit IDs
@@ -289,24 +301,12 @@ def check_post_item_success(self, expected_item_get_data: dict) -> None:
         Also merges in any properties that were defined in the catalogue item but are not given in the expected data.
 
         :param expected_item_get_data: Dictionary containing the expected item data returned as would be required for a
-                                       `ItemSchema`. Does not need mandatory IDs (e.g. `system_id`) as they will be to
-                                       check they are as expected.
+                                       `ItemSchema`. Does not need mandatory IDs (e.g. `system_id`) as they will be
+                                       added automatically to check they are as expected.
         """
 
         # Where properties are involved in the catalogue item, need to merge them
-        catalogue_item_data = self._post_response_catalogue_item.json()
-        expected_item_get_data = expected_item_get_data.copy()
-        if "properties" in expected_item_get_data:
-            expected_merged_properties = []
-            supplied_properties_dict = {
-                supplied_property["name"]: supplied_property
-                for supplied_property in expected_item_get_data["properties"]
-            }
-            for prop in catalogue_item_data["properties"]:
-                supplied_property = supplied_properties_dict.get(prop["name"])
-                expected_merged_properties.append(supplied_property if supplied_property else prop)
-
-            expected_item_get_data["properties"] = expected_merged_properties
+        expected_item_get_data = self.merge_properties_in_expected_item_get_data(expected_item_get_data)
 
         assert self._post_response_item.status_code == 201
         assert self._post_response_item.json() == self.add_ids_to_expected_item_get_data(expected_item_get_data)
@@ -608,7 +608,7 @@ def get_item(self, item_id: str) -> None:
         """
         Gets an item with the given ID.
 
-        :param item_id: ID of the catalogue item to be obtained.
+        :param item_id: ID of the item to be obtained.
         """
 
         self._get_response_item = self.test_client.get(f"/v1/items/{item_id}")
@@ -838,21 +838,8 @@ def check_patch_item_response_success(self, expected_item_get_data: dict) -> Non
                                                  they are as expected.
         """
 
-        # TODO: Move this to common function - its done in the CreateDSL too
         # Where properties are involved in the catalogue item, need to merge them
-        catalogue_item_data = self._post_response_catalogue_item.json()
-        expected_item_get_data = expected_item_get_data.copy()
-        if "properties" in expected_item_get_data:
-            expected_merged_properties = []
-            supplied_properties_dict = {
-                supplied_property["name"]: supplied_property
-                for supplied_property in expected_item_get_data["properties"]
-            }
-            for prop in catalogue_item_data["properties"]:
-                supplied_property = supplied_properties_dict.get(prop["name"])
-                expected_merged_properties.append(supplied_property if supplied_property else prop)
-
-            expected_item_get_data["properties"] = expected_merged_properties
+        expected_item_get_data = self.merge_properties_in_expected_item_get_data(expected_item_get_data)
 
         assert self._patch_response_item.status_code == 200
         assert self._patch_response_item.json() == self.add_ids_to_expected_item_get_data(expected_item_get_data)

diff --git a/test/e2e/test_system.py b/test/e2e/test_system.py
@@ -5,7 +5,6 @@
 # Expect some duplicate code inside tests as the tests for the different entities can be very similar
 # pylint: disable=duplicate-code
 
-from test.conftest import add_ids_to_properties
 from test.e2e.conftest import E2ETestHelpers
 from test.e2e.mock_schemas import USAGE_STATUS_POST_B
 from test.mock_data import (

diff --git a/test/e2e/test_usage_status.py b/test/e2e/test_usage_status.py
@@ -14,7 +14,12 @@
     USAGE_STATUS_POST_D,
     USAGE_STATUS_POST_D_EXPECTED,
 )
-from test.e2e.test_item import CATALOGUE_CATEGORY_POST_A, CATALOGUE_ITEM_POST_A, ITEM_POST, MANUFACTURER_POST
+from test.mock_data import (
+    CATALOGUE_CATEGORY_POST_DATA_LEAF_NO_PARENT_NO_PROPERTIES,
+    CATALOGUE_ITEM_DATA_REQUIRED_VALUES_ONLY,
+    ITEM_DATA_REQUIRED_VALUES_ONLY,
+    MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY,
+)
 
 from bson import ObjectId
 
@@ -129,18 +134,20 @@ def test_delete_with_a_non_existent_id(test_client):
 def test_delete_usage_status_that_is_a_part_of_item(test_client):
     """Test trying to delete a usage status that is a part of an Item"""
     # pylint: disable=duplicate-code
-    response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A)
+    response = test_client.post(
+        "/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_DATA_LEAF_NO_PARENT_NO_PROPERTIES
+    )
     catalogue_category = response.json()
 
     response = test_client.post("/v1/systems", json=SYSTEM_POST_A)
     system_id = response.json()["id"]
 
-    response = test_client.post("/v1/manufacturers", json=MANUFACTURER_POST)
+    response = test_client.post("/v1/manufacturers", json=MANUFACTURER_POST_DATA_REQUIRED_VALUES_ONLY)
 
     manufacturer_id = response.json()["id"]
 
     catalogue_item_post = {
-        **CATALOGUE_ITEM_POST_A,
+        **CATALOGUE_ITEM_DATA_REQUIRED_VALUES_ONLY,
         "catalogue_category_id": catalogue_category["id"],
         "manufacturer_id": manufacturer_id,
         "properties": add_ids_to_properties(
@@ -160,11 +167,10 @@ def test_delete_usage_status_that_is_a_part_of_item(test_client):
     usage_status_id = response.json()["id"]
 
     item_post = {
-        **ITEM_POST,
+        **ITEM_DATA_REQUIRED_VALUES_ONLY,
         "catalogue_item_id": catalogue_item_id,
         "system_id": system_id,
         "usage_status_id": usage_status_id,
-        "properties": add_ids_to_properties(catalogue_category["properties"], ITEM_POST["properties"]),
     }
     # pylint: enable=duplicate-code
     response = test_client.post("/v1/items", json=item_post)

diff --git a/test/unit/repositories/test_catalogue_category.py b/test/unit/repositories/test_catalogue_category.py
@@ -1,10 +1,10 @@
-# pylint: disable=too-many-lines
 """
 Unit tests for the `CatalogueCategoryRepo` repository.
 """
 
 # 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 (
     CATALOGUE_CATEGORY_IN_DATA_LEAF_NO_PARENT_NO_PROPERTIES,

diff --git a/test/unit/services/test_catalogue_category.py b/test/unit/services/test_catalogue_category.py
@@ -1,9 +1,9 @@
-# pylint: disable=too-many-lines
 """
 Unit tests for the `CatalogueCategoryService` service.
 """
 
 # Expect some duplicate code inside tests as the tests for the different entities can be very similar
+# pylint: disable=too-many-lines
 # pylint: disable=duplicate-code
 
 from test.mock_data import (

diff --git a/test/unit/services/test_catalogue_item.py b/test/unit/services/test_catalogue_item.py
@@ -1,9 +1,9 @@
-# pylint: disable=too-many-lines
 """
 Unit tests for the `CatalogueCategoryService` service.
 """
 
 # Expect some duplicate code inside tests as the tests for the different entities can be very similar
+# pylint: disable=too-many-lines
 # pylint: disable=duplicate-code
 
 from test.mock_data import (