Use exitStack to support multiple resource creations#420
Closed
dbasunag wants to merge 2 commits intoopendatahub-io:mainfrom
Closed
Use exitStack to support multiple resource creations#420dbasunag wants to merge 2 commits intoopendatahub-io:mainfrom
dbasunag wants to merge 2 commits intoopendatahub-io:mainfrom
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change refactors Model Registry test infrastructure to support multiple resource instances in parallel. Fixtures, utility functions, and test code are updated to handle lists of resources rather than single objects. New utility functions enable programmatic creation of multiple Kubernetes resources, and test logic is adjusted accordingly throughout the suite. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ 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{'/build-push-pr-image', '/lgtm', '/verified', '/cherry-pick', '/wip', '/hold'} |
![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: 5
🔭 Outside diff range comments (1)
tests/model_registry/conftest.py (1)
193-195: Return type inconsistency in post-upgrade path.The post-upgrade path yields a single
ModelRegistryinstance but the return type annotation indicateslist[Any]. This should yield a list for consistency.- mr_instance = ModelRegistry(name=MR_INSTANCE_NAME, namespace=model_registry_namespace, ensure_exists=True) - yield mr_instance - mr_instance.delete(wait=True) + mr_instance = ModelRegistry(name=MR_INSTANCE_NAME, namespace=model_registry_namespace, ensure_exists=True) + yield [mr_instance] + mr_instance.delete(wait=True)
🧹 Nitpick comments (5)
tests/model_registry/rest_api/test_multple_mr.py (1)
16-16: Consider making NUM_RESOURCES configurable.The hardcoded
NUM_RESOURCES = {"num": 3}could be made configurable through pytest parameters or environment variables to allow different test scenarios without code changes.-NUM_RESOURCES = {"num": 3} +NUM_RESOURCES = {"num": int(os.environ.get("MR_TEST_INSTANCES", "3"))}Don't forget to add the import:
+import ostests/model_registry/rbac/test_mr_rbac_sa.py (2)
41-42: Consider adding bounds checking for list access.The parameter type change from single string to list is consistent with the multi-instance refactor. However, accessing
[0]without bounds checking could fail if the list is empty.Consider adding a check like:
+ if not model_registry_instance_rest_endpoint: + pytest.fail("No model registry endpoints available") rest_endpoint = model_registry_instance_rest_endpoint[0]Also applies to: 72-73
78-78: Fix logging inconsistency.Line 78 logs the entire list while other log statements (line 48) only log the first element. This creates inconsistent log output.
- LOGGER.info(f"Targeting Model Registry REST endpoint: {model_registry_instance_rest_endpoint}") + LOGGER.info(f"Targeting Model Registry REST endpoint: {model_registry_instance_rest_endpoint[0]}")tests/model_registry/upgrade/test_model_registry_upgrade.py (1)
38-38: Consider adding defensive checks for list access.Similar to other files, accessing
[0]without bounds checking could fail if the lists are empty. While this is unlikely in the test context, adding defensive checks would make the code more robust.Consider adding checks like:
assert model_registry_client, "No model registry clients available" assert model_registry_instance, "No model registry instances available"Also applies to: 56-56, 68-68, 77-77, 88-88
tests/model_registry/conftest.py (1)
401-402: Document the behavior of using only the first client.The fixture receives a list of clients but only uses the first one. Consider adding a comment to clarify this is intentional.
def registered_model(request: FixtureRequest, model_registry_client: list[ModelRegistryClient]) -> RegisteredModel: + # Use the first client from the list for single model registration return model_registry_client[0].register_model(
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
tests/model_registry/conftest.py(9 hunks)tests/model_registry/constants.py(3 hunks)tests/model_registry/image_validation/test_verify_rhoai_images.py(2 hunks)tests/model_registry/negative_tests/test_db_migration.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(3 hunks)tests/model_registry/rbac/conftest.py(0 hunks)tests/model_registry/rbac/test_mr_rbac.py(6 hunks)tests/model_registry/rbac/test_mr_rbac_sa.py(3 hunks)tests/model_registry/rest_api/conftest.py(11 hunks)tests/model_registry/rest_api/constants.py(1 hunks)tests/model_registry/rest_api/test_model_registry_rest_api.py(1 hunks)tests/model_registry/rest_api/test_model_registry_secure_db.py(4 hunks)tests/model_registry/rest_api/test_multple_mr.py(1 hunks)tests/model_registry/rest_api/utils.py(3 hunks)tests/model_registry/upgrade/test_model_registry_upgrade.py(3 hunks)tests/model_registry/utils.py(2 hunks)
💤 Files with no reviewable changes (1)
- tests/model_registry/rbac/conftest.py
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
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#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: fege
PR: opendatahub-io/opendatahub-tests#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
tests/model_registry/negative_tests/test_model_registry_creation_negative.py (7)
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#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: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#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
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: 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/rest_api/utils.py (5)
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#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: 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#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
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.
tests/model_registry/negative_tests/test_db_migration.py (4)
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#296
File: tests/model_registry/rbac/test_mr_rbac_sa.py:1-4
Timestamp: 2025-05-06T23:36:04.545Z
Learning: The opendatahub-tests repository is pinned to Python 3.12, which supports `typing.Self` natively, so there's no need for backward compatibility imports from typing_extensions.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
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/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/image_validation/test_verify_rhoai_images.py (4)
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/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#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
tests/model_registry/python_client/test_model_registry_creation.py (5)
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: 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: 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: fege
PR: opendatahub-io/opendatahub-tests#283
File: tests/model_registry/rbac/utils.py:31-32
Timestamp: 2025-05-22T14:14:45.746Z
Learning: In the OpenDataHub tests codebase, the ModelRegistryClient accepts the port parameter as a string, and explicit conversion to integer is not required.
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.
tests/model_registry/rbac/test_mr_rbac.py (9)
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#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#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: 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: 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: 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/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: fege
PR: opendatahub-io/opendatahub-tests#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
tests/model_registry/rest_api/test_model_registry_secure_db.py (10)
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/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: 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#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#283
File: tests/model_registry/test_user_permission.py:31-31
Timestamp: 2025-05-05T13:48:02.409Z
Learning: In the opendatahub-tests repository, bypassing TLS verification with `--insecure-skip-tls-verify=true` is an acceptable practice in test environments, particularly in the model registry tests.
Learnt from: fege
PR: opendatahub-io/opendatahub-tests#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
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/constants.py (5)
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#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: 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.
tests/model_registry/rest_api/constants.py (1)
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.
tests/model_registry/rest_api/test_multple_mr.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#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: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/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: 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#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
tests/model_registry/rest_api/test_model_registry_rest_api.py (7)
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#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: 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: 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: 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#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
tests/model_registry/rbac/test_mr_rbac_sa.py (7)
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: 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#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: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: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/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: 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/utils.py (5)
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#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: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: 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 (5)
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#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: 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#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
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/rest_api/conftest.py (8)
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: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.
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/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#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
tests/model_registry/conftest.py (9)
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: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: 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/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: 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: fege
PR: opendatahub-io/opendatahub-tests#409
File: tests/model_registry/negative_tests/test_db_migration.py:47-57
Timestamp: 2025-07-09T08:50:22.172Z
Learning: In negative tests for model registry database migration failures (like test_db_migration_negative), the model registry pod is expected to be in a failed state (not Running) when the database is dirty. The test fetches logs from the failed pod to verify the expected error message about dirty database version, so waiting for Running status would be inappropriate.
🧬 Code Graph Analysis (10)
tests/model_registry/negative_tests/test_db_migration.py (1)
tests/model_registry/conftest.py (1)
model_registry_instance(179-227)
tests/model_registry/image_validation/test_verify_rhoai_images.py (1)
tests/model_registry/conftest.py (1)
model_registry_instance(179-227)
tests/model_registry/python_client/test_model_registry_creation.py (1)
tests/model_registry/conftest.py (4)
model_registry_client(373-397)registered_model(401-412)model_registry_rest_url(498-500)model_registry_rest_headers(504-509)
tests/model_registry/rbac/test_mr_rbac.py (4)
tests/model_registry/rbac/conftest.py (2)
test_idp_user(236-273)login_as_test_user(356-370)tests/model_registry/conftest.py (1)
model_registry_instance_rest_endpoint(254-264)tests/model_registry/rbac/utils.py (1)
build_mr_client_args(17-38)utilities/infra.py (1)
get_openshift_token(596-604)
tests/model_registry/constants.py (1)
utilities/constants.py (1)
ModelFormat(12-19)
tests/model_registry/rest_api/test_multple_mr.py (3)
tests/model_registry/rest_api/utils.py (3)
validate_resource_attributes(91-112)get_register_model_data(254-262)register_model_rest_api(55-88)utilities/constants.py (2)
DscComponents(154-175)ManagementState(160-162)tests/model_registry/conftest.py (4)
model_registry_instance(179-227)model_registry_namespace(58-59)model_registry_rest_url(498-500)model_registry_rest_headers(504-509)
tests/model_registry/rbac/test_mr_rbac_sa.py (3)
tests/model_registry/conftest.py (1)
model_registry_instance_rest_endpoint(254-264)tests/model_registry/rbac/utils.py (1)
build_mr_client_args(17-38)tests/model_registry/rbac/conftest.py (1)
sa_token(59-100)
tests/model_registry/utils.py (3)
tests/model_registry/exceptions.py (1)
ModelRegistryResourceNotFoundError(5-6)tests/model_registry/rest_api/utils.py (1)
ModelRegistryV1Alpha1(265-266)utilities/constants.py (2)
Annotations(128-147)KubernetesIo(129-133)
tests/model_registry/upgrade/test_model_registry_upgrade.py (3)
tests/model_registry/conftest.py (3)
model_registry_client(373-397)registered_model(401-412)model_registry_instance(179-227)tests/model_registry/utils.py (1)
get_and_validate_registered_model(320-346)tests/model_registry/rest_api/utils.py (1)
ModelRegistryV1Alpha1(265-266)
tests/model_registry/conftest.py (4)
tests/model_registry/rest_api/utils.py (1)
ModelRegistryV1Alpha1(265-266)utilities/constants.py (2)
Labels(178-206)Protocols(89-96)tests/model_registry/utils.py (8)
get_endpoint_from_mr_service(61-65)get_mr_service_by_label(34-58)wait_for_pods_running(211-244)get_mr_service_objects(376-408)get_mr_pvc_objects(411-428)get_mr_secret_objects(431-448)get_mr_deployment_objects(451-481)get_model_registry_objects(493-528)tests/conftest.py (3)
admin_client(55-56)teardown_resources(76-83)current_client_token(71-72)
🔇 Additional comments (41)
tests/model_registry/utils.py (9)
6-11: LGTM: Import additions are appropriate.The added imports for
Deployment,PersistentVolumeClaim, andSecretare necessary for the new utility functions and align with the multi-instance resource creation approach.
17-23: LGTM: Constants import updated for multi-instance support.The import changes from single-instance constants to base names and the addition of secret-related constants are consistent with the multi-instance refactoring.
376-408: LGTM: Service factory function is well-implemented.The
get_mr_service_objects()function correctly creates MySQL services with appropriate annotations and port configurations. The naming convention usingbase_name+ index supports multi-instance testing.
411-428: LGTM: PVC factory function is correctly implemented.The
get_mr_pvc_objects()function creates PersistentVolumeClaims with appropriate access modes and sizing for MySQL data persistence.
431-448: LGTM: Secret factory function is properly configured.The
get_mr_secret_objects()function uses the imported constants for string data and annotations, ensuring consistency across the codebase.
451-481: LGTM: Deployment factory function is well-structured.The
get_mr_deployment_objects()function correctly references the corresponding secrets and uses the deployment template helper function. The resource lifecycle management with teardown flags is properly implemented.
484-490: LGTM: Standard labels helper provides consistency.The
get_mr_standard_labels()function ensures consistent Kubernetes labeling across all Model Registry resources.
493-528: LGTM: ModelRegistry factory function is comprehensive.The
get_model_registry_objects()function correctly handles both ModelRegistry and ModelRegistryV1Alpha1 classes, configures MySQL connection details, and supports optional SSL configuration parameters.
531-539: LGTM: MySQL configuration helper is well-structured.The
get_mysql_config()function creates the necessary MySQL connection configuration referencing the deployment service and using the constants for database credentials.tests/model_registry/negative_tests/test_model_registry_creation_negative.py (2)
14-16: LGTM: Constants updated for multi-instance support.The import changes from single-instance constants to base names align with the multi-instance refactoring approach throughout the codebase.
68-82: LGTM: Consistent use of base name constants.The test correctly uses the updated base name constants in the MySQL configuration and ModelRegistry resource creation. This maintains consistency with the multi-instance naming convention.
tests/model_registry/negative_tests/test_db_migration.py (2)
2-2: LGTM: Type import updated for multi-instance support.The addition of
Anyto the typing imports is necessary for the updated type annotation.
34-34: Retainmodel_registry_instancefixture for side effects
Themodel_registry_instance: list[Any]parameter is injected solely to invoke its fixture and set up the Model Registry before marking the database dirty and deleting the deployment. Although it isn’t referenced in the test body, its side effects are required for correct test setup. No changes needed.Likely an incorrect or invalid review comment.
tests/model_registry/rest_api/utils.py (3)
1-1: LGTM: Import addition is appropriate.The
copyimport is necessary for the deep copying functionality in the new utility function.
12-12: LGTM: Updated import statement.The import of
MODEL_REGISTER_DATAis necessary for the new utility function.
254-262: LGTM: Model data factory function is well-implemented.The
get_register_model_data()function correctly creates multiple distinct model registration data entries by deep copying the base template and modifying the name and description fields. The use ofcopy.deepcopy()prevents unintended mutations of the original data.tests/model_registry/image_validation/test_verify_rhoai_images.py (1)
2-2: LGTM: Type import updated for multi-instance support.The addition of
Anyto the typing imports is necessary for the updated type annotation.tests/model_registry/rbac/test_mr_rbac.py (2)
45-45: LGTM! Proper fixture dependency management.Good addition of the
model_registry_instancefixture dependency to ensure proper resource lifecycle management alongside the existing fixture.
51-51: Consistent multi-instance refactoring.The changes correctly adapt to the new multi-instance fixture design by:
- Updating parameter type from
strtolist[str]- Accessing the first element
[0]for single-instance test logic- Maintaining backwards compatibility with existing test behavior
The pattern is applied consistently across all test methods in the class.
Also applies to: 58-58, 68-68, 82-82, 93-93, 106-106, 113-113, 125-125
tests/model_registry/constants.py (2)
3-3: LGTM! Cleaned up unused import.Removing the unused
Annotationsimport while keepingModelFormatwhich is still used in line 17.
28-29: Well-designed multi-instance naming convention.The refactoring from fixed names to base names with indexed suffixes supports the multi-instance architecture effectively:
MR_INSTANCE_BASE_NAMEprovides a consistent base for programmatic resource generationMR_INSTANCE_NAMEmaintains backwards compatibility by defaulting to index 0- Same pattern applied consistently to database resource names
This approach enables both single-instance (backwards compatible) and multi-instance testing scenarios.
Also applies to: 39-40
tests/model_registry/python_client/test_model_registry_creation.py (2)
61-61: Consistent multi-instance adaptation.The changes correctly adapt the test to work with the new multi-instance fixture:
- Parameter type updated from
ModelRegistryClienttolist[ModelRegistryClient]- Accessing first element
[0]for existing single-instance test logic- Test behavior remains unchanged
This follows the same pattern established across the test suite.
Also applies to: 64-64
130-130: Proper REST URL list handling.The test correctly adapts to the new
list[str]fixture signature formodel_registry_rest_urland accesses the first element for the existing single-instance test logic.Also applies to: 140-140
tests/model_registry/rest_api/test_model_registry_rest_api.py (1)
100-100: Proper multi-instance fixture adaptation.The changes correctly adapt to the new multi-instance fixture design:
model_registry_instanceis now a list, accessing first element[0]- Test logic remains unchanged - only the way the fixture is accessed has been updated
- Consistent pattern applied across all affected test methods
This maintains backwards compatibility while supporting the new multi-instance architecture.
Also applies to: 107-107, 114-114
tests/model_registry/rest_api/test_multple_mr.py (3)
19-40: Comprehensive multi-instance test parameterization.The parameterization correctly sets up multiple instances for all required fixtures:
- Proper DSC component configuration for managed state
- Consistent NUM_RESOURCES parameter across all fixtures
- Correct use of indirect=True for fixture parameterization
This ensures that 3 instances of each resource type are created consistently.
50-57: Effective multiple instance validation.The test correctly validates multiple Model Registry instances:
- Iterates through the expected number of resources
- Uses proper naming convention with base name + index
- Validates each instance exists using
ensure_exists=True- Includes logging for debugging
This provides good coverage for multi-instance resource creation.
59-74: Comprehensive multi-registry API testing.The test thoroughly validates REST API functionality across multiple registries:
- Generates unique test data for each registry using
get_register_model_data- Tests registration against each registry endpoint
- Validates all resource types (registered model, model version, model artifact)
- Uses proper resource attribute validation
This ensures that each Model Registry instance is fully functional and independent.
tests/model_registry/rest_api/test_model_registry_secure_db.py (1)
43-43: LGTM! Parameter and fixture name updates are consistent.The changes update parameter and fixture names from
model_registry_mysql_configtomodel_registry_instance, which aligns with the broader refactoring to support multiple Model Registry instances. The test logic remains unchanged and the naming is more consistent.Also applies to: 58-59, 90-90, 104-105
tests/model_registry/rest_api/constants.py (1)
44-57: LGTM! Well-structured test constants added.The new constants provide standardized test data for the expanded multi-instance testing scenarios. They follow the existing patterns with proper typing and clear naming conventions. These constants will help ensure consistency across multiple Model Registry instance tests.
tests/model_registry/upgrade/test_model_registry_upgrade.py (1)
34-34: LGTM! Consistent parameter type updates for multi-instance support.The parameter type changes from single objects to lists are consistently applied across all test methods. The usage of
[0]to access the first element is appropriate for upgrade tests that typically work with a single instance.Also applies to: 52-53, 65-65, 75-75, 84-84
tests/model_registry/rest_api/conftest.py (4)
19-21: LGTM! Improved imports and constants usage.The addition of
get_mysql_configandget_mr_standard_labelsimports along with the updated constantDB_BASE_RESOURCES_NAMEaligns with the dynamic configuration approach and removes hard-coded values.Also applies to: 24-24
47-47: LGTM! Consistent list parameter handling.The fixture parameter types have been consistently updated to accept lists instead of single objects, and all usages properly access the first element
[0]. This maintains backward compatibility while enabling multi-instance support.Also applies to: 50-50, 59-59, 81-81, 118-118, 129-129
144-144: LGTM! Enhanced fixture with dynamic configuration.The addition of the
requestparameter and dynamic MySQL configuration usingget_mysql_config()is a significant improvement. The conditional SSL root certificate configuration and use ofget_mr_standard_labels()makes the fixture more flexible and maintainable.Also applies to: 162-175
244-244: LGTM! Proper deployment list handling.The fixture correctly handles the list of deployments by accessing the first element and maintains consistency in variable usage throughout the patching logic.
Also applies to: 254-268
tests/model_registry/conftest.py (7)
1-2: Imports and constants are appropriate for multi-instance support.The addition of
ExitStackfor managing multiple context managers and the new utility functions for creating resource objects align well with the multi-instance refactoring.Also applies to: 27-34, 41-45
71-87: Multi-instance support implemented correctly for database service.The fixture properly handles creating multiple services based on the
numparameter and usesExitStackto manage their contexts appropriately.
245-251: Correct implementation for multi-instance service retrieval.The fixture properly handles multiple ModelRegistry instances and returns their corresponding services.
256-265: Multi-instance REST endpoint retrieval implemented correctly.The fixture properly extracts REST endpoints from multiple services.
373-398: Multi-instance client creation implemented correctly.The fixture properly creates a ModelRegistry client for each REST endpoint. The simple string splitting for endpoint parsing is intentional per the codebase design patterns.
443-443: Label selector updated correctly.The fixture properly uses the new
DB_RESOURCE_NAMEconstant for the label selector.
498-501: Multi-instance REST URL formatting implemented correctly.The fixture properly formats multiple REST endpoints as HTTPS URLs.
| self: Self, | ||
| admin_client: DynamicClient, | ||
| model_registry_instance: ModelRegistry, | ||
| model_registry_instance: list[Any], |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify if model_registry_instance parameter is used.
The type annotation change from ModelRegistry to list[Any] aligns with the fixture changes, but the test doesn't appear to use the model_registry_instance parameter in its logic.
🏁 Script executed:
#!/bin/bash
# Check if model_registry_instance parameter is used in this test
rg -A 10 -B 5 "model_registry_instance" tests/model_registry/image_validation/test_verify_rhoai_images.pyLength of output: 819
Clarify model_registry_instance fixture usage
The model_registry_instance parameter isn’t referenced in the test body but is needed to spin up the ModelRegistry instance before its pods are validated. To prevent confusion (and linter “unused parameter” errors), please annotate it with a comment or a noqa directive.
• File: tests/model_registry/image_validation/test_verify_rhoai_images.py
Line 45
Current:
model_registry_instance: list[Any],
Suggested:
model_registry_instance: list[Any], # noqa: F841 – ensures registry instance is created for pod validation
🤖 Prompt for AI Agents
In tests/model_registry/image_validation/test_verify_rhoai_images.py at line 45,
the parameter model_registry_instance is not used in the test body but is
required to initialize the ModelRegistry instance. To avoid confusion and linter
warnings about unused variables, add a noqa directive comment to the parameter
line like this: model_registry_instance: list[Any], # noqa: F841 – ensures
registry instance is created for pod validation.
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
1 participant
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: