Skip to content

Commit b3b9424

Browse files
lugi0lugi0
authored
Add Catalog orderBy tests (#805)
* feat: add orderBy tests Signed-off-by: lugi0 <lgiorgi@redhat.com> * fix: cleanup and address comments Signed-off-by: lugi0 <lgiorgi@redhat.com> * fix: don't pass on single item Signed-off-by: lugi0 <lgiorgi@redhat.com> * feat: handle artifacts sorting Signed-off-by: lugi0 <lgiorgi@redhat.com> --------- Signed-off-by: lugi0 <lgiorgi@redhat.com>
1 parent 8199930 commit b3b9424

File tree

3 files changed

+1127
-900
lines changed

3 files changed

+1127
-900
lines changed

‎tests/model_registry/model_catalog/test_sorting_functionality.py‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
import pytest
2+
from typing import Self
3+
4+
from simple_logger.logger import get_logger
5+
from tests.model_registry.model_catalog.utils import (
6+
get_models_from_catalog_api,
7+
get_sources_with_sorting,
8+
get_artifacts_with_sorting,
9+
validate_items_sorted_correctly,
10+
)
11+
from tests.model_registry.model_catalog.constants import VALIDATED_CATALOG_ID
12+
13+
LOGGER = get_logger(name=__name__)
14+
15+
pytestmark = [pytest.mark.usefixtures("updated_dsc_component_state_scope_session", "model_registry_namespace")]
16+
17+
18+
class TestModelsSorting:
19+
"""Test sorting functionality for FindModels endpoint"""
20+
21+
@pytest.mark.parametrize(
22+
"order_by,sort_order",
23+
[
24+
("ID", "ASC"),
25+
("ID", "DESC"),
26+
pytest.param(
27+
"NAME",
28+
"ASC",
29+
marks=pytest.mark.xfail(
30+
reason="RHOAIENG-38056: Backend bug - NAME sorting not implemented, falls back to ID sorting"
31+
),
32+
),
33+
pytest.param(
34+
"NAME",
35+
"DESC",
36+
marks=pytest.mark.xfail(
37+
reason="RHOAIENG-38056: Backend bug - NAME sorting not implemented, falls back to ID sorting"
38+
),
39+
),
40+
("CREATE_TIME", "ASC"),
41+
("CREATE_TIME", "DESC"),
42+
("LAST_UPDATE_TIME", "ASC"),
43+
("LAST_UPDATE_TIME", "DESC"),
44+
],
45+
)
46+
def test_models_sorting_works_correctly(
47+
self: Self,
48+
order_by: str,
49+
sort_order: str,
50+
model_catalog_rest_url: list[str],
51+
model_registry_rest_headers: dict[str, str],
52+
):
53+
"""
54+
RHOAIENG-37260: Test models endpoint sorts correctly by field and order
55+
"""
56+
LOGGER.info(f"Testing models sorting: orderBy={order_by}, sortOrder={sort_order}")
57+
58+
response = get_models_from_catalog_api(
59+
model_catalog_rest_url=model_catalog_rest_url,
60+
model_registry_rest_headers=model_registry_rest_headers,
61+
order_by=order_by,
62+
sort_order=sort_order,
63+
)
64+
65+
assert validate_items_sorted_correctly(response["items"], order_by, sort_order)
66+
67+
68+
class TestSourcesSorting:
69+
"""Test sorting functionality for FindSources endpoint"""
70+
71+
@pytest.mark.parametrize(
72+
"order_by,sort_order",
73+
[
74+
("ID", "ASC"),
75+
("ID", "DESC"),
76+
("NAME", "ASC"),
77+
("NAME", "DESC"),
78+
],
79+
)
80+
def test_sources_sorting_works_correctly(
81+
self: Self,
82+
order_by: str,
83+
sort_order: str,
84+
model_catalog_rest_url: list[str],
85+
model_registry_rest_headers: dict[str, str],
86+
):
87+
"""
88+
RHOAIENG-37260: Test sources endpoint sorts correctly by supported fields
89+
"""
90+
LOGGER.info(f"Testing sources sorting: orderBy={order_by}, sortOrder={sort_order}")
91+
92+
response = get_sources_with_sorting(
93+
model_catalog_rest_url=model_catalog_rest_url,
94+
model_registry_rest_headers=model_registry_rest_headers,
95+
order_by=order_by,
96+
sort_order=sort_order,
97+
)
98+
99+
assert validate_items_sorted_correctly(response["items"], order_by, sort_order)
100+
101+
@pytest.mark.parametrize("unsupported_field", ["CREATE_TIME", "LAST_UPDATE_TIME"])
102+
def test_sources_rejects_unsupported_fields(
103+
self: Self,
104+
unsupported_field: str,
105+
model_catalog_rest_url: list[str],
106+
model_registry_rest_headers: dict[str, str],
107+
):
108+
"""
109+
RHOAIENG-37260: Test sources endpoint rejects fields it doesn't support
110+
"""
111+
LOGGER.info(f"Testing sources rejection of unsupported field: {unsupported_field}")
112+
113+
with pytest.raises(Exception, match="unsupported order by field"):
114+
get_sources_with_sorting(
115+
model_catalog_rest_url=model_catalog_rest_url,
116+
model_registry_rest_headers=model_registry_rest_headers,
117+
order_by=unsupported_field,
118+
sort_order="ASC",
119+
)
120+
121+
122+
class TestArtifactsSorting:
123+
"""Test sorting functionality for GetAllModelArtifacts endpoint
124+
Fixed on a random model from the validated catalog since we need more than 1 artifact to test sorting.
125+
"""
126+
127+
@pytest.mark.parametrize(
128+
"order_by,sort_order,randomly_picked_model_from_catalog_api_by_source",
129+
[
130+
("ID", "ASC", {"catalog_id": VALIDATED_CATALOG_ID, "header_type": "registry"}),
131+
("ID", "DESC", {"catalog_id": VALIDATED_CATALOG_ID, "header_type": "registry"}),
132+
pytest.param(
133+
"NAME",
134+
"ASC",
135+
{"catalog_id": VALIDATED_CATALOG_ID, "header_type": "registry"},
136+
marks=pytest.mark.xfail(reason="RHOAIENG-38056: falls back to ID sorting"),
137+
),
138+
pytest.param(
139+
"NAME",
140+
"DESC",
141+
{"catalog_id": VALIDATED_CATALOG_ID, "header_type": "registry"},
142+
marks=pytest.mark.xfail(reason="RHOAIENG-38056: falls back to ID sorting"),
143+
),
144+
],
145+
indirect=["randomly_picked_model_from_catalog_api_by_source"],
146+
)
147+
def test_artifacts_sorting_works_correctly(
148+
self: Self,
149+
order_by: str,
150+
sort_order: str,
151+
model_catalog_rest_url: list[str],
152+
model_registry_rest_headers: dict[str, str],
153+
randomly_picked_model_from_catalog_api_by_source: tuple[dict, str, str],
154+
):
155+
"""
156+
RHOAIENG-37260: Test artifacts endpoint sorts correctly by supported fields
157+
"""
158+
_, model_name, _ = randomly_picked_model_from_catalog_api_by_source
159+
LOGGER.info(f"Testing artifacts sorting for {model_name}: orderBy={order_by}, sortOrder={sort_order}")
160+
161+
response = get_artifacts_with_sorting(
162+
model_catalog_rest_url=model_catalog_rest_url,
163+
model_registry_rest_headers=model_registry_rest_headers,
164+
source_id=VALIDATED_CATALOG_ID,
165+
model_name=model_name,
166+
order_by=order_by,
167+
sort_order=sort_order,
168+
)
169+
170+
assert validate_items_sorted_correctly(response["items"], order_by, sort_order)

‎tests/model_registry/model_catalog/utils.py‎

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -697,17 +697,21 @@ def get_models_from_catalog_api(
697697
page_size: int = 100,
698698
source_label: str | None = None,
699699
q: str | None = None,
700+
order_by: str | None = None,
701+
sort_order: str | None = None,
700702
additional_params: str = "",
701703
) -> dict[str, Any]:
702704
"""
703-
Helper method to get models from catalog API with optional filtering
705+
Helper method to get models from catalog API with optional filtering and sorting
704706
705707
Args:
706708
model_catalog_rest_url: REST URL for model catalog
707709
model_registry_rest_headers: Headers for model registry REST API
708710
page_size: Number of results per page
709711
source_label: Source label(s) to filter by (must be comma-separated for multiple filters)
710712
q: Free-form keyword search to filter models
713+
order_by: Field to order results by (ID, NAME, CREATE_TIME, LAST_UPDATE_TIME)
714+
sort_order: Sort order (ASC or DESC)
711715
additional_params: Additional query parameters (e.g., "&filterQuery=name='model_name'")
712716
713717
Returns:
@@ -724,6 +728,12 @@ def get_models_from_catalog_api(
724728
if q:
725729
params["q"] = q
726730

731+
if order_by:
732+
params["orderBy"] = order_by
733+
734+
if sort_order:
735+
params["sortOrder"] = sort_order
736+
727737
# Parse additional_params string into params dict for proper URL encoding
728738
if additional_params:
729739
# Remove leading & if present
@@ -829,3 +839,130 @@ def validate_performance_data_files_on_pod(model_catalog_pod: Pod) -> dict[str,
829839
LOGGER.warning(f"Found models with missing performance data files: {validation_results}")
830840

831841
return validation_results
842+
843+
844+
def get_sources_with_sorting(
845+
model_catalog_rest_url: list[str],
846+
model_registry_rest_headers: dict[str, str],
847+
order_by: str,
848+
sort_order: str,
849+
) -> dict[str, Any]:
850+
"""
851+
Get sources with sorting parameters
852+
853+
Args:
854+
model_catalog_rest_url: REST URL for model catalog
855+
model_registry_rest_headers: Headers for model registry REST API
856+
order_by: Field to order results by (ID, NAME)
857+
sort_order: Sort order (ASC or DESC)
858+
859+
Returns:
860+
Dictionary containing the API response
861+
"""
862+
base_url = f"{model_catalog_rest_url[0]}sources"
863+
params = {
864+
"orderBy": order_by,
865+
"sortOrder": sort_order,
866+
"pageSize": 100,
867+
}
868+
869+
return execute_get_command(url=base_url, headers=model_registry_rest_headers, params=params)
870+
871+
872+
def get_artifacts_with_sorting(
873+
model_catalog_rest_url: list[str],
874+
model_registry_rest_headers: dict[str, str],
875+
source_id: str,
876+
model_name: str,
877+
order_by: str,
878+
sort_order: str,
879+
) -> dict[str, Any]:
880+
"""
881+
Get artifacts with sorting parameters
882+
883+
Args:
884+
model_catalog_rest_url: REST URL for model catalog
885+
model_registry_rest_headers: Headers for model registry REST API
886+
source_id: Source ID for the model
887+
model_name: Name of the model
888+
order_by: Field to order results by
889+
sort_order: Sort order (ASC or DESC)
890+
891+
Returns:
892+
Dictionary containing the API response
893+
"""
894+
base_url = f"{model_catalog_rest_url[0]}sources/{source_id}/models/{model_name}/artifacts"
895+
params = {
896+
"orderBy": order_by,
897+
"sortOrder": sort_order,
898+
"pageSize": 100,
899+
}
900+
901+
return execute_get_command(url=base_url, headers=model_registry_rest_headers, params=params)
902+
903+
904+
def validate_items_sorted_correctly(items: list[dict], field: str, order: str) -> bool:
905+
"""
906+
Extract field values and verify they're in correct order
907+
908+
Args:
909+
items: List of items to validate
910+
field: Field name to check sorting on
911+
order: Sort order (ASC or DESC)
912+
913+
Returns:
914+
True if items are sorted correctly, False otherwise
915+
"""
916+
if len(items) <= 1:
917+
if field == "NAME" and items[0].get("artifactType") == "model-artifact":
918+
# When testing sorting for model artifacts we use only models from the validated catalog, since
919+
# they almost all have more than 1 artifact. However, some of these models still return a single artifact.
920+
# Given that this is currently the expected behavior, we return True.
921+
single_artifact_models = [
922+
"mistral-small-24B",
923+
"gemma-2",
924+
"granite-3.1-8b-base-quantized.w4a16",
925+
"granite-3.1-8b-instruct-FP8-dynamic",
926+
"granite-3.1-8b-starter-v2",
927+
]
928+
if any(single_artifact_model in items[0].get("uri") for single_artifact_model in single_artifact_models):
929+
return True
930+
else:
931+
# In any other case, we expect at least 2 items to sort.
932+
raise ValueError(f"At least 2 items are required to sort, got {len(items)}")
933+
934+
# Extract field values for comparison
935+
values = []
936+
for item in items:
937+
if field == "ID":
938+
value = item.get("id")
939+
elif field == "NAME":
940+
value = item.get("name")
941+
elif field == "CREATE_TIME":
942+
value = item.get("createTimeSinceEpoch")
943+
elif field == "LAST_UPDATE_TIME":
944+
value = item.get("lastUpdateTimeSinceEpoch")
945+
else:
946+
raise ValueError(f"Invalid field: {field}")
947+
948+
if value is None:
949+
raise ValueError(f"Field {field} is missing from item: {item}")
950+
951+
values.append(value)
952+
953+
# Convert values to appropriate types for comparison
954+
if field == "ID":
955+
# Convert IDs to integers for numeric comparison
956+
try:
957+
values = [int(v) for v in values]
958+
except ValueError:
959+
# If conversion fails, fall back to string comparison
960+
values = [str(v) for v in values]
961+
962+
# Check if values are in correct order
963+
if order == "ASC":
964+
return all(values[i] <= values[i + 1] for i in range(len(values) - 1))
965+
elif order == "DESC":
966+
return all(values[i] >= values[i + 1] for i in range(len(values) - 1))
967+
else:
968+
raise ValueError(f"Invalid sort order: {order}")

0 commit comments

Comments
 (0)