Skip to content

Commit e2ae95a

Browse files
fegedbasunag
fege
and
dbasunag
authored
test: update filter options validation for numeric properties (#886)
* test: update filter options validation for numeric properties - Update SQL query to match new materialized views - Add support for numeric properties with range validation (double_value, int_value) - Update property naming convention for artifact properties (artifacts.{name}.{type}) - Add metricsType and model_id to excluded filter fields * fix: add back the fixture test_idp_user to properly test the use non-admin case --------- Co-authored-by: Debarati Basu-Nag <dbasunag@redhat.com>
1 parent 3a0c2cf commit e2ae95a

File tree

3 files changed

+109
-53
lines changed

3 files changed

+109
-53
lines changed

‎tests/model_registry/model_catalog/db_constants.py‎

Lines changed: 49 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,49 @@
11
# Constants useful for querying the model catalog database and parsing its responses
22

33
# SQL query for filter_options endpoint database validation
4-
# Replicates the exact database query used by GetFilterableProperties for the filter_options endpoint
5-
# in kubeflow/model-registry catalog/internal/db/service/catalog_model.go
6-
# Note: Uses dynamic type_id lookup via 'kf.CatalogModel' name since type_id appears to be dynamic
4+
# Queries materialized views (context_property_options, artifact_property_options) that aggregate
5+
# filterable properties for CatalogModel. Based on GetFilterableProperties in
6+
# kubeflow/model-registry catalog/internal/db/service/catalog_model.go (PR #1875)
7+
#
8+
# Property naming:
9+
# - Context properties: base name only (special case: 'validated_on.array_value' for arrays)
10+
# - Artifact properties: 'artifacts.{name}.{type}' where type is string_value/array_value/double_value/int_value
11+
#
12+
# Return format:
13+
# - String/array properties: text array of values
14+
# - Numeric properties: 2-element text array [min, max] converted from double/int columns
715
FILTER_OPTIONS_DB_QUERY = """
8-
SELECT name, array_agg(string_value) FROM (
9-
SELECT
10-
name,
11-
string_value
12-
FROM "ContextProperty" WHERE
13-
context_id IN (
14-
SELECT id FROM "Context" WHERE type_id = (
15-
SELECT id FROM "Type" WHERE name = 'kf.CatalogModel'
16-
)
17-
)
18-
AND string_value IS NOT NULL
19-
AND string_value != ''
20-
AND string_value IS NOT JSON ARRAY
16+
SELECT
17+
CASE
18+
WHEN name = 'validated_on' AND array_value IS NOT NULL THEN name || '.array_value'
19+
ELSE name
20+
END AS name,
21+
COALESCE(string_value, array_value, '{}'::text[]) AS array_agg
22+
FROM context_property_options
23+
WHERE type_id = (SELECT id FROM "Type" WHERE name = 'kf.CatalogModel')
2124
22-
UNION
25+
UNION ALL
2326
24-
SELECT
25-
name,
26-
json_array_elements_text(string_value::json) AS string_value
27-
FROM "ContextProperty" WHERE
28-
context_id IN (
29-
SELECT id FROM "Context" WHERE type_id = (
30-
SELECT id FROM "Type" WHERE name = 'kf.CatalogModel'
31-
)
32-
)
33-
AND string_value IS JSON ARRAY
34-
)
35-
GROUP BY name HAVING MAX(CHAR_LENGTH(string_value)) <= 100;
27+
SELECT
28+
'artifacts.' ||
29+
CASE
30+
WHEN string_value IS NOT NULL THEN name || '.string_value'
31+
WHEN array_value IS NOT NULL THEN name || '.array_value'
32+
WHEN min_double_value IS NOT NULL THEN name || '.double_value'
33+
WHEN min_int_value IS NOT NULL THEN name || '.int_value'
34+
ELSE name
35+
END AS name,
36+
CASE
37+
WHEN min_double_value IS NOT NULL THEN
38+
ARRAY[min_double_value::text, max_double_value::text]
39+
WHEN min_int_value IS NOT NULL THEN
40+
ARRAY[min_int_value::text, max_int_value::text]
41+
ELSE
42+
COALESCE(string_value, array_value, '{}'::text[])
43+
END AS array_agg
44+
FROM artifact_property_options
45+
46+
ORDER BY name;
3647
"""
3748

3849
# SQL query for search functionality database validation
@@ -155,5 +166,12 @@
155166
"""
156167

157168
# Fields that are explicitly filtered out by the filter_options endpoint API
158-
# From db_catalog.go:204-206 in kubeflow/model-registry GetFilterOptions method
159-
API_EXCLUDED_FILTER_FIELDS = {"source_id", "logo", "license_link"}
169+
# From db_catalog.go in kubeflow/model-registry GetFilterOptions method
170+
# Updated with PR #1875 to include metricsType and model_id exclusions
171+
API_EXCLUDED_FILTER_FIELDS = {
172+
"source_id",
173+
"logo",
174+
"license_link",
175+
"artifacts.metricsType.string_value", # artifact property with full name
176+
"artifacts.model_id.string_value", # artifact property with full name
177+
}

‎tests/model_registry/model_catalog/test_filter_options_endpoint.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ def test_filter_options_endpoint_validation(
9595
],
9696
indirect=["user_token_for_api_calls"],
9797
)
98-
@pytest.mark.xfail(strict=True, reason="RHOAIENG-37069: backend/API discrepancy expected")
9998
def test_comprehensive_coverage_against_database(
10099
self: Self,
101100
model_catalog_rest_url: list[str],

‎tests/model_registry/model_catalog/utils.py‎

Lines changed: 60 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -417,32 +417,71 @@ def compare_filter_options_with_database(
417417
# Log detailed comparison for each property
418418
for prop_name in sorted(set(expected_properties.keys()) | set(api_filters.keys())):
419419
if prop_name in expected_properties and prop_name in api_filters:
420-
db_values = set(expected_properties[prop_name])
421-
api_values = set(api_filters[prop_name]["values"])
422-
423-
missing_values = db_values - api_values
424-
extra_values = api_values - db_values
425-
426-
if missing_values:
427-
error_msg = (
428-
f"Property '{prop_name}': DB has {len(missing_values)} values missing from API: {missing_values}"
429-
)
430-
LOGGER.error(error_msg)
431-
comparison_errors.append(error_msg)
432-
if extra_values:
433-
error_msg = (
434-
f"Property '{prop_name}': API has {len(extra_values)} values missing from DB: {extra_values}"
435-
)
436-
LOGGER.error(error_msg)
437-
comparison_errors.append(error_msg)
438-
if not missing_values and not extra_values:
439-
LOGGER.info(f"Property '{prop_name}': Perfect match ({len(api_values)} values)")
420+
db_data = expected_properties[prop_name]
421+
api_filter = api_filters[prop_name]
422+
423+
# Check if this is a numeric property (has "range" in API response)
424+
if "range" in api_filter:
425+
# Numeric property: DB has [min, max] as 2-element array
426+
if len(db_data) == 2:
427+
try:
428+
db_min, db_max = float(db_data[0]), float(db_data[1])
429+
api_min = api_filter["range"]["min"]
430+
api_max = api_filter["range"]["max"]
431+
432+
if db_min != api_min or db_max != api_max:
433+
error_msg = (
434+
f"Property '{prop_name}': Range mismatch - DB: [{db_min}, {db_max}], "
435+
f"API: [{api_min}, {api_max}]"
436+
)
437+
LOGGER.error(error_msg)
438+
comparison_errors.append(error_msg)
439+
else:
440+
LOGGER.info(f"Property '{prop_name}': Perfect range match (min={api_min}, max={api_max})")
441+
except (ValueError, TypeError) as e:
442+
error_msg = f"Property '{prop_name}': Failed to parse numeric values - {e}"
443+
LOGGER.error(error_msg)
444+
comparison_errors.append(error_msg)
445+
else:
446+
error_msg = f"Property '{prop_name}': Expected 2 values for range, got {len(db_data)}"
447+
LOGGER.error(error_msg)
448+
comparison_errors.append(error_msg)
449+
else:
450+
# String/array property: compare values as sets
451+
db_values = set(db_data)
452+
api_values = set(api_filter["values"])
453+
454+
missing_values = db_values - api_values
455+
extra_values = api_values - db_values
456+
457+
if missing_values:
458+
error_msg = (
459+
f"Property '{prop_name}': DB has {len(missing_values)} "
460+
f"values missing from API: {missing_values}"
461+
)
462+
LOGGER.error(error_msg)
463+
comparison_errors.append(error_msg)
464+
if extra_values:
465+
error_msg = (
466+
f"Property '{prop_name}': API has {len(extra_values)} values missing from DB: {extra_values}"
467+
)
468+
LOGGER.error(error_msg)
469+
comparison_errors.append(error_msg)
470+
if not missing_values and not extra_values:
471+
LOGGER.info(f"Property '{prop_name}': Perfect match ({len(api_values)} values)")
440472
elif prop_name in expected_properties:
441473
error_msg = f"Property '{prop_name}': In DB ({len(expected_properties[prop_name])} values) but NOT in API"
442474
LOGGER.error(error_msg)
443475
comparison_errors.append(error_msg)
444476
elif prop_name in api_filters:
445-
error_msg = f"Property '{prop_name}': In API ({len(api_filters[prop_name]['values'])} values) but NOT in DB"
477+
LOGGER.info(f"Property name: '{prop_name}' in API filters: {api_filters[prop_name]}")
478+
# For properties only in API, we can't reliably get DB values, so skip logging them
479+
if "range" in api_filters[prop_name]:
480+
error_msg = f"Property '{prop_name}': In API (range property) but NOT in DB"
481+
else:
482+
error_msg = (
483+
f"Property '{prop_name}': In API ({len(api_filters[prop_name]['values'])} values) but NOT in DB"
484+
)
446485
LOGGER.error(error_msg)
447486
comparison_errors.append(error_msg)
448487

0 commit comments

Comments
 (0)