allow skipping must-gather collection for tests#644
allow skipping must-gather collection for tests#644dbasunag merged 2 commits intoopendatahub-io:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds helper functions to detect pytest node markers and skip must-gather when marked. Updates pytest_exception_interact to honor the new marker alongside the CLI option. Declares the skip_must_gather marker in pytest.ini. Applies the marker to two model registry test modules. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Comment |
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/build-push-pr-image', '/verified', '/cherry-pick', '/lgtm', '/hold', '/wip'} |
![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
🧹 Nitpick comments (3)
pytest.ini (1)
23-23: Nit: fix marker description grammar and align phrasing with others.Update to read more clearly and match the “Mark tests that …” style used above.
- skip_must_gather: Tests that does not require must-gather for triaging + skip_must_gather: Mark tests that do not require must-gather collection for triagingconftest.py (2)
446-446: Silence Ruff ARG001: prefix unusedcallparameter with underscore.Keeps the hook signature while fixing the linter warning. [Based on static_analysis_hints]
-def pytest_exception_interact(node: Item | Collector, call: CallInfo[Any], report: TestReport | CollectReport) -> None: +def pytest_exception_interact(node: Item | Collector, _call: CallInfo[Any], report: TestReport | CollectReport) -> None:
438-444: Streamline marker check; drop unused helper
Replace both functions with:def is_skip_must_gather(node: Node) -> bool: return any(node.iter_markers(name="skip_must_gather"))
get_all_node_markersisn’t referenced elsewhere and can be removed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
conftest.py(2 hunks)pytest.ini(1 hunks)tests/model_registry/image_validation/test_verify_rhoai_images.py(1 hunks)tests/model_registry/model_catalog/test_default_model_catalog.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-21T19:08:14.982Z
Learnt from: dbasunag
PR: opendatahub-io/opendatahub-tests#527
File: tests/model_registry/rbac/conftest.py:358-359
Timestamp: 2025-08-21T19:08:14.982Z
Learning: In tests/model_registry/rbac/conftest.py, the model catalog configmap only exists in one specific namespace (py_config["model_registry_namespace"]) regardless of where model registry instances are deployed, making it a shared resource that serves all model registry instances across namespaces.
Applied to files:
tests/model_registry/model_catalog/test_default_model_catalog.py
🧬 Code graph analysis (1)
conftest.py (1)
tests/conftest.py (1)
nodes(562-563)
🪛 Ruff (0.13.1)
conftest.py
446-446: Unused function argument: call
(ARG001)
🔇 Additional comments (4)
conftest.py (2)
22-22: LGTM: importing Node for precise type hints.Accurate and scoped import.
448-448: LGTM: condition now honors the skip_must_gather marker.The gate correctly prevents collection when marked.
tests/model_registry/model_catalog/test_default_model_catalog.py (1)
33-33: LGTM: applied skip_must_gather at class scope.Consistent with the new marker behavior.
tests/model_registry/image_validation/test_verify_rhoai_images.py (1)
32-33: LGTM: test opts out of must-gather collection.Matches the conftest logic and pytest.ini marker declaration.
|
Status of building tag latest: success. |
|
/cherry-pick 2.25 |
|
Cherry pick action created PR #661 successfully 🎉! |
4 participants
Description
How Has This Been Tested?
Merge criteria:
Summary by CodeRabbit