Add QG Markers for AI Safety and marker deprecation warning#1193
Merged
dbasunag merged 3 commits intoopendatahub-io:mainfrom Mar 10, 2026
Merged
Add QG Markers for AI Safety and marker deprecation warning#1193dbasunag merged 3 commits intoopendatahub-io:mainfrom
dbasunag merged 3 commits intoopendatahub-io:mainfrom
Conversation
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/verified', '/build-push-pr-image', '/cherry-pick', '/wip', '/hold', '/lgtm'} |
for more information, see https://pre-commit.ci
Contributor
📝 WalkthroughWalkthroughChanges add pytest marker decorators (tier1, smoke) across multiple test files for improved test categorization and selection. Configuration update adds deprecation notice to pytest.ini. One test file restructures tier2 testing into a dedicated test function separate from parameterized tier1 tests. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
dbasunag
approved these changes
Mar 10, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pytest.ini (1)
14-14: Deprecation notice is informational only.The inline deprecation warning in the marker description won't trigger pytest's deprecation warning system. If you want pytest to emit actual warnings when
@pytest.mark.sanityis used, consider using apytest_configurehook withwarnings.warn()inconftest.py.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pytest.ini` at line 14, The marker description for the pytest mark "sanity" is only an inline note and won't trigger pytest deprecation warnings; add a pytest_configure hook in conftest.py that registers the "sanity" marker and emits a real deprecation via warnings.warn() when the marker is used (use pytest.Config.getini or inspect node.iter_markers for "sanity" in pytest_collection_modifyitems or in pytest_configure to detect usage), update the marker registration to include the deprecation note, and ensure the warning uses the proper DeprecationWarning category so pytest will surface it.tests/model_explainability/guardrails/test_guardrails.py (1)
217-217: Minor formatting: missing space after comma.- "model_namespace, orchestrator_config, guardrails_gateway_config,guardrails_orchestrator", + "model_namespace, orchestrator_config, guardrails_gateway_config, guardrails_orchestrator",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/model_explainability/guardrails/test_guardrails.py` at line 217, The tuple/string argument list in the test contains a missing space after a comma in the literal "model_namespace, orchestrator_config, guardrails_gateway_config,guardrails_orchestrator"; update that literal so there's a space after the comma (i.e., "..., guardrails_gateway_config, guardrails_orchestrator") in tests/model_explainability/guardrails/test_guardrails.py to fix the formatting.
🤖 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_explainability/lm_eval/test_lm_eval.py`:
- Around line 58-73: The test function test_lmeval_huggingface_model_tier2
currently declares unused fixture parameters admin_client and model_namespace;
remove them from the function signature or convert them to setup-only fixtures
by adding `@pytest.mark.usefixtures`("admin_client", "model_namespace") above the
test. Update the declaration for test_lmeval_huggingface_model_tier2 (which uses
lmevaljob_hf_pod) to match the pattern used in test_lmeval_huggingface_model so
those fixtures remain available for dependency setup without being passed as
unused parameters.
---
Nitpick comments:
In `@pytest.ini`:
- Line 14: The marker description for the pytest mark "sanity" is only an inline
note and won't trigger pytest deprecation warnings; add a pytest_configure hook
in conftest.py that registers the "sanity" marker and emits a real deprecation
via warnings.warn() when the marker is used (use pytest.Config.getini or inspect
node.iter_markers for "sanity" in pytest_collection_modifyitems or in
pytest_configure to detect usage), update the marker registration to include the
deprecation note, and ensure the warning uses the proper DeprecationWarning
category so pytest will surface it.
In `@tests/model_explainability/guardrails/test_guardrails.py`:
- Line 217: The tuple/string argument list in the test contains a missing space
after a comma in the literal "model_namespace, orchestrator_config,
guardrails_gateway_config,guardrails_orchestrator"; update that literal so
there's a space after the comma (i.e., "..., guardrails_gateway_config,
guardrails_orchestrator") in
tests/model_explainability/guardrails/test_guardrails.py to fix the formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c853b2f6-2653-4a7c-84f7-380345bd9f69
📒 Files selected for processing (5)
pytest.initests/llama_stack/safety/test_trustyai_fms_provider.pytests/model_explainability/guardrails/test_guardrails.pytests/model_explainability/lm_eval/test_lm_eval.pytests/model_explainability/trustyai_service/service/test_trustyai_service.py
kpunwatk
approved these changes
Mar 10, 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.
Add tier1, tier2 and smoke markers to AI Safety Tests and add a sanity marker deprecation warning
Summary by CodeRabbit