fix: Remove closed jira references and jira references from doc string#1228
fix: Remove closed jira references and jira references from doc string#1228dbasunag merged 5 commits intoopendatahub-io:mainfrom
Conversation
Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com>
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/lgtm', '/build-push-pr-image', '/hold', '/wip', '/cherry-pick', '/verified'} |
📝 WalkthroughWalkthroughRemoved Jira-related markers and docstring references across KServe tests; deleted a Jira-conditional KEDA metric-patching block and its imports. Most edits are metadata/docstring cleanup or replacing Jira markers with generic skips; one autoscaling test lost the conditional metric-injection branch. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/model_serving/model_server/kserve/storage/oci/test_oci_image.py (1)
56-67: Unconditional skip lacks re-enablement tracking.The
@pytest.mark.skipdecorator will prevent this test from ever running until manually removed. Consider using@pytest.mark.xfailwithstrict=Falseif the underlying issue is intermittent, or add a TODO/FIXME comment with acceptance criteria for when to un-skip.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/model_serving/model_server/kserve/storage/oci/test_oci_image.py` around lines 56 - 67, The test test_model_car_using_rest is unconditionally skipped via `@pytest.mark.skip`, which prevents automatic re-enablement; replace the unconditional skip with a non-strict xfail (e.g., `@pytest.mark.xfail`(strict=False, reason="RHOAIENG-12306")) or add a conditional skip that checks an env flag so the test can be re-enabled automatically, and add a TODO/FIXME comment above the test referencing RHOAIENG-12306 plus clear acceptance criteria for when to remove the xfail/skip; update references to the test function name test_model_car_using_rest and its fixture model_car_inference_service so reviewers can find the changed decorator and comment.
🤖 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/ingress/test_route_visibility.py`:
- Line 192: Replace the unconditional `@pytest.mark.skip` decorator applied to the
GRPC raw-route tests with either `@pytest.mark.xfail`(strict=False, reason="...
GH-<issue>") so the tests run and failures are visible, or with
`@pytest.mark.skipif`(<capability check>, reason="... GH-<issue>") so skipping is
conditional; update both occurrences (the class-level decorator currently set to
`@pytest.mark.skip`(reason="skipping grpc raw for tgis-caikit") around the grpc
raw-route tests) to include a short GitHub issue reference in the reason string
(e.g., "GH-12345") and, if using skipif, implement a clear capability predicate
(e.g., HAS_GRPC_RAW or a helper function) so CI sees regressions while still
allowing intentional skips.
---
Nitpick comments:
In `@tests/model_serving/model_server/kserve/storage/oci/test_oci_image.py`:
- Around line 56-67: The test test_model_car_using_rest is unconditionally
skipped via `@pytest.mark.skip`, which prevents automatic re-enablement; replace
the unconditional skip with a non-strict xfail (e.g.,
`@pytest.mark.xfail`(strict=False, reason="RHOAIENG-12306")) or add a conditional
skip that checks an env flag so the test can be re-enabled automatically, and
add a TODO/FIXME comment above the test referencing RHOAIENG-12306 plus clear
acceptance criteria for when to remove the xfail/skip; update references to the
test function name test_model_car_using_rest and its fixture
model_car_inference_service so reviewers can find the changed decorator and
comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: ab7a4440-0d47-4350-b60f-453f3e53562b
📒 Files selected for processing (2)
tests/model_serving/model_server/kserve/ingress/test_route_visibility.pytests/model_serving/model_server/kserve/storage/oci/test_oci_image.py
|
Status of building tag latest: success. |
opendatahub-io#1228) * fix: Remove closed jira references and jira references from doc string Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com> * fix: add the skips based on review comments Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com> --------- Signed-off-by: Debarati Basu-Nag <dbasunag@redhat.com> Signed-off-by: Shehan Saleem <ssaleem@redhat.com>
Pull Request
Summary
Jira references in doc string interferes with pytest-jira plugin and causes fail/x-pass unnecessarily.
Closed jira references with pytest.mark.jira are not doing anything, so removed those as well
Related Issues
How it has been tested
Additional Requirements
Summary by CodeRabbit