QG refator smoke (reduce time)#1201
Merged
mwaykole merged 2 commits intoopendatahub-io:mainfrom Mar 11, 2026
Merged
Conversation
Signed-off-by: Milind waykole <mwaykole@redhat.com>
mwaykole
requested review from
Raghul-M,
brettmthompson and
threcc
as code owners
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/build-push-pr-image', '/wip', '/hold', '/lgtm', '/verified', '/cherry-pick'} |
Raghul-M
approved these changes
Mar 11, 2026
Contributor
📝 WalkthroughWalkthroughThis pull request updates pytest test markers across 20 test files in the model serving test suite, changing test categorization labels from Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/model_serving/model_server/kserve/inference_service_configuration/test_isvc_pull_secret_updates.py (1)
27-39:⚠️ Potential issue | 🟡 MinorApply
@pytest.mark.tier1at class level or to all three test methods.The marker is applied only to
test_initial_pull_secret_set, buttest_update_pull_secretandtest_remove_pull_secretdepend on fixtures that require test 1 to complete first. Running with-m tier1will skip tests 2 and 3, breaking the fixture chain. Align with the codebase pattern used in similar test files (e.g.,test_missing_required_fields.py,test_unsupported_content_type.py): apply tier markers at the class level via decorator stacking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/model_serving/model_server/kserve/inference_service_configuration/test_isvc_pull_secret_updates.py` around lines 27 - 39, The pytest marker `@pytest.mark.tier1` is only applied to test_initial_pull_secret_set causing test_update_pull_secret and test_remove_pull_secret to be skipped when running with -m tier1 and breaking the fixture chain; fix by applying the marker at the class level (decorate the test class that contains test_initial_pull_secret_set, test_update_pull_secret, and test_remove_pull_secret) or alternatively add `@pytest.mark.tier1` to both test_update_pull_secret and test_remove_pull_secret so all three tests (test_initial_pull_secret_set, test_update_pull_secret, test_remove_pull_secret) share the same tier1 marker as in similar test files.
🧹 Nitpick comments (1)
tests/model_serving/model_server/kserve/raw_deployment/test_kserve_raw_routes_reconciliation.py (1)
26-31: Marker change noted, but pre-existing typos in method name.Method
test_raw_onnx_rout_reconciliationand docstring contain "rout" instead of "route". Consider fixing in a follow-up.Typo fix suggestion
- def test_raw_onnx_rout_reconciliation(self, ovms_raw_inference_service): + def test_raw_onnx_route_reconciliation(self, ovms_raw_inference_service): """ Verify that the KServe Raw ONNX model can be queried using REST - and ensure that the model rout reconciliation works correctly . + and ensure that the model route reconciliation works correctly. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/model_serving/model_server/kserve/raw_deployment/test_kserve_raw_routes_reconciliation.py` around lines 26 - 31, Rename the test and update its docstring to fix the typo "rout" → "route": change the test function name test_raw_onnx_rout_reconciliation to test_raw_onnx_route_reconciliation and update the docstring sentence "model rout reconciliation" to "model route reconciliation" (ensure any references to test_raw_onnx_rout_reconciliation in fixtures/markers like ovms_raw_inference_service or tests referencing this name are also updated).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tests/model_serving/model_server/kserve/authentication/test_kserve_token_authentication_raw.py`:
- Line 13: Remove the class-level `@pytest.mark.smoke` decorator from the
TestKServeTokenAuthenticationRaw class so the smoke marker is only applied to
the individual happy-path test methods; keep the method-level `@pytest.mark.smoke`
on the happy-path tests (e.g., test_successful_raw_model_authentication) and
remove any redundant `@pytest.mark.smoke` from other methods such as
test_raw_disable_enable_authentication_no_pod_rollout so non-core tests
(test_disabled_raw_model_authentication,
test_re_enabled_raw_model_authentication) are no longer included in the smoke
gate.
---
Outside diff comments:
In
`@tests/model_serving/model_server/kserve/inference_service_configuration/test_isvc_pull_secret_updates.py`:
- Around line 27-39: The pytest marker `@pytest.mark.tier1` is only applied to
test_initial_pull_secret_set causing test_update_pull_secret and
test_remove_pull_secret to be skipped when running with -m tier1 and breaking
the fixture chain; fix by applying the marker at the class level (decorate the
test class that contains test_initial_pull_secret_set, test_update_pull_secret,
and test_remove_pull_secret) or alternatively add `@pytest.mark.tier1` to both
test_update_pull_secret and test_remove_pull_secret so all three tests
(test_initial_pull_secret_set, test_update_pull_secret, test_remove_pull_secret)
share the same tier1 marker as in similar test files.
---
Nitpick comments:
In
`@tests/model_serving/model_server/kserve/raw_deployment/test_kserve_raw_routes_reconciliation.py`:
- Around line 26-31: Rename the test and update its docstring to fix the typo
"rout" → "route": change the test function name
test_raw_onnx_rout_reconciliation to test_raw_onnx_route_reconciliation and
update the docstring sentence "model rout reconciliation" to "model route
reconciliation" (ensure any references to test_raw_onnx_rout_reconciliation in
fixtures/markers like ovms_raw_inference_service or tests referencing this name
are also updated).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 32b1abca-7350-48fb-ac88-ad8acb7b4e3e
📒 Files selected for processing (20)
tests/model_serving/model_server/kserve/authentication/test_kserve_token_authentication_raw.pytests/model_serving/model_server/kserve/authentication/test_non_admin_users.pytests/model_serving/model_server/kserve/components/kserve_dsc_deployment_mode/test_kserve_dsc_default_deployment_mode.pytests/model_serving/model_server/kserve/components/test_custom_resources.pytests/model_serving/model_server/kserve/components/test_model_serving_components.pytests/model_serving/model_server/kserve/inference_graph/test_inference_graph_raw.pytests/model_serving/model_server/kserve/inference_service_configuration/test_isvc_env_vars_updates.pytests/model_serving/model_server/kserve/inference_service_configuration/test_isvc_pull_secret_updates.pytests/model_serving/model_server/kserve/inference_service_configuration/test_isvc_replicas_update.pytests/model_serving/model_server/kserve/kueue/test_kueue_isvc_raw.pytests/model_serving/model_server/kserve/metrics/test_model_metrics.pytests/model_serving/model_server/kserve/metrics/test_non_admin_users.pytests/model_serving/model_server/kserve/model_car/test_oci_image.pytests/model_serving/model_server/kserve/negative/test_invalid_model_name.pytests/model_serving/model_server/kserve/negative/test_malformed_json_payload.pytests/model_serving/model_server/kserve/negative/test_missing_required_fields.pytests/model_serving/model_server/kserve/negative/test_unsupported_content_type.pytests/model_serving/model_server/kserve/negative/test_wrong_input_data_type.pytests/model_serving/model_server/kserve/raw_deployment/test_kserve_raw_routes_reconciliation.pytests/model_serving/model_server/kserve/stop_resume/test_raw_stop_resume_model.py
threcc
approved these changes
Mar 11, 2026
|
Status of building tag latest: success. |
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
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Related Issues
How it has been tested
Additional Requirements
Summary by CodeRabbit