add mariadb tests for MR and reorganize some fixtures#401
Merged
dbasunag merged 5 commits intoopendatahub-io:mainfrom Jul 11, 2025
Merged
add mariadb tests for MR and reorganize some fixtures#401dbasunag merged 5 commits intoopendatahub-io:mainfrom
dbasunag merged 5 commits intoopendatahub-io:mainfrom
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis update introduces new utilities and fixtures for managing MariaDB and its operator within OpenShift-based tests, centralizes constants, and refactors Model Registry test fixtures to streamline resource setup and dependencies. It adds comprehensive MariaDB integration tests, updates test fixture usage across Model Registry test modules, and enhances test data constants. Changes
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/lgtm', '/hold', '/cherry-pick', '/build-push-pr-image', '/wip', '/verified'} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
utilities/mariadb_utils.py (1)
13-20: Simplify the list comprehension.The static analysis tool correctly identified an unnecessary comprehension. The current code can be simplified.
- def _get_mariadb_pods() -> list[Pod]: - _pods = [ - _pod - for _pod in Pod.get( - dyn_client=client, - namespace=mariadb.namespace, - label_selector=f"app.kubernetes.io/instance={mariadb.name}", - ) - ] - return _pods + def _get_mariadb_pods() -> list[Pod]: + return list(Pod.get( + dyn_client=client, + namespace=mariadb.namespace, + label_selector=f"app.kubernetes.io/instance={mariadb.name}", + ))tests/model_registry/conftest.py (1)
268-277: Consider the trade-off between dependency reduction and code duplicationThe rewritten fixture removes the dependency on
model_registry_instance_servicebut duplicates the service lookup logic. While this improves test isolation, it could lead to maintenance issues if the lookup logic needs to change.If you want to avoid duplication while keeping the fixtures independent, consider extracting the common logic:
def _get_model_registry_service(admin_client: DynamicClient, namespace: str, instance_name: str) -> Service: """Helper to get model registry service without fixture dependency.""" return get_mr_service_by_label( client=admin_client, namespace_name=namespace, mr_instance=ModelRegistry(name=instance_name, namespace=namespace), )Then use it in both fixtures to maintain DRY principles.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
tests/conftest.py(4 hunks)tests/model_explainability/trustyai_service/conftest.py(1 hunks)tests/model_registry/conftest.py(4 hunks)tests/model_registry/image_validation/test_verify_rhoai_images.py(2 hunks)tests/model_registry/negative_tests/test_model_registry_creation_negative.py(3 hunks)tests/model_registry/python_client/test_model_registry_creation.py(1 hunks)tests/model_registry/rbac/conftest.py(1 hunks)tests/model_registry/rbac/test_mr_rbac.py(1 hunks)tests/model_registry/rbac/test_mr_rbac_sa.py(2 hunks)tests/model_registry/rest_api/conftest.py(2 hunks)tests/model_registry/rest_api/constants.py(1 hunks)tests/model_registry/rest_api/mariadb/conftest.py(1 hunks)tests/model_registry/rest_api/mariadb/test_mr_mariadb.py(1 hunks)tests/model_registry/rest_api/test_model_registry_rest_api.py(3 hunks)tests/model_registry/scc/test_model_registry_scc.py(3 hunks)tests/model_registry/upgrade/test_model_registry_upgrade.py(2 hunks)utilities/constants.py(1 hunks)utilities/mariadb_utils.py(1 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
tests/model_registry/rbac/conftest.py (7)
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:212-224
Timestamp: 2025-06-16T13:00:51.478Z
Learning: In the opendatahub-tests repository, the OAuth cluster configuration's identityProviders field should not be empty/None and doesn't require defensive programming checks when concatenating with lists.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: jiripetrlik
PR: opendatahub-io/opendatahub-tests#335
File: tests/rag/test_rag.py:0-0
Timestamp: 2025-06-19T14:32:17.658Z
Learning: The `wait_for_replicas()` method from ocp_resources.deployment.Deployment raises an exception when timeout is reached rather than returning a boolean value, so it doesn't need explicit return value checking in tests.
Learnt from: jiripetrlik
PR: opendatahub-io/opendatahub-tests#335
File: tests/rag/test_rag.py:0-0
Timestamp: 2025-06-19T14:32:17.658Z
Learning: The `wait_for_replicas()` method from ocp_resources.deployment.Deployment raises an exception when timeout is reached rather than returning a boolean value, so it doesn't need explicit return value checking in tests.
Learnt from: israel-hdez
PR: opendatahub-io/opendatahub-tests#346
File: tests/model_serving/model_server/inference_graph/conftest.py:85-92
Timestamp: 2025-06-11T16:40:11.593Z
Learning: The helper `create_isvc` (used in tests/model_serving utilities) already waits until the created InferenceService reports Condition READY=True before returning, so additional readiness waits in fixtures are unnecessary.
tests/model_registry/rbac/test_mr_rbac.py (6)
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/utils.py:40-44
Timestamp: 2025-06-18T06:42:37.705Z
Learning: In tests/model_registry/rest_api/utils.py, the user prefers to keep the default value of verify=False in execute_model_registry_post_command to maintain backward compatibility, rather than changing it to True for security reasons.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:200-216
Timestamp: 2025-06-05T14:32:40.247Z
Learning: In the opendatahub-tests repository, the test fixtures should raise exceptions on cleanup failures rather than just logging warnings. The user fege prefers strict cleanup behavior where tests fail if cleanup doesn't work properly, rather than silently continuing.
tests/model_registry/image_validation/test_verify_rhoai_images.py (5)
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/utils.py:40-44
Timestamp: 2025-06-18T06:42:37.705Z
Learning: In tests/model_registry/rest_api/utils.py, the user prefers to keep the default value of verify=False in execute_model_registry_post_command to maintain backward compatibility, rather than changing it to True for security reasons.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:257-264
Timestamp: 2025-06-18T08:27:21.114Z
Learning: In tests/model_registry/rest_api/conftest.py, the user fege prefers using next() without a default value to raise StopIteration when the mysql container is not found, rather than gracefully handling it with None checks. This is a deliberate "fail fast" design choice.
tests/model_registry/python_client/test_model_registry_creation.py (5)
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:257-264
Timestamp: 2025-06-18T08:27:21.114Z
Learning: In tests/model_registry/rest_api/conftest.py, the user fege prefers using next() without a default value to raise StopIteration when the mysql container is not found, rather than gracefully handling it with None checks. This is a deliberate "fail fast" design choice.
tests/model_registry/scc/test_model_registry_scc.py (7)
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME, making it safe to use in directory operations.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/utils.py:40-44
Timestamp: 2025-06-18T06:42:37.705Z
Learning: In tests/model_registry/rest_api/utils.py, the user prefers to keep the default value of verify=False in execute_model_registry_post_command to maintain backward compatibility, rather than changing it to True for security reasons.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
tests/model_registry/rbac/test_mr_rbac_sa.py (6)
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/utils.py:40-44
Timestamp: 2025-06-18T06:42:37.705Z
Learning: In tests/model_registry/rest_api/utils.py, the user prefers to keep the default value of verify=False in execute_model_registry_post_command to maintain backward compatibility, rather than changing it to True for security reasons.
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#296
File: tests/model_registry/rbac/test_mr_rbac_sa.py:0-0
Timestamp: 2025-05-07T10:40:15.681Z
Learning: The `build_mr_client_args` function in the Model Registry RBAC tests uses simple string splitting (`rest_endpoint.split(":")`) for endpoint parsing by design. This implementation is intentionally non-robust to serve as a detection mechanism for any unexpected changes to the endpoint format.
tests/model_registry/upgrade/test_model_registry_upgrade.py (4)
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/utils.py:40-44
Timestamp: 2025-06-18T06:42:37.705Z
Learning: In tests/model_registry/rest_api/utils.py, the user prefers to keep the default value of verify=False in execute_model_registry_post_command to maintain backward compatibility, rather than changing it to True for security reasons.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:257-264
Timestamp: 2025-06-18T08:27:21.114Z
Learning: In tests/model_registry/rest_api/conftest.py, the user fege prefers using next() without a default value to raise StopIteration when the mysql container is not found, rather than gracefully handling it with None checks. This is a deliberate "fail fast" design choice.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
tests/model_registry/negative_tests/test_model_registry_creation_negative.py (6)
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/utils.py:40-44
Timestamp: 2025-06-18T06:42:37.705Z
Learning: In tests/model_registry/rest_api/utils.py, the user prefers to keep the default value of verify=False in execute_model_registry_post_command to maintain backward compatibility, rather than changing it to True for security reasons.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:257-264
Timestamp: 2025-06-18T08:27:21.114Z
Learning: In tests/model_registry/rest_api/conftest.py, the user fege prefers using next() without a default value to raise StopIteration when the mysql container is not found, rather than gracefully handling it with None checks. This is a deliberate "fail fast" design choice.
tests/model_registry/rest_api/mariadb/test_mr_mariadb.py (2)
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:257-264
Timestamp: 2025-06-18T08:27:21.114Z
Learning: In tests/model_registry/rest_api/conftest.py, the user fege prefers using next() without a default value to raise StopIteration when the mysql container is not found, rather than gracefully handling it with None checks. This is a deliberate "fail fast" design choice.
utilities/mariadb_utils.py (3)
Learnt from: jiripetrlik
PR: opendatahub-io/opendatahub-tests#335
File: tests/rag/test_rag.py:0-0
Timestamp: 2025-06-19T14:32:17.658Z
Learning: The `wait_for_replicas()` method from ocp_resources.deployment.Deployment raises an exception when timeout is reached rather than returning a boolean value, so it doesn't need explicit return value checking in tests.
Learnt from: jiripetrlik
PR: opendatahub-io/opendatahub-tests#335
File: tests/rag/test_rag.py:0-0
Timestamp: 2025-06-19T14:32:17.658Z
Learning: The `wait_for_replicas()` method from ocp_resources.deployment.Deployment raises an exception when timeout is reached rather than returning a boolean value, so it doesn't need explicit return value checking in tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#296
File: tests/model_registry/conftest.py:438-444
Timestamp: 2025-05-06T23:35:36.174Z
Learning: In the ocp-resources library, when creating Kubernetes resources like Role and RoleBinding, it's preferred to use the `ensure_exists=True` constructor parameter instead of calling `.wait()` after resource creation.
tests/model_explainability/trustyai_service/conftest.py (7)
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: israel-hdez
PR: opendatahub-io/opendatahub-tests#346
File: tests/model_serving/model_server/inference_graph/conftest.py:85-92
Timestamp: 2025-06-11T16:40:11.593Z
Learning: The helper `create_isvc` (used in tests/model_serving utilities) already waits until the created InferenceService reports Condition READY=True before returning, so additional readiness waits in fixtures are unnecessary.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:200-216
Timestamp: 2025-06-05T14:32:40.247Z
Learning: In the opendatahub-tests repository, the test fixtures should raise exceptions on cleanup failures rather than just logging warnings. The user fege prefers strict cleanup behavior where tests fail if cleanup doesn't work properly, rather than silently continuing.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#296
File: tests/model_registry/conftest.py:438-444
Timestamp: 2025-05-06T23:35:36.174Z
Learning: In the ocp-resources library, when creating Kubernetes resources like Role and RoleBinding, it's preferred to use the `ensure_exists=True` constructor parameter instead of calling `.wait()` after resource creation.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
tests/model_registry/rest_api/conftest.py (6)
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/utils.py:40-44
Timestamp: 2025-06-18T06:42:37.705Z
Learning: In tests/model_registry/rest_api/utils.py, the user prefers to keep the default value of verify=False in execute_model_registry_post_command to maintain backward compatibility, rather than changing it to True for security reasons.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:257-264
Timestamp: 2025-06-18T08:27:21.114Z
Learning: In tests/model_registry/rest_api/conftest.py, the user fege prefers using next() without a default value to raise StopIteration when the mysql container is not found, rather than gracefully handling it with None checks. This is a deliberate "fail fast" design choice.
tests/conftest.py (7)
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:200-216
Timestamp: 2025-06-05T14:32:40.247Z
Learning: In the opendatahub-tests repository, the test fixtures should raise exceptions on cleanup failures rather than just logging warnings. The user fege prefers strict cleanup behavior where tests fail if cleanup doesn't work properly, rather than silently continuing.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME, making it safe to use in directory operations.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME="must-gather-collected", making it safe to use in directory operations.
tests/model_registry/rest_api/mariadb/conftest.py (6)
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:257-264
Timestamp: 2025-06-18T08:27:21.114Z
Learning: In tests/model_registry/rest_api/conftest.py, the user fege prefers using next() without a default value to raise StopIteration when the mysql container is not found, rather than gracefully handling it with None checks. This is a deliberate "fail fast" design choice.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
tests/model_registry/conftest.py (8)
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:257-264
Timestamp: 2025-06-18T08:27:21.114Z
Learning: In tests/model_registry/rest_api/conftest.py, the user fege prefers using next() without a default value to raise StopIteration when the mysql container is not found, rather than gracefully handling it with None checks. This is a deliberate "fail fast" design choice.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: israel-hdez
PR: opendatahub-io/opendatahub-tests#346
File: tests/model_serving/model_server/inference_graph/conftest.py:85-92
Timestamp: 2025-06-11T16:40:11.593Z
Learning: The helper `create_isvc` (used in tests/model_serving utilities) already waits until the created InferenceService reports Condition READY=True before returning, so additional readiness waits in fixtures are unnecessary.
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#296
File: tests/model_registry/rbac/test_mr_rbac_sa.py:0-0
Timestamp: 2025-05-07T10:40:15.681Z
Learning: The `build_mr_client_args` function in the Model Registry RBAC tests uses simple string splitting (`rest_endpoint.split(":")`) for endpoint parsing by design. This implementation is intentionally non-robust to serve as a detection mechanism for any unexpected changes to the endpoint format.
tests/model_registry/rest_api/test_model_registry_rest_api.py (11)
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/utils.py:40-44
Timestamp: 2025-06-18T06:42:37.705Z
Learning: In tests/model_registry/rest_api/utils.py, the user prefers to keep the default value of verify=False in execute_model_registry_post_command to maintain backward compatibility, rather than changing it to True for security reasons.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:257-264
Timestamp: 2025-06-18T08:27:21.114Z
Learning: In tests/model_registry/rest_api/conftest.py, the user fege prefers using next() without a default value to raise StopIteration when the mysql container is not found, rather than gracefully handling it with None checks. This is a deliberate "fail fast" design choice.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#296
File: tests/model_registry/rbac/test_mr_rbac_sa.py:0-0
Timestamp: 2025-05-07T10:40:15.681Z
Learning: The `build_mr_client_args` function in the Model Registry RBAC tests uses simple string splitting (`rest_endpoint.split(":")`) for endpoint parsing by design. This implementation is intentionally non-robust to serve as a detection mechanism for any unexpected changes to the endpoint format.
Learnt from: brettmthompson
PR: opendatahub-io/opendatahub-tests#269
File: tests/model_serving/model_server/serverless/test_scale_to_zero.py:0-0
Timestamp: 2025-04-29T00:49:40.918Z
Learning: In test cases for the opendatahub-tests repository, when verifying specific behaviors resulting from a sequence of operations, hard-coded validation values like "configurationGeneration=3" can be appropriate assertions to validate expected states, especially when they represent an invariant that should remain consistent after specific operations.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME, making it safe to use in directory operations.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME="must-gather-collected", making it safe to use in directory operations.
🧬 Code Graph Analysis (11)
tests/model_registry/rbac/conftest.py (1)
tests/model_registry/rbac/utils.py (1)
wait_for_oauth_openshift_deployment(70-88)
tests/model_registry/image_validation/test_verify_rhoai_images.py (1)
tests/model_registry/conftest.py (1)
model_registry_instance_mysql(199-236)
tests/model_registry/scc/test_model_registry_scc.py (1)
utilities/constants.py (1)
DscComponents(152-173)
tests/model_registry/rbac/test_mr_rbac_sa.py (1)
tests/model_registry/rbac/conftest.py (1)
sa_token(60-101)
tests/model_registry/upgrade/test_model_registry_upgrade.py (2)
tests/model_registry/conftest.py (1)
model_registry_instance_mysql(199-236)tests/model_registry/rest_api/utils.py (1)
ModelRegistryV1Alpha1(269-270)
tests/model_registry/rest_api/mariadb/test_mr_mariadb.py (3)
tests/model_registry/rest_api/utils.py (1)
validate_resource_attributes(106-127)utilities/constants.py (2)
DscComponents(152-173)ManagementState(158-160)tests/model_registry/rest_api/conftest.py (2)
registered_model_rest_api(59-69)updated_model_registry_resource(73-100)
utilities/mariadb_utils.py (2)
utilities/constants.py (1)
Timeout(207-217)tests/model_explainability/trustyai_service/conftest.py (1)
mariadb(138-168)
tests/model_explainability/trustyai_service/conftest.py (1)
utilities/constants.py (2)
KServeDeploymentType(6-9)Labels(176-204)
tests/model_registry/rest_api/conftest.py (1)
tests/model_registry/conftest.py (2)
is_model_registry_oauth(482-483)created_mysql_metadata_db(189-195)
tests/model_registry/conftest.py (3)
tests/model_registry/utils.py (2)
get_mr_service_by_label(22-46)get_endpoint_from_mr_service(49-53)tests/conftest.py (1)
admin_client(65-66)utilities/constants.py (1)
Protocols(89-96)
tests/model_registry/rest_api/test_model_registry_rest_api.py (3)
tests/model_registry/rest_api/utils.py (2)
validate_resource_attributes(106-127)ModelRegistryV1Alpha1(269-270)utilities/constants.py (1)
DscComponents(152-173)tests/model_registry/conftest.py (1)
model_registry_instance_mysql(199-236)
🪛 Pylint (3.3.7)
utilities/mariadb_utils.py
[refactor] 13-20: Unnecessary use of a comprehension, use list(Pod.get(dyn_client=client, namespace=mariadb.namespace, label_selector=f'app.kubernetes.io/instance={mariadb.name}')) instead.
(R1721)
tests/conftest.py
[refactor] 629-631: Do not raise StopIteration in generator, use return statement instead
(R1708)
🔇 Additional comments (32)
utilities/constants.py (1)
311-312: LGTM! Good centralization of constants.These constants properly centralize namespace and resource identifiers that were likely hardcoded elsewhere. The naming follows existing conventions and the values are appropriate for OpenShift operator management.
tests/model_registry/rbac/conftest.py (1)
234-234: LGTM! Ensures OAuth deployment readiness after restore.Adding the OAuth deployment wait after configuration restore provides symmetric behavior and ensures the deployment is stable before proceeding. This improves test reliability.
tests/model_registry/rbac/test_mr_rbac.py (1)
45-50: LGTM! Proper integration of OAuth and MySQL fixtures.The addition of these fixtures aligns the RBAC tests with the refactored model registry infrastructure, ensuring proper OAuth configuration and MySQL-backed model registry setup.
tests/model_registry/image_validation/test_verify_rhoai_images.py (2)
31-36: LGTM! Consistent fixture integration.The addition of these fixtures properly aligns the image validation tests with the refactored model registry infrastructure using MySQL and OAuth configuration.
51-51: LGTM! Correct fixture parameter update.The parameter change to
model_registry_instance_mysqlcorrectly reflects the fixture refactoring to use MySQL-backed model registry instances.tests/model_registry/scc/test_model_registry_scc.py (2)
4-4: LGTM! Simplified import statement.The change from aliased import to direct import improves readability without affecting functionality.
71-77: LGTM! Consistent fixture integration.The addition of these fixtures aligns the security context validation tests with the updated model registry infrastructure using MySQL and OAuth configuration, maintaining consistency across the test suite.
tests/model_registry/python_client/test_model_registry_creation.py (1)
45-50: LGTM: Fixture additions align with test infrastructure refactoring.The addition of
created_mysql_metadata_db,model_registry_instance_mysql, andregistered_modelfixtures standardizes the test setup across the model registry test suite, which is consistent with the broader refactoring mentioned in the AI summary.tests/model_registry/rbac/test_mr_rbac_sa.py (2)
29-34: LGTM: Fixture additions support OAuth and MySQL-backed testing.The addition of
is_model_registry_oauth,created_mysql_metadata_db, andmodel_registry_instance_mysqlfixtures aligns with the broader test infrastructure refactoring to support OAuth-enabled and MySQL-backed model registry testing.
46-46: LGTM: Parameter reordering doesn't affect functionality.The reordering of the
sa_tokenparameter is cosmetic and doesn't impact the test logic.tests/model_registry/upgrade/test_model_registry_upgrade.py (1)
47-47: LGTM: Fixture rename improves clarity.The rename from
model_registry_instancetomodel_registry_instance_mysqlmakes the fixture usage more explicit about the MySQL-backed deployment and aligns with the broader test infrastructure refactoring.Also applies to: 59-59, 69-69, 78-78, 82-84
utilities/mariadb_utils.py (1)
11-35: LGTM: Well-designed utility functions for MariaDB operations.The functions provide essential waiting functionality for MariaDB pods and operator deployments. The use of
TimeoutSamplerfor polling and proper condition waiting follows good patterns for Kubernetes resource management.Also applies to: 37-47
tests/model_registry/negative_tests/test_model_registry_creation_negative.py (2)
54-59: LGTM: Fixture additions support negative testing infrastructure.The addition of
model_registry_db_secret_negative_testandmodel_registry_db_deployment_negative_testfixtures provides the necessary database resources for negative testing scenarios.
94-94: LGTM: OAuth proxy configuration aligns with broader refactoring.The change from
ISTIO_CONFIG_DICTtoOAUTH_PROXY_CONFIG_DICTis consistent with the broader OAuth-related refactoring across the model registry test suite.tests/model_explainability/trustyai_service/conftest.py (1)
51-51: LGTM! Import consolidation is correct.The import of
OPENSHIFT_OPERATORSandMARIADBfromutilities.constantsaligns with the centralization of constants and the relocation of MariaDB fixtures totests/conftest.py.tests/model_registry/rest_api/constants.py (1)
44-57: LGTM! Well-structured test constants.The new constants provide comprehensive test data for model registry resource updates and follow the existing pattern in the file. They support the new MariaDB tests and existing test refactoring.
tests/model_registry/rest_api/conftest.py (2)
60-63: LGTM! OAuth parameter addition is appropriate.The addition of
is_model_registry_oauthparameter enables OAuth-aware testing scenarios and aligns with the broader test suite updates.
161-161: LGTM! Parameter consolidation simplifies dependencies.Replacing two separate parameters with
created_mysql_metadata_dbaligns with the fixture refactoring intests/model_registry/conftest.pyand simplifies the dependency chain.tests/model_registry/rest_api/mariadb/test_mr_mariadb.py (5)
27-52: LGTM! Well-structured test class with proper fixture setup.The test class is properly configured with MariaDB-specific fixtures and follows pytest best practices with clear parametrization.
73-83: LGTM! Comprehensive resource validation tests.The parametrized test validates all three model registry resource types (registered model, model version, and model artifact) using the shared validation utility.
118-132: LGTM! Thorough model artifact update tests.The test covers multiple update scenarios for model artifacts including description, format name, and format version updates.
164-178: LGTM! Comprehensive model version update tests.The test validates description updates, state transitions (archived/live), and custom property updates for model versions.
210-224: LGTM! Complete registered model update tests.The test covers all update scenarios for registered models including descriptions, state transitions, and custom properties.
tests/conftest.py (3)
588-619: LGTM! Well-implemented MariaDB operator installation fixture.The fixture properly checks for existing installation, waits for deployment readiness, and ensures cleanup on teardown. The manual install plan approval and specific CSV version provide good control over the operator lifecycle.
636-642: LGTM! Proper CR lifecycle management.The fixture correctly creates the MariaDB operator CR, waits for deployment condition, and ensures operator deployments are ready before yielding.
629-631: Fix StopIteration issue in generator context.Using
next()without a default value in a generator can cause StopIteration to be raised, which is problematic in generator contexts. This should be handled more gracefully.Apply this fix to handle the case when no MariadbOperator is found:
- mariadb_operator_cr_dict: dict[str, Any] = next( - example for example in alm_examples if example["kind"] == "MariadbOperator" - ) + mariadb_operator_cr_dict: dict[str, Any] | None = next( + (example for example in alm_examples if example["kind"] == "MariadbOperator"), None + ) + if not mariadb_operator_cr_dict: + raise ResourceNotFoundError(f"No MariadbOperator dict found in alm_examples for CSV {mariadb_csv.name}")Then remove the duplicate check on lines 633-634:
mariadb_operator_cr_dict["clusterName"] = "1ce302f6-e674-47fa-85be-875ae295c96c" - if not mariadb_operator_cr_dict: - raise ResourceNotFoundError(f"No MariadbOperator dict found in alm_examples for CSV {mariadb_csv.name}")⛔ Skipped due to learnings
Learnt from: fege PR: opendatahub-io/opendatahub-tests#320 File: tests/model_registry/rest_api/conftest.py:257-264 Timestamp: 2025-06-18T08:27:21.114Z Learning: In tests/model_registry/rest_api/conftest.py, the user fege prefers using next() without a default value to raise StopIteration when the mysql container is not found, rather than gracefully handling it with None checks. This is a deliberate "fail fast" design choice.tests/model_registry/rest_api/test_model_registry_rest_api.py (3)
4-19: LGTM!The import refactoring improves code organization by centralizing constants and using the correct
py_configalias consistently.
59-63: LGTM!The fixture additions (
created_mysql_metadata_dbandmodel_registry_instance_mysql) properly integrate with the refactored MySQL-backed model registry setup.
103-123: LGTM!The consistent renaming from
model_registry_instancetomodel_registry_instance_mysqlacross all test methods clearly indicates the use of MySQL-backed instances.tests/model_registry/rest_api/mariadb/conftest.py (2)
1-22: LGTM!The imports and logger setup are appropriate for MariaDB fixture functionality.
53-72: LGTM!The
deployed_mariadbfixture properly manages the MariaDB lifecycle with appropriate cleanup of secrets.tests/model_registry/conftest.py (1)
189-196: LGTM!The new
created_mysql_metadata_dbfixture provides a clean interface to access the MySQL deployment while maintaining proper dependency ordering.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
utilities/mariadb_utils.py (1)
13-20: Consider simplifying the list comprehension.The list comprehension can be simplified as suggested by static analysis.
- _pods = [ - _pod - for _pod in Pod.get( - dyn_client=client, - namespace=mariadb.namespace, - label_selector=f"app.kubernetes.io/instance={mariadb.name}", - ) - ] + _pods = list(Pod.get( + dyn_client=client, + namespace=mariadb.namespace, + label_selector=f"app.kubernetes.io/instance={mariadb.name}", + ))tests/conftest.py (1)
632-632: Consider making the clusterName configurable.The hardcoded UUID for
clusterNamemight be intentional for testing, but consider whether this should be parameterizable or generated dynamically.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
tests/conftest.py(4 hunks)tests/model_explainability/trustyai_service/conftest.py(1 hunks)tests/model_registry/conftest.py(4 hunks)tests/model_registry/image_validation/test_verify_rhoai_images.py(2 hunks)tests/model_registry/negative_tests/test_model_registry_creation_negative.py(3 hunks)tests/model_registry/python_client/test_model_registry_creation.py(1 hunks)tests/model_registry/rbac/conftest.py(1 hunks)tests/model_registry/rbac/test_mr_rbac.py(1 hunks)tests/model_registry/rbac/test_mr_rbac_sa.py(2 hunks)tests/model_registry/rest_api/conftest.py(2 hunks)tests/model_registry/rest_api/constants.py(1 hunks)tests/model_registry/rest_api/mariadb/conftest.py(1 hunks)tests/model_registry/rest_api/mariadb/test_mr_mariadb.py(1 hunks)tests/model_registry/rest_api/test_model_registry_rest_api.py(3 hunks)tests/model_registry/scc/test_model_registry_scc.py(3 hunks)tests/model_registry/upgrade/test_model_registry_upgrade.py(2 hunks)utilities/constants.py(1 hunks)utilities/mariadb_utils.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/model_registry/rest_api/constants.py
🚧 Files skipped from review as they are similar to previous changes (14)
- utilities/constants.py
- tests/model_registry/rbac/conftest.py
- tests/model_registry/rbac/test_mr_rbac.py
- tests/model_registry/scc/test_model_registry_scc.py
- tests/model_registry/python_client/test_model_registry_creation.py
- tests/model_registry/rbac/test_mr_rbac_sa.py
- tests/model_registry/image_validation/test_verify_rhoai_images.py
- tests/model_registry/negative_tests/test_model_registry_creation_negative.py
- tests/model_registry/upgrade/test_model_registry_upgrade.py
- tests/model_registry/rest_api/mariadb/test_mr_mariadb.py
- tests/model_registry/rest_api/test_model_registry_rest_api.py
- tests/model_registry/rest_api/conftest.py
- tests/model_registry/rest_api/mariadb/conftest.py
- tests/model_registry/conftest.py
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#401
File: tests/model_registry/rest_api/mariadb/conftest.py:89-110
Timestamp: 2025-07-04T00:17:47.777Z
Learning: In tests/model_registry/rest_api/mariadb/conftest.py, the model_registry_with_mariadb fixture should always use OAUTH_PROXY_CONFIG_DICT for the oauth_proxy parameter regardless of the is_model_registry_oauth parameter value, based on expected product behavior for MariaDB-backed ModelRegistry instances.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
tests/model_explainability/trustyai_service/conftest.py (7)
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#401
File: tests/model_registry/rest_api/mariadb/conftest.py:89-110
Timestamp: 2025-07-04T00:17:47.777Z
Learning: In tests/model_registry/rest_api/mariadb/conftest.py, the model_registry_with_mariadb fixture should always use OAUTH_PROXY_CONFIG_DICT for the oauth_proxy parameter regardless of the is_model_registry_oauth parameter value, based on expected product behavior for MariaDB-backed ModelRegistry instances.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: israel-hdez
PR: opendatahub-io/opendatahub-tests#346
File: tests/model_serving/model_server/inference_graph/conftest.py:85-92
Timestamp: 2025-06-11T16:40:11.593Z
Learning: The helper `create_isvc` (used in tests/model_serving utilities) already waits until the created InferenceService reports Condition READY=True before returning, so additional readiness waits in fixtures are unnecessary.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#296
File: tests/model_registry/conftest.py:438-444
Timestamp: 2025-05-06T23:35:36.174Z
Learning: In the ocp-resources library, when creating Kubernetes resources like Role and RoleBinding, it's preferred to use the `ensure_exists=True` constructor parameter instead of calling `.wait()` after resource creation.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
tests/conftest.py (8)
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#401
File: tests/model_registry/rest_api/mariadb/conftest.py:89-110
Timestamp: 2025-07-04T00:17:47.777Z
Learning: In tests/model_registry/rest_api/mariadb/conftest.py, the model_registry_with_mariadb fixture should always use OAUTH_PROXY_CONFIG_DICT for the oauth_proxy parameter regardless of the is_model_registry_oauth parameter value, based on expected product behavior for MariaDB-backed ModelRegistry instances.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:200-216
Timestamp: 2025-06-05T14:32:40.247Z
Learning: In the opendatahub-tests repository, the test fixtures should raise exceptions on cleanup failures rather than just logging warnings. The user fege prefers strict cleanup behavior where tests fail if cleanup doesn't work properly, rather than silently continuing.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME, making it safe to use in directory operations.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME="must-gather-collected", making it safe to use in directory operations.
utilities/mariadb_utils.py (3)
Learnt from: jiripetrlik
PR: opendatahub-io/opendatahub-tests#335
File: tests/rag/test_rag.py:0-0
Timestamp: 2025-06-19T14:32:17.658Z
Learning: The `wait_for_replicas()` method from ocp_resources.deployment.Deployment raises an exception when timeout is reached rather than returning a boolean value, so it doesn't need explicit return value checking in tests.
Learnt from: jiripetrlik
PR: opendatahub-io/opendatahub-tests#335
File: tests/rag/test_rag.py:0-0
Timestamp: 2025-06-19T14:32:17.658Z
Learning: The `wait_for_replicas()` method from ocp_resources.deployment.Deployment raises an exception when timeout is reached rather than returning a boolean value, so it doesn't need explicit return value checking in tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#296
File: tests/model_registry/conftest.py:438-444
Timestamp: 2025-05-06T23:35:36.174Z
Learning: In the ocp-resources library, when creating Kubernetes resources like Role and RoleBinding, it's preferred to use the `ensure_exists=True` constructor parameter instead of calling `.wait()` after resource creation.
🧬 Code Graph Analysis (1)
tests/model_explainability/trustyai_service/conftest.py (1)
utilities/constants.py (2)
KServeDeploymentType(6-9)Labels(176-204)
🪛 Pylint (3.3.7)
tests/conftest.py
[refactor] 629-631: Do not raise StopIteration in generator, use return statement instead
(R1708)
utilities/mariadb_utils.py
[refactor] 13-20: Unnecessary use of a comprehension, use list(Pod.get(dyn_client=client, namespace=mariadb.namespace, label_selector=f'app.kubernetes.io/instance={mariadb.name}')) instead.
(R1721)
🔇 Additional comments (5)
utilities/mariadb_utils.py (2)
11-35: Well-implemented function for waiting on MariaDB pods.The function correctly uses TimeoutSampler to wait for pods to appear, then ensures each pod reaches Ready condition. The logic is sound and follows good practices.
37-47: LGTM - Proper use of wait_for_replicas method.The function correctly waits for all expected MariaDB operator deployments to have ready replicas. Based on retrieved learnings, the
wait_for_replicas()method appropriately raises exceptions on timeout.tests/model_explainability/trustyai_service/conftest.py (1)
51-51: Good centralization of constants.Moving constants to
utilities.constantsimproves maintainability and reduces duplication across the test suite.tests/conftest.py (2)
588-618: Well-structured MariaDB operator installation fixture.The fixture properly handles operator installation with manual approval, waits for deployments to be ready, and includes appropriate cleanup. The session scope is appropriate for operator lifecycle management.
637-642: Excellent use of condition waiting and deployment verification.The fixture properly waits for the MariadbOperator to reach "Deployed" condition and ensures all related deployments are ready using the utility function. This follows best practices for resource readiness verification.
lugi0
reviewed
Jul 4, 2025
fege
reviewed
Jul 4, 2025
fege
reviewed
Jul 4, 2025
dbasunag
force-pushed
the
cp_maria
branch
2 times, most recently
from
17feeaf to
8bde25e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/conftest.py (1)
629-634: Fix potential StopIteration and logic ordering issues.This issue matches a previous review comment. The
next()call can raiseStopIterationif no MariadbOperator is found, and the existence check happens after theclusterNameassignment.
🧹 Nitpick comments (1)
tests/conftest.py (1)
635-635: Extract hardcoded clusterName to a constant.The hardcoded UUID
"1ce302f6-e674-47fa-85be-875ae295c96c"should be extracted to a constant for better maintainability.+MARIADB_CLUSTER_NAME = "1ce302f6-e674-47fa-85be-875ae295c96c" + @pytest.fixture(scope="class") def mariadb_operator_cr( admin_client: DynamicClient, installed_mariadb_operator: None ) -> Generator[MariadbOperator, Any, Any]: # ... existing code ... - mariadb_operator_cr_dict["clusterName"] = "1ce302f6-e674-47fa-85be-875ae295c96c" + mariadb_operator_cr_dict["clusterName"] = MARIADB_CLUSTER_NAME
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
tests/conftest.py(4 hunks)tests/model_explainability/trustyai_service/conftest.py(1 hunks)tests/model_registry/conftest.py(3 hunks)tests/model_registry/image_validation/test_verify_rhoai_images.py(2 hunks)tests/model_registry/negative_tests/test_model_registry_creation_negative.py(3 hunks)tests/model_registry/python_client/test_model_registry_creation.py(1 hunks)tests/model_registry/rbac/conftest.py(1 hunks)tests/model_registry/rbac/test_mr_rbac.py(1 hunks)tests/model_registry/rbac/test_mr_rbac_sa.py(2 hunks)tests/model_registry/rest_api/conftest.py(2 hunks)tests/model_registry/rest_api/constants.py(1 hunks)tests/model_registry/rest_api/mariadb/conftest.py(1 hunks)tests/model_registry/rest_api/mariadb/test_mr_mariadb.py(1 hunks)tests/model_registry/rest_api/test_model_registry_rest_api.py(3 hunks)tests/model_registry/scc/test_model_registry_scc.py(3 hunks)tests/model_registry/upgrade/test_model_registry_upgrade.py(2 hunks)utilities/constants.py(1 hunks)utilities/mariadb_utils.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- utilities/constants.py
- tests/model_registry/python_client/test_model_registry_creation.py
- tests/model_registry/image_validation/test_verify_rhoai_images.py
- tests/model_registry/scc/test_model_registry_scc.py
- tests/model_registry/rbac/conftest.py
- tests/model_registry/upgrade/test_model_registry_upgrade.py
- utilities/mariadb_utils.py
- tests/model_registry/rbac/test_mr_rbac.py
- tests/model_registry/rbac/test_mr_rbac_sa.py
- tests/model_registry/rest_api/constants.py
- tests/model_registry/rest_api/conftest.py
- tests/model_registry/rest_api/mariadb/test_mr_mariadb.py
- tests/model_registry/rest_api/mariadb/conftest.py
- tests/model_registry/rest_api/test_model_registry_rest_api.py
- tests/model_registry/negative_tests/test_model_registry_creation_negative.py
- tests/model_registry/conftest.py
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#401
File: tests/model_registry/rest_api/mariadb/conftest.py:89-110
Timestamp: 2025-07-04T00:17:47.799Z
Learning: In tests/model_registry/rest_api/mariadb/conftest.py, the model_registry_with_mariadb fixture should always use OAUTH_PROXY_CONFIG_DICT for the oauth_proxy parameter regardless of the is_model_registry_oauth parameter value, based on expected product behavior for MariaDB-backed ModelRegistry instances.
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
tests/model_explainability/trustyai_service/conftest.py (8)
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#401
File: tests/model_registry/rest_api/mariadb/conftest.py:89-110
Timestamp: 2025-07-04T00:17:47.799Z
Learning: In tests/model_registry/rest_api/mariadb/conftest.py, the model_registry_with_mariadb fixture should always use OAUTH_PROXY_CONFIG_DICT for the oauth_proxy parameter regardless of the is_model_registry_oauth parameter value, based on expected product behavior for MariaDB-backed ModelRegistry instances.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: israel-hdez
PR: opendatahub-io/opendatahub-tests#346
File: tests/model_serving/model_server/inference_graph/conftest.py:85-92
Timestamp: 2025-06-11T16:40:11.593Z
Learning: The helper `create_isvc` (used in tests/model_serving utilities) already waits until the created InferenceService reports Condition READY=True before returning, so additional readiness waits in fixtures are unnecessary.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:200-216
Timestamp: 2025-06-05T14:32:40.247Z
Learning: In the opendatahub-tests repository, the test fixtures should raise exceptions on cleanup failures rather than just logging warnings. The user fege prefers strict cleanup behavior where tests fail if cleanup doesn't work properly, rather than silently continuing.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#296
File: tests/model_registry/conftest.py:438-444
Timestamp: 2025-05-06T23:35:36.174Z
Learning: In the ocp-resources library, when creating Kubernetes resources like Role and RoleBinding, it's preferred to use the `ensure_exists=True` constructor parameter instead of calling `.wait()` after resource creation.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/test_mr_rbac.py:64-77
Timestamp: 2025-06-16T11:26:53.789Z
Learning: In Model Registry RBAC tests, client instantiation tests are designed to verify the ability to create and use the MR python client, with actual API functionality testing covered by separate existing tests.
tests/conftest.py (9)
Learnt from: adolfo-ab
PR: opendatahub-io/opendatahub-tests#334
File: tests/model_explainability/trustyai_service/test_trustyai_service.py:52-65
Timestamp: 2025-06-05T10:05:17.642Z
Learning: For TrustyAI image validation tests: operator image tests require admin_client, related_images_refs, and trustyai_operator_configmap fixtures, while service image tests would require different fixtures like trustyai_service_with_pvc_storage, model_namespace, and current_client_token.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#401
File: tests/model_registry/rest_api/mariadb/conftest.py:89-110
Timestamp: 2025-07-04T00:17:47.799Z
Learning: In tests/model_registry/rest_api/mariadb/conftest.py, the model_registry_with_mariadb fixture should always use OAUTH_PROXY_CONFIG_DICT for the oauth_proxy parameter regardless of the is_model_registry_oauth parameter value, based on expected product behavior for MariaDB-backed ModelRegistry instances.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:355-371
Timestamp: 2025-06-16T11:25:51.461Z
Learning: In the opendatahub-tests repository, the login_as_test_user fixture in tests/model_registry/rbac/conftest.py intentionally logs back in as the original admin user without providing a password using login_with_user_password. This behavior is acceptable in their testing environment.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#338
File: tests/model_registry/rbac/test_mr_rbac.py:24-53
Timestamp: 2025-06-06T12:22:57.057Z
Learning: In the opendatahub-tests repository, prefer keeping test parameterization configurations inline rather than extracting them to separate variables/constants, as it makes triaging easier by avoiding the need to jump between different parts of the file to understand the test setup.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:200-216
Timestamp: 2025-06-05T14:32:40.247Z
Learning: In the opendatahub-tests repository, the test fixtures should raise exceptions on cleanup failures rather than just logging warnings. The user fege prefers strict cleanup behavior where tests fail if cleanup doesn't work properly, rather than silently continuing.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#354
File: tests/model_registry/rbac/conftest.py:166-175
Timestamp: 2025-06-16T11:25:39.599Z
Learning: In tests/model_registry/rbac/conftest.py, predictable names are intentionally used for test resources (like RoleBindings and groups) instead of random names. This design choice prioritizes exposing cleanup failures from previous test runs through name collisions rather than masking such issues with random names. The philosophy is that test failures should be observable and informative to help debug underlying infrastructure or cleanup issues.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#320
File: tests/model_registry/rest_api/conftest.py:257-264
Timestamp: 2025-06-18T08:27:21.114Z
Learning: In tests/model_registry/rest_api/conftest.py, the user fege prefers using next() without a default value to raise StopIteration when the mysql container is not found, rather than gracefully handling it with None checks. This is a deliberate "fail fast" design choice.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME, making it safe to use in directory operations.
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#240
File: conftest.py:328-335
Timestamp: 2025-05-13T17:27:26.289Z
Learning: In the opendatahub-tests codebase, py_config["must_gather_collector"]["must_gather_base_directory"] is never empty as it's set during pytest_sessionstart() by combining get_base_dir() with BASE_DIRECTORY_NAME="must-gather-collected", making it safe to use in directory operations.
🧬 Code Graph Analysis (1)
tests/model_explainability/trustyai_service/conftest.py (1)
utilities/constants.py (2)
KServeDeploymentType(6-9)Labels(178-206)
🔇 Additional comments (4)
tests/model_explainability/trustyai_service/conftest.py (1)
51-51: LGTM! Good consolidation of constants.Centralizing the
OPENSHIFT_OPERATORSandMARIADBconstants inutilities.constantsimproves maintainability and reduces duplication.tests/conftest.py (3)
13-13: LGTM! Necessary imports for MariaDB operator support.All the new imports are properly utilized in the MariaDB operator fixtures and centralized constant imports.
Also applies to: 15-15, 17-17, 19-19, 24-24, 35-35, 52-53, 57-57, 59-59
588-618: LGTM! Well-structured MariaDB operator installation fixture.The session-scoped fixture properly handles operator installation, deployment readiness checks, and cleanup. The logic is sound and follows best practices.
636-641: LGTM! Proper CR lifecycle management.The fixture correctly creates the MariadbOperator custom resource, waits for deployment conditions, and ensures operator deployments are ready before yielding.
adolfo-ab
reviewed
Jul 8, 2025
Collaborator
Author
|
/build-push-pr-image |
|
Status of building tag pr-401: success. |
Collaborator
Author
|
/verified |
Collaborator
Author
|
/build-push-pr-image |
for more information, see https://pre-commit.ci
|
Status of building tag pr-401: success. |
lugi0
approved these changes
Jul 11, 2025
|
Status of building tag latest: success. |
This was referenced Jul 21, 2025
This was referenced Jul 29, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
How Has This Been Tested?
Merge criteria: