Refactor test to verify TrustyAIService works with multiple namespaces#408
Refactor test to verify TrustyAIService works with multiple namespaces#408adolfo-ab merged 3 commits intoopendatahub-io:mainfrom
Conversation
📝 WalkthroughSummary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis change removes all multi-namespace TrustyAIService test fixtures and related logic from the main test configuration, relocating them to a new dedicated module. It introduces a new test suite and fixture set for multi-namespace scenarios and marks resource-creation functions as context managers. The main single-namespace test module is simplified accordingly. Changes
Estimated code review effort2 (~20 minutes) Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (5)
🧠 Learnings (1)📓 Common learnings💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🔇 Additional comments (10)
✨ 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
|
|
/wip |
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/build-push-pr-image', '/verified', '/wip', '/lgtm', '/hold', '/cherry-pick'} |
kpunwatk
changed the title
kpunwatk
force-pushed
the
multi_ns
branch
3 times, most recently
from
aa095d3 to
9098f49
Compare
kpunwatk
changed the title
|
/wip cancel |
|
/verified |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py (1)
58-58: Address the missing tests mentioned in the previous review.A past review comment indicated that additional tests are missing. Please ensure all necessary test scenarios are covered or create a follow-up issue to track the missing tests.
🧹 Nitpick comments (1)
tests/model_explainability/trustyai_service/service/multi_ns/conftest.py (1)
51-51: Usedeploy()instead ofcreate()for namespace creation.Based on previous review feedback, use
ns.deploy()instead ofns.create()as it's more graceful and follows the recommended pattern in the openshift-python-wrapper.Apply this change:
- ns.create() + ns.deploy()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tests/model_explainability/trustyai_service/service/conftest.py(0 hunks)tests/model_explainability/trustyai_service/service/multi_ns/conftest.py(1 hunks)tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py(1 hunks)tests/model_explainability/trustyai_service/service/test_trustyai_service.py(0 hunks)tests/model_explainability/trustyai_service/utils.py(6 hunks)
💤 Files with no reviewable changes (2)
- tests/model_explainability/trustyai_service/service/test_trustyai_service.py
- tests/model_explainability/trustyai_service/service/conftest.py
🧰 Additional context used
🧠 Learnings (3)
📓 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#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: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_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py (1)
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_explainability/trustyai_service/service/multi_ns/conftest.py (10)
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: 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#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/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: 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#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: 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#354
File: utilities/user_utils.py:71-76
Timestamp: 2025-06-17T00:29:48.834Z
Learning: In utilities/user_utils.py, the wait_for_user_creation function's retry decorator is intentionally configured to only catch ExceptionUserLogin, not ClusterLoginError. The function catches the result of login_with_user_password and raises ExceptionUserLogin based on the boolean return value, which is the intended retry pattern.
🔇 Additional comments (4)
tests/model_explainability/trustyai_service/utils.py (1)
1-1: Good use of explicit context manager decorators.Adding the
@contextmanagerdecorator to these generator functions is a good practice. It makes the intent clear and ensures these utilities can be properly used with Python'swithstatement for resource management.Also applies to: 119-119, 160-160, 178-178, 205-205, 233-233
tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py (1)
14-117: Well-structured multi-namespace test implementation.The test class properly uses parameterization with
ns_indexto validate TrustyAI service operations across multiple namespaces. The test coverage includes key operations: inference data sending, data upload, metric scheduling, and metric deletion.tests/model_explainability/trustyai_service/service/multi_ns/conftest.py (2)
94-94: Verify the intention behindteardown=False.The
trustyai_service_with_pvc_storage_multi_nsandmlserver_runtime_multi_nsfixtures useteardown=False, which means these resources won't be automatically cleaned up. This could lead to resource leaks if not handled properly.Please confirm if this is intentional. If these resources are meant to be cleaned up by the fixture's cleanup logic (via ExitStack), consider setting
teardown=Trueto ensure proper cleanup even if the test fails unexpectedly.Also applies to: 115-115
1-237: Excellent fixture design for multi-namespace testing.The fixture structure is well-designed:
- Proper use of
ExitStackfor managing multiple context managers- Consistent pattern across all fixtures
- Good separation of concerns with each fixture handling specific resources
- Proper dependency management between fixtures
kpunwatk
force-pushed
the
multi_ns
branch
5 times, most recently
from
05a6f92 to
f74a172
Compare
kpunwatk
force-pushed
the
multi_ns
branch
5 times, most recently
from
b7c2551 to
a190a03
Compare
kpunwatk
force-pushed
the
multi_ns
branch
3 times, most recently
from
ae5ad37 to
fcdf529
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py (1)
103-118: Consider removing unused parameter.The method correctly implements metric deletion testing. However, the
minio_data_connection_multi_nsparameter is not used in the method body.If this parameter is not required for fixture dependency ordering, consider removing it:
def test_drift_metric_delete_multiple_ns( self, ns_index, admin_client, current_client_token, - minio_data_connection_multi_ns, trustyai_service_with_pvc_storage_multi_ns ):
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tests/model_explainability/trustyai_service/service/conftest.py(0 hunks)tests/model_explainability/trustyai_service/service/multi_ns/conftest.py(1 hunks)tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py(1 hunks)tests/model_explainability/trustyai_service/service/test_trustyai_service.py(1 hunks)tests/model_explainability/trustyai_service/utils.py(6 hunks)
🧠 Learnings (2)
📓 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#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#446
File: tests/model_registry/conftest.py:595-612
Timestamp: 2025-07-17T15:42:04.167Z
Learning: In tests/model_registry/conftest.py, the db_deployment_1 fixture (and similar duplicated resource fixtures) do not require the admin_client parameter or explicit dependencies on related fixtures like db_secret_1, db_pvc_1, and db_service_1, even though the original model_registry_db_deployment fixture includes these parameters.
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#446
File: tests/model_registry/conftest.py:579-591
Timestamp: 2025-07-17T15:43:04.876Z
Learning: In tests/model_registry/conftest.py, the db_service_1 and db_service_2 fixtures do not require the admin_client parameter for Service resource creation, despite the existing model_registry_db_service fixture using client=admin_client. This inconsistency was confirmed as intentional by user lugi0.
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#446
File: tests/model_registry/conftest.py:733-770
Timestamp: 2025-07-17T15:42:23.880Z
Learning: In tests/model_registry/conftest.py, the model_registry_instance_1 and model_registry_instance_2 fixtures do not need explicit database dependency fixtures (like db_deployment_1, db_secret_1, etc.) in their function signatures. Pytest's dependency injection automatically handles the fixture dependencies when they reference db_name_1 and db_name_2 parameters. This is the correct pattern for these Model Registry instance fixtures.
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#446
File: tests/model_registry/conftest.py:579-591
Timestamp: 2025-07-17T15:43:04.876Z
Learning: In tests/model_registry/conftest.py, Service resources can be created without explicitly passing the admin_client parameter when using the context manager approach with "with Service()". The client parameter is optional for Service resource creation.
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_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py (2)
Learnt from: adolfo-ab
PR: #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: Snomaan6846
PR: #444
File: tests/model_serving/model_runtime/mlserver/basic_model_deployment/test_mlserver_basic_model_deployment.py:48-714
Timestamp: 2025-07-16T12:20:29.672Z
Learning: In tests/model_serving/model_runtime/mlserver/basic_model_deployment/test_mlserver_basic_model_deployment.py, the same get_deployment_config_dict() function is called twice in each pytest.param because different fixtures (mlserver_inference_service and mlserver_serving_runtime) need the same deployment configuration data. This duplication is intentional to provide identical configuration to multiple fixtures.
🧬 Code Graph Analysis (1)
tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py (4)
tests/model_explainability/trustyai_service/trustyai_service_utils.py (6)
send_inferences_and_verify_trustyai_service_registered(272-336)verify_upload_data_to_trustyai_service(519-551)TrustyAIServiceMetrics(32-41)verify_trustyai_service_metric_scheduling_request(465-516)verify_trustyai_service_metric_delete_request(554-595)Drift(37-41)utilities/constants.py (3)
MinIo(251-304)PodConfig(267-300)Buckets(263-265)tests/conftest.py (2)
admin_client(65-66)current_client_token(81-82)tests/model_explainability/trustyai_service/service/multi_ns/conftest.py (5)
trustyai_service_with_pvc_storage_multi_ns(73-90)gaussian_credit_model_multi_ns(114-149)isvc_getter_token_multi_ns(215-222)model_namespaces(43-54)minio_data_connection_multi_ns(57-70)
🪛 Flake8 (7.2.0)
tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py
[error] 72-72: over-indented
(E117)
[error] 89-89: over-indented
(E117)
💤 Files with no reviewable changes (1)
- tests/model_explainability/trustyai_service/service/conftest.py
✅ Files skipped from review due to trivial changes (1)
- tests/model_explainability/trustyai_service/utils.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/model_explainability/trustyai_service/service/test_trustyai_service.py
- tests/model_explainability/trustyai_service/service/multi_ns/conftest.py
🧰 Additional context used
🧠 Learnings (2)
📓 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#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#446
File: tests/model_registry/conftest.py:595-612
Timestamp: 2025-07-17T15:42:04.167Z
Learning: In tests/model_registry/conftest.py, the db_deployment_1 fixture (and similar duplicated resource fixtures) do not require the admin_client parameter or explicit dependencies on related fixtures like db_secret_1, db_pvc_1, and db_service_1, even though the original model_registry_db_deployment fixture includes these parameters.
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#446
File: tests/model_registry/conftest.py:579-591
Timestamp: 2025-07-17T15:43:04.876Z
Learning: In tests/model_registry/conftest.py, the db_service_1 and db_service_2 fixtures do not require the admin_client parameter for Service resource creation, despite the existing model_registry_db_service fixture using client=admin_client. This inconsistency was confirmed as intentional by user lugi0.
Learnt from: lugi0
PR: opendatahub-io/opendatahub-tests#446
File: tests/model_registry/conftest.py:733-770
Timestamp: 2025-07-17T15:42:23.880Z
Learning: In tests/model_registry/conftest.py, the model_registry_instance_1 and model_registry_instance_2 fixtures do not need explicit database dependency fixtures (like db_deployment_1, db_secret_1, etc.) in their function signatures. Pytest's dependency injection automatically handles the fixture dependencies when they reference db_name_1 and db_name_2 parameters. This is the correct pattern for these Model Registry instance fixtures.
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#446
File: tests/model_registry/conftest.py:579-591
Timestamp: 2025-07-17T15:43:04.876Z
Learning: In tests/model_registry/conftest.py, Service resources can be created without explicitly passing the admin_client parameter when using the context manager approach with "with Service()". The client parameter is optional for Service resource creation.
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_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py (2)
Learnt from: adolfo-ab
PR: #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: Snomaan6846
PR: #444
File: tests/model_serving/model_runtime/mlserver/basic_model_deployment/test_mlserver_basic_model_deployment.py:48-714
Timestamp: 2025-07-16T12:20:29.672Z
Learning: In tests/model_serving/model_runtime/mlserver/basic_model_deployment/test_mlserver_basic_model_deployment.py, the same get_deployment_config_dict() function is called twice in each pytest.param because different fixtures (mlserver_inference_service and mlserver_serving_runtime) need the same deployment configuration data. This duplication is intentional to provide identical configuration to multiple fixtures.
🧬 Code Graph Analysis (1)
tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py (4)
tests/model_explainability/trustyai_service/trustyai_service_utils.py (6)
send_inferences_and_verify_trustyai_service_registered(272-336)verify_upload_data_to_trustyai_service(519-551)TrustyAIServiceMetrics(32-41)verify_trustyai_service_metric_scheduling_request(465-516)verify_trustyai_service_metric_delete_request(554-595)Drift(37-41)utilities/constants.py (3)
MinIo(251-304)PodConfig(267-300)Buckets(263-265)tests/conftest.py (2)
admin_client(65-66)current_client_token(81-82)tests/model_explainability/trustyai_service/service/multi_ns/conftest.py (5)
trustyai_service_with_pvc_storage_multi_ns(73-90)gaussian_credit_model_multi_ns(114-149)isvc_getter_token_multi_ns(215-222)model_namespaces(43-54)minio_data_connection_multi_ns(57-70)
🪛 Flake8 (7.2.0)
tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py
[error] 72-72: over-indented
(E117)
[error] 89-89: over-indented
(E117)
🔇 Additional comments (4)
tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py (4)
1-13: LGTM!The imports are well-organized and all imported modules and constants are appropriately used in the test methods.
15-34: LGTM!The class definition and parametrization structure correctly sets up multi-namespace testing scenarios. The use of predictable namespace names and proper fixture parametrization follows established patterns.
36-59: LGTM!The test method correctly implements multi-namespace testing by using the
ns_indexparameter to select appropriate resources from fixture lists. All required parameters are properly passed to the verification utility function.
33-33: All multi-namespace tests have been migrated successfully.Verified that the new
test_trustyai_service_multi_ns.pyincludes all four key test methods (inference sending, data upload, drift metric scheduling, and drift metric deletion) and no additional multi-namespace tests or TODO/FIXME markers remain. Everything appears covered.
new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py modified: tests/model_explainability/trustyai_service/service/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py modified: tests/model_explainability/trustyai_service/service/test_trustyai_service.py modified: tests/model_explainability/trustyai_service/utils.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.pyi new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py modified: tests/model_explainability/trustyai_service/service/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py modified: tests/model_explainability/trustyai_service/service/test_trustyai_service.py modified: tests/model_explainability/trustyai_service/utils.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py modified: tests/model_explainability/trustyai_service/service/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/conftest.py new file: tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py modified: tests/model_explainability/trustyai_service/service/test_trustyai_service.py modified: tests/model_explainability/trustyai_service/utils.py new file: tests/model_explainability/trustyai_service/service/multi_ns/test_trustyai_service_multi_ns.py
for more information, see https://pre-commit.ci
|
/lgtm |
|
Status of building tag latest: success. |
4 participants
Addresses: https://issues.redhat.com/browse/RHOAIENG-28243
Description
This PR removes duplication across fixtures such as model_namespace_2, gaussian_credit_model_2, etc. A new directory has been created to host test classes specifically targeting multi-namespace scenarios. Additionally, a dedicated conftest.py file was introduced to define reusable fixtures capable of iterating over multiple namespaces. The existing test class for PVC-based drift service scenarios has been refactored to leverage this new structure.