Clean up tests

[VKTB]

• [ ] Refactor tests where possible
• [ ] Remove code duplication

etc

[VKTB]

Update the with pytest.raises ... tests to have a .assert_not_called(). For example, in the test_update_with_nonexistent_id in test/unit/services/test_catalogue_item.py, we should add catalogue_item_repository_mock.update.assert_not_called().

Ref: https://github.com/ral-facilities/inventory-management-system-api/pull/78#discussion_r1363959210

[joelvdavies]

• find_one mocks are returning objects when they should return dictionaries in the test helpers of many tests
• Can also use model_dump to assign In models from Out models helping to simplify tests as any fields such as id that are not present will be ignored

See #110

[joelvdavies]

I think it would be nice to combine post requests in unit tests e.g. a utility function something like

def _post_catalogue_categories(test_client, catalogue_categories: list[dict], manufacturer: Optional[dict] = None):
    """Utility function that posts a catalogue categories and a manufacturer and returns a list of
    corresponding dictionaries containing the ids ready for passing to a catalogue item

    :param catalogue_categories: Catalogue category dictionaries to post (When None uses the default)
    :param manufacturer: Manufactuer dictionary to post (When None uses the default)
    :returns: List of dictionary containing the catalogue category id and manufacturer id for each given
              category dictionary
    """
    id_dicts = []

    manufacturer_id = None
    if manufacturer is not None:
        response = test_client.post("/v1/manufacturers", json=manufacturer)
        manufacturer_id = response.json()["id"]

    for catalogue_category in catalogue_categories:
        response = test_client.post("/v1/catalogue-categories", json=catalogue_category)
        id_dict = {"catalogue_category_id": response.json()["id"]}
        if manufacturer_id is not None:
            id_dict["manufacturer_id"] = manufacturer_id
        id_dicts.append(id_dict)

    return id_dicts

Could be used in catalogue_items to avoid posting the categories and manufacturer's repeatedly, items likewise repeats posting manufacturers, catalogue categories and catalogue items but with functions like these they could be imported and added upon.

[VKTB]

A util class like the one below might be nice

class TestClient:

    def __init__(self, test_client):
        # self.catalogue_category = {
        #     "id": None,
        #     "parent_id": None,
        #     "name": None,
        #     "code": None,
        #     "is_leaf": None,
        #     "catalogue_item_properties": None,
        # }
        self.test_client = test_client
        self.catalogue_categories = {}
        self.catalogue_items = {}
        self.catalogue_items_expected = {}
        self.items = {}
        self.manufacturers = {}
        self.systems = {}

    def create_catalogue_category(self, catalogue_category_data: dict):
        response = self.test_client.post("/v1/catalogue-categories", json=catalogue_category_data)
        catalogue_category = response.json()
        self.catalogue_categories[catalogue_category["id"]] = catalogue_category
        return response.status_code, catalogue_category

    def create_catalogue_item(self, catalogue_item_data: dict):
        properties = []
        for catalogue_item_prop in catalogue_item_data["properties"]:
            catalogue_category_prop = self._get_catalogue_category_property_by_name(
                catalogue_item_data["catalogue_category_id"], catalogue_item_prop["name"]
            )
            properties.append({"id": catalogue_category_prop["id"], "value": catalogue_item_prop["value"]})

        catalogue_item_data = {**catalogue_item_data, "properties": properties}
        response = self.test_client.post("/v1/catalogue-items", json=catalogue_item_data)
        catalogue_item = response.json()
        self.catalogue_items[catalogue_item["id"]] = catalogue_item
        return response.status_code, catalogue_item

    def create_item(self, item_data: dict) -> None:
        pass

    def create_manufacturer(self, manufacturer_data: dict):
        response = self.test_client.post("/v1/manufacturers", json=manufacturer_data)
        manufacturer = response.json()
        self.manufacturers[manufacturer["id"]] = manufacturer
        return response.status_code, manufacturer

    def _get_catalogue_category_property_by_name(self, catalogue_category_id: str, property_name: str) -> dict:
        return next(
            prop
            for prop in self.catalogue_categories[catalogue_category_id]["catalogue_item_properties"]
            if prop["name"] == property_name
        )

    def get_catalogue_item_expected_data(self, catalogue_item_id: str) -> dict:
        return

def test_create_catalogue_item(test_client):
    """
    Test creating a catalogue item.
    """
    tclient = TestClient(test_client)
    _, catalogue_category = tclient.create_catalogue_category(CATALOGUE_CATEGORY_POST_A)
    _, manufacturer = tclient.create_manufacturer(MANUFACTURER)
    status_code, catalogue_item = tclient.create_catalogue_item(
        {
            **CATALOGUE_ITEM_POST_A,
            "catalogue_category_id": catalogue_category["id"],
            "manufacturer_id": manufacturer["id"],
        }
    )

    assert status_code == 201
    assert catalogue_item == {}

[joelvdavies]

See also https://medium.com/@bas.dijkstra54/making-test-frameworks-readable-the-domain-specific-language-c154c9a9abcb