Move and type OADP related functions from tests directory to the upper level utilities directory and add unit tests #3301
+457
−92
Conversation
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
📝 WalkthroughWalkthroughMoves Velero-related utilities (VeleroRestore, check_file_in_vm, is_storage_class_support_volume_mode) from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (4)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
{libs,utilities}/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/test_*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/conftest.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (58)📓 Common learnings📚 Learning: 2026-01-06T20:32:10.845ZApplied to files:
📚 Learning: 2026-01-06T20:32:10.845ZApplied to files:
📚 Learning: 2025-06-03T12:36:36.982ZApplied to files:
📚 Learning: 2025-10-30T10:43:48.886ZApplied to files:
📚 Learning: 2025-09-03T07:23:37.045ZApplied to files:
📚 Learning: 2025-09-03T07:22:50.446ZApplied to files:
📚 Learning: 2025-09-29T20:33:51.007ZApplied to files:
📚 Learning: 2025-12-29T10:07:12.804ZApplied to files:
📚 Learning: 2025-12-16T20:11:03.645ZApplied to files:
📚 Learning: 2025-09-07T13:16:32.011ZApplied to files:
📚 Learning: 2025-08-06T13:57:34.740ZApplied to files:
📚 Learning: 2026-01-06T20:32:10.845ZApplied to files:
📚 Learning: 2026-01-06T20:32:10.845ZApplied to files:
📚 Learning: 2026-01-06T20:32:10.845ZApplied to files:
📚 Learning: 2026-01-07T11:51:57.253ZApplied to files:
📚 Learning: 2025-12-07T13:52:56.070ZApplied to files:
📚 Learning: 2025-11-26T16:03:07.813ZApplied to files:
📚 Learning: 2025-12-16T10:28:54.212ZApplied to files:
📚 Learning: 2025-12-07T13:52:56.070ZApplied to files:
📚 Learning: 2025-08-09T01:46:48.039ZApplied to files:
📚 Learning: 2025-06-05T11:36:15.214ZApplied to files:
📚 Learning: 2025-12-16T15:09:49.597ZApplied to files:
📚 Learning: 2025-09-15T06:49:53.478ZApplied to files:
📚 Learning: 2025-09-28T14:43:07.181ZApplied to files:
📚 Learning: 2025-09-28T14:43:07.181ZApplied to files:
📚 Learning: 2025-08-09T01:52:26.683ZApplied to files:
📚 Learning: 2025-06-03T14:34:04.796ZApplied to files:
📚 Learning: 2025-08-20T23:43:28.117ZApplied to files:
📚 Learning: 2025-06-18T09:31:06.311ZApplied to files:
📚 Learning: 2026-01-04T19:37:57.919ZApplied to files:
📚 Learning: 2025-05-18T09:24:43.335ZApplied to files:
📚 Learning: 2025-06-23T19:24:28.327ZApplied to files:
📚 Learning: 2025-08-14T10:28:22.958ZApplied to files:
📚 Learning: 2025-05-26T13:36:16.136ZApplied to files:
📚 Learning: 2025-05-27T11:44:14.859ZApplied to files:
📚 Learning: 2026-01-06T20:32:10.845ZApplied to files:
📚 Learning: 2026-01-06T20:32:10.845ZApplied to files:
📚 Learning: 2026-01-05T10:33:55.037ZApplied to files:
📚 Learning: 2025-12-16T14:00:59.076ZApplied to files:
📚 Learning: 2025-11-08T07:36:57.616ZApplied to files:
📚 Learning: 2025-06-03T14:39:26.630ZApplied to files:
📚 Learning: 2025-12-22T15:56:00.157ZApplied to files:
📚 Learning: 2025-06-03T14:38:59.401ZApplied to files:
📚 Learning: 2025-06-10T09:41:58.035ZApplied to files:
📚 Learning: 2025-12-15T12:33:06.686ZApplied to files:
📚 Learning: 2025-12-22T16:27:40.244ZApplied to files:
📚 Learning: 2026-01-07T09:52:12.342ZApplied to files:
📚 Learning: 2025-09-17T14:56:18.111ZApplied to files:
📚 Learning: 2025-10-16T12:47:04.521ZApplied to files:
📚 Learning: 2025-09-10T13:25:59.799ZApplied to files:
📚 Learning: 2025-05-21T09:41:46.532ZApplied to files:
📚 Learning: 2025-06-18T09:21:34.315ZApplied to files:
📚 Learning: 2026-01-07T11:52:04.988ZApplied to files:
📚 Learning: 2025-12-16T14:06:22.391ZApplied to files:
📚 Learning: 2025-08-27T12:38:26.496ZApplied to files:
📚 Learning: 2025-09-29T19:05:24.987ZApplied to files:
📚 Learning: 2026-01-06T11:47:20.240ZApplied to files:
🧬 Code graph analysis (3)utilities/oadp.py (4)
tests/storage/storage_migration/utils.py (1)
tests/data_protection/oadp/test_velero.py (3)
🪛 Ruff (0.14.10)utilities/oadp.py208-208: Boolean-typed positional argument in function definition (FBT001) 208-208: Boolean default positional argument in function definition (FBT002) 210-210: Boolean-typed positional argument in function definition (FBT001) 210-210: Boolean default positional argument in function definition (FBT002) 212-212: Dynamically typed expressions (typing.Any) are disallowed in (ANN401) utilities/unittests/test_oadp.py5-5: Unused Remove unused (RUF100) 27-27: Unused Remove unused (RUF100) 701-701: Unused method argument: (ARG002) tests/data_protection/oadp/test_velero.py98-98: Unused function argument: (ARG001) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (17)
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 |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3301 +/- ##
==========================================
+ Coverage 98.38% 98.41% +0.02%
==========================================
Files 25 25
Lines 2166 2205 +39
==========================================
+ Hits 2131 2170 +39
Misses 35 35
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
openshift-virtualization-qe-bot-5
requested review from
RoniKishner,
geetikakay,
kshvaika,
rnetser and
vsibirsk
openshift-virtualization-qe-bot
added
the
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot-5
requested review from
dshchedr,
josemacassan,
jpeimer and
kgoldbla
openshift-virtualization-qe-bot
added
tox:verify-bugs-are-open:running
openshift-virtualization-qe-bot
previously approved these changes
Dec 30, 2025
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/data_protection/oadp/conftest.pytests/data_protection/oadp/test_velero.pytests/data_protection/oadp/utils.pyutilities/oadp.py
💤 Files with no reviewable changes (1)
- tests/data_protection/oadp/utils.py
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 2920
File: utilities/network.py:31-31
Timestamp: 2025-12-07T13:52:56.070Z
Learning: In the openshift-virtualization-tests repository, the utilities/ package is not intended to be used as a valid, structured module with clear focus and ownership. The team prefers using specialized library locations like tests/network/libs/ for network-related helper code rather than placing everything in utilities/.
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1678
File: utilities/oadp.py:5-5
Timestamp: 2025-08-26T14:51:22.273Z
Learning: qwang1 contributed the Backup resource to openshift-python-wrapper and the PR was merged. They use uv for dependency management in openshift-virtualization-tests.
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 2920
File: utilities/network.py:31-31
Timestamp: 2025-12-07T13:52:56.070Z
Learning: In the openshift-virtualization-tests repository, it is acceptable for utilities/ modules to import from tests/network/libs/ (e.g., utilities/network.py importing from tests/network/libs/sriovnetworknode). The libs modules under tests/network/libs/ are library/tool modules, not test code, and this import direction does not violate architectural principles in this codebase.
Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds architecture-specific OS_FLAVOR attributes to the Images.Cirros class (OS_FLAVOR_CIRROS for x86_64/ARM64, OS_FLAVOR_FEDORA for s390x), enabling conditional logic based on the underlying OS flavor in tests.
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1678
File: utilities/oadp.py:1-5
Timestamp: 2025-09-07T13:16:32.011Z
Learning: In the openshift-virtualization-tests project utilities/oadp.py, DynamicClient instances are passed as parameters rather than created internally, so kubernetes.config import is not needed for client creation fallback patterns.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-22T15:56:00.157Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, do NOT use "REQUEST_CHANGES" review type if the PR author has already marked the PR as verified (e.g., with `/verified` command). Test execution plans are informational guides, not blocking requirements. Use COMMENT event for informational test plans, or only REQUEST_CHANGES if there are actual code issues that need to be addressed before merging.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 2014
File: tests/storage/cdi_clone/test_clone.py:0-0
Timestamp: 2025-09-10T13:25:59.799Z
Learning: In the openshift-virtualization-tests repository, when creating DataVolume objects using ocp_resources, use `api_name="storage"` rather than the default `api_name="pvc"`.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1723
File: conftest.py:496-502
Timestamp: 2025-08-27T12:38:26.496Z
Learning: In PR #1723, the user rnetser clarified that updating downstream string comparisons to handle DataVolume enums from StorageClassConfig is not part of the current PR scope, which focuses only on adding conformance storage class support.
Learnt from: dalia-frank
Repo: RedHatQE/openshift-virtualization-tests PR: 1845
File: tests/storage/cdi_import/test_import_http.py:280-282
Timestamp: 2025-08-27T08:34:49.132Z
Learning: The create_pod_for_pvc helper function is still used in other tests beyond the current PR scope and may need to be addressed in a separate PR to avoid breaking existing functionality.
📚 Learning: 2025-08-28T11:29:15.768Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1678
File: utilities/oadp.py:5-5
Timestamp: 2025-08-28T11:29:15.768Z
Learning: In the openshift-virtualization-tests project, DynamicClient is imported from kubernetes.dynamic, not openshift.dynamic. The standard import pattern is `from kubernetes.dynamic import DynamicClient`.
Applied to files:
utilities/oadp.py
📚 Learning: 2025-09-07T13:16:32.011Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1678
File: utilities/oadp.py:1-5
Timestamp: 2025-09-07T13:16:32.011Z
Learning: In the openshift-virtualization-tests project utilities/oadp.py, DynamicClient instances are passed as parameters rather than created internally, so kubernetes.config import is not needed for client creation fallback patterns.
Applied to files:
utilities/oadp.pytests/data_protection/oadp/conftest.pytests/data_protection/oadp/test_velero.py
📚 Learning: 2025-12-07T13:52:56.070Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 2920
File: utilities/network.py:31-31
Timestamp: 2025-12-07T13:52:56.070Z
Learning: In the openshift-virtualization-tests repository, it is acceptable for utilities/ modules to import from tests/network/libs/ (e.g., utilities/network.py importing from tests/network/libs/sriovnetworknode). The libs modules under tests/network/libs/ are library/tool modules, not test code, and this import direction does not violate architectural principles in this codebase.
Applied to files:
tests/data_protection/oadp/conftest.pytests/data_protection/oadp/test_velero.py
📚 Learning: 2025-09-10T13:25:59.799Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 2014
File: tests/storage/cdi_clone/test_clone.py:0-0
Timestamp: 2025-09-10T13:25:59.799Z
Learning: In the openshift-virtualization-tests repository, when creating DataVolume objects using ocp_resources, use `api_name="storage"` rather than the default `api_name="pvc"`.
Applied to files:
tests/data_protection/oadp/conftest.pytests/data_protection/oadp/test_velero.py
📚 Learning: 2025-10-16T12:47:04.521Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 2249
File: tests/install_upgrade_operators/must_gather/test_must_gather.py:428-441
Timestamp: 2025-10-16T12:47:04.521Z
Learning: In openshift-virtualization-tests repository, DataVolumes in the openshift-virtualization-os-images namespace are volatile resources managed by DataImportCron. They can be created/destroyed between must-gather collection listing and file retrieval, requiring FileNotFoundError exception handling in test_crd_resources to skip these volatile resources gracefully while still validating DataVolumes in other namespaces. There is no pytest_generate_tests hook that filters out datavolumes from test parametrization.
Applied to files:
tests/data_protection/oadp/conftest.pytests/data_protection/oadp/test_velero.py
📚 Learning: 2025-12-15T12:33:06.686Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 3024
File: tests/network/connectivity/utils.py:17-17
Timestamp: 2025-12-15T12:33:06.686Z
Learning: In the test suite, ensure the ipv6_network_data fixture returns a factory function (Callable) and that all call sites invoke it to obtain the actual data dict, i.e., use ipv6_network_data() at call sites. This enables future extensibility for configuring secondary interfaces' IP addresses without changing call sites. Apply this pattern to all Python test files under tests.
Applied to files:
tests/data_protection/oadp/conftest.pytests/data_protection/oadp/test_velero.py
📚 Learning: 2025-12-22T16:27:40.244Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 3196
File: tests/network/upgrade/test_upgrade_network.py:4-4
Timestamp: 2025-12-22T16:27:40.244Z
Learning: For PRs that remove tests, rely on pytest --collect-only to verify the test discovery results (which tests are selected/deselected) and ensure the removal is clean and the test module remains functional. Full test execution is not required for test deletion PRs. This guideline applies to test files anywhere under the tests/ directory (e.g., tests/network/upgrade/test_upgrade_network.py) and should be used for similar test-deletion scenarios across the repository.
Applied to files:
tests/data_protection/oadp/conftest.pytests/data_protection/oadp/test_velero.py
📚 Learning: 2025-08-09T01:52:26.683Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Applied to files:
tests/data_protection/oadp/test_velero.py
📚 Learning: 2025-05-21T09:41:46.532Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/conftest.py:156-176
Timestamp: 2025-05-21T09:41:46.532Z
Learning: In the RedHatQE/openshift-virtualization-tests codebase, the pattern of calling to_dict() on a DataVolume object and then accessing dv.res to populate data_volume_template is the preferred approach rather than building the template dict explicitly.
Applied to files:
tests/data_protection/oadp/test_velero.py
🧬 Code graph analysis (1)
tests/data_protection/oadp/conftest.py (1)
utilities/oadp.py (3)
VeleroRestore(149-186)create_rhel_vm(97-146)is_storage_class_support_volume_mode(197-201)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
🔇 Additional comments (6)
tests/data_protection/oadp/conftest.py (1)
15-20: Import updates look correct.The import sources for
VeleroRestoreandis_storage_class_support_volume_modehave been properly updated to referenceutilities.oadpinstead of the tests directory. This aligns with the PR objective to move OADP utilities out of tests.tests/data_protection/oadp/test_velero.py (1)
4-6: Import updates are correct.The import source for
check_file_in_vmhas been properly updated toutilities.oadp. Thewait_for_restored_dvfunction remains in the tests directory, indicating a selective migration of utilities.utilities/oadp.py (4)
8-9: New imports support the moved functionality correctly.The imports for
Restore,StorageProfile, andconsoleare appropriate for the newly addedVeleroRestoreclass and helper functions.Also applies to: 12-12
21-26: Constants imported for the moved utilities.The additional constants (
FILE_NAME_FOR_BACKUP,LS_COMMAND,TEXT_TO_TEST,TIMEOUT_20SEC) are correctly imported to support thecheck_file_in_vmfunction.
189-194: Helper function implementation is straightforward.The
check_file_in_vmfunction appropriately validates file existence and content in a VM via console interaction.
197-201: Storage class volume mode check is correctly implemented.The
is_storage_class_support_volume_modefunction properly queries the StorageProfile to determine volume mode support.
…r level utilities directory For upcoming oadp-chaos test cases. And add unit tests for oadp utilities. Signed-off-by: Qixuan Wang <qwang@redhat.com>
qwang1
force-pushed
the
qwang-oadp-move
branch
from
e660a1f to
d998ef6
Compare
openshift-virtualization-qe-bot
added
size/XXL
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
approved these changes
Jan 8, 2026
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
utilities/oadp.py (1)
83-92: LOW: Inconsistent logging style between VeleroBackup and VeleroRestore.
VeleroBackup.__exit__uses f-string formatting while the newVeleroRestore.__exit__(lines 241-256) uses structured logging withextra={}. The coding guidelines require structured key-value pairs for searchability.Consider updating
VeleroBackupto match the structured logging pattern for consistency:♻️ Suggested refactor
def __exit__(self, exception_type, exception_value, traceback) -> None: try: if self.teardown: delete_velero_resource(resource=self, client=self.client) else: - LOGGER.info(f"Skipping Velero delete for {self.kind} {self.name} (teardown=False)") + LOGGER.info( + "Skipping Velero delete", + extra={ + "resource_kind": self.kind, + "resource_name": self.name, + "teardown": False, + }, + ) except Exception: - LOGGER.exception(f"Failed to delete Velero {self.kind} {self.name}") + LOGGER.exception( + "Failed to delete Velero resource", + extra={ + "resource_kind": self.kind, + "resource_name": self.name, + }, + ) finally: super().__exit__(exception_type, exception_value, traceback)Based on learnings, this is a refactoring PR and the author may prefer to address this separately.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
tests/data_protection/oadp/conftest.pytests/data_protection/oadp/test_velero.pytests/data_protection/oadp/utils.pytests/storage/storage_migration/utils.pyutilities/constants.pyutilities/oadp.pyutilities/unittests/test_oadp.py
💤 Files with no reviewable changes (1)
- tests/data_protection/oadp/utils.py
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: NEVER add linter suppressions like# noqa,# type: ignore,# pylint: disable— fix the code instead of suppressing linter errors
Google-format docstrings are REQUIRED for all public functions with non-obvious return values OR side effects
ALWAYS use absolute imports — NEVER use relative imports in Python files
ALWAYS import specific functions — usefrom module import func, NEVER useimport module
ALWAYS use named arguments for function calls with more than one argument
NEVER use single-letter variable names — ALWAYS use descriptive, meaningful names
No dead code — every function, variable, fixture MUST be used or removed. Code marked with# skip-unused-codeis excluded from dead code analysis
Prefer direct attribute access — usefoo.attrdirectly. Save to variables only when reusing the same attribute multiple times improves readability or extracting clarifies intent
INFO level logging REQUIRED for — test phase transitions, resource creation/deletion, configuration changes, API responses, intermediate state
ERROR level logging REQUIRED for — exceptions with full context: what failed, expected vs actual values, resource state
NEVER use DEBUG level logging — if a log is needed, use INFO
NEVER log secrets, tokens, passwords, or PII
Log format REQUIRED — structured key-value pairs for searchability using format likeLOGGER.info("VM created", extra={"name": vm_name, "namespace": ns})
ALWAYS re-raise exceptions with context — useraise NewError("message") from original_errorto preserve stack trace
Do not catch bareException— catch specific exception types only
NEVER silently swallow exceptions — at minimum, log the error before continuing
ALWAYS usewithfor resources — files, connections, locks MUST use context managers
ALWAYS usetimeout_samplerfromtimeout_samplerpackage for any operation that waits for a condition, NEVER usetime.sleep()in loops
Use implicit boolean checks —if items:NOTif len(items) > 0:or `if items...
Files:
utilities/oadp.pyutilities/unittests/test_oadp.pytests/storage/storage_migration/utils.pyutilities/constants.pytests/data_protection/oadp/test_velero.pytests/data_protection/oadp/conftest.py
{libs,utilities}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Type hints are MANDATORY — mypy strict mode in
libs/, all new public functions underutilities/MUST be typed
Files:
utilities/oadp.pyutilities/unittests/test_oadp.pyutilities/constants.py
**/test_*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/test_*.py: All new tests MUST have markers — check pytest.ini for available markers, NEVER commit unmarked tests
Each test verifies ONE aspect only — single purpose, easy to understand
Tests MUST be independent — usepytest-dependencyONLY when test B requires side effects from test A. When using@pytest.mark.dependency, a comment explaining WHY the dependency exists is REQUIRED
ALWAYS use@pytest.mark.usefixtureswhen fixture return value is not used by test
Use pytest assertions —assert actual == expected, NEVER useself.assertEqual()
Include failure messages in assertions —assert condition, "descriptive message explaining failure"
Files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/test_velero.py
**/conftest.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/conftest.py: Fixtures MUST do ONE action only — single responsibility principle required
Fixture naming REQUIRED — ALWAYS use NOUNS (what they provide), NEVER verbs. Use descriptive names likevm_with_diskinstead ofcreate_vm_with_disk
Fixture parametrization format — userequest.paramwith dict structure for complex parameters
Fixture ordering REQUIRED — pytest native fixtures first, then session-scoped, then module/class/function scoped
Fixture scope rules — usescope="function"for setup requiring test isolation,scope="class"for setup shared across test class,scope="module"for expensive setup in a test module,scope="session"for setup persisting entire test run. NEVER use broader scope if fixture modifies state or creates per-test resources
Fixtures with cleanup MUST useyield— useyield resourcefollowed by cleanup code, NEVER return + finalizer
Files:
tests/data_protection/oadp/conftest.py
🧠 Learnings (58)
📓 Common learnings
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-22T15:56:00.157Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, do NOT use "REQUEST_CHANGES" review type if the PR author has already marked the PR as verified (e.g., with `/verified` command). Test execution plans are informational guides, not blocking requirements. Use COMMENT event for informational test plans, or only REQUEST_CHANGES if there are actual code issues that need to be addressed before merging.
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 3301
File: utilities/unittests/test_oadp.py:5-5
Timestamp: 2026-01-07T11:52:04.988Z
Learning: In utilities/unittests/test_oadp.py (and similar test files in openshift-virtualization-tests), the `# flake8: noqa: E402` directive is required by pre-commit hooks, even when Ruff reports it as unused. This is needed because mock setup must precede imports to prevent circular dependencies, triggering E402 warnings that must be suppressed.
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:00:59.076Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, CodeRabbit must post ONLY an inline review comment on the Files Changed tab and then stop immediately without generating any follow-up comments in the PR discussion thread. No acknowledgment messages, no confirmation of posting, no explanation - silence after posting the inline review equals success. Additional comments create empty/meaningless reviews that clutter the PR.
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1678
File: utilities/oadp.py:5-5
Timestamp: 2025-08-26T14:51:22.273Z
Learning: qwang1 contributed the Backup resource to openshift-python-wrapper and the PR was merged. They use uv for dependency management in openshift-virtualization-tests.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T10:28:54.212Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for test execution plans, only set "Run smoke tests: True" if there is a verified, traceable dependency path from smoke tests to the changed code. Session-scoped fixtures or infrastructure-sounding changes do NOT automatically imply smoke test impact - the dependency chain must be explicitly verified using shell scripts before recommending smoke test execution.
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3228
File: .coderabbit.yaml:30-41
Timestamp: 2026-01-05T10:33:55.037Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers minimal pre-merge checks in CodeRabbit configuration: only docstrings enforcement (80% threshold) is needed, not title or description checks.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3062
File: conftest.py:333-333
Timestamp: 2025-12-16T20:11:03.645Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for changes affecting py_config["os_login_param"], follow this verification methodology: (1) Find all smoke tests: `rg "pytest.mark.smoke" --type=py -B2 | grep "def test_"`, (2) For each smoke test file, search for VM creation patterns: `rg "VirtualMachineForTests|running_vm|VirtualMachineForTestsFromTemplate|wait_for_ssh|check_ssh_connectivity"`, (3) Trace the dependency chain: smoke test → VirtualMachineForTests/running_vm() → wait_for_ssh_connectivity() (default enabled) → vm.login_params property → py_config["os_login_param"][vm.os_flavor], (4) Check utilities/virt.py for login_params usage: `rg "os_login_param|login_params" utilities/virt.py -C3`. Any smoke test creating VMs with default SSH connectivity checks (running_vm with check_ssh_connectivity=True) depends on os_login_param, even if the test doesn't directly reference it.
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1678
File: utilities/oadp.py:1-5
Timestamp: 2025-09-07T13:16:32.011Z
Learning: In the openshift-virtualization-tests project utilities/oadp.py, DynamicClient instances are passed as parameters rather than created internally, so kubernetes.config import is not needed for client creation fallback patterns.
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.845Z
Learning: Move fixtures and helpers to shared location — move to `utilities/` or `tests/conftest.py` ONLY when used by different team directories
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 2014
File: tests/storage/cdi_clone/test_clone.py:0-0
Timestamp: 2025-09-10T13:25:59.799Z
Learning: In the openshift-virtualization-tests repository, when creating DataVolume objects using ocp_resources, use `api_name="storage"` rather than the default `api_name="pvc"`.
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 2920
File: utilities/network.py:31-31
Timestamp: 2025-12-07T13:52:56.070Z
Learning: In the openshift-virtualization-tests repository, the utilities/ package is not intended to be used as a valid, structured module with clear focus and ownership. The team prefers using specialized library locations like tests/network/libs/ for network-related helper code rather than placing everything in utilities/.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1028
File: utilities/infra.py:1257-1258
Timestamp: 2025-08-06T13:57:34.740Z
Learning: In the openshift-virtualization-tests repository, all existing calls to the `get_cluster_platform` function in utilities/infra.py already pass the `admin_client` parameter, so the breaking change requiring this parameter does not actually break any existing code.
📚 Learning: 2026-01-06T20:32:10.845Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.845Z
Learning: Applies to {libs,utilities}/**/*.py : Type hints are MANDATORY — mypy strict mode in `libs/`, all new public functions under `utilities/` MUST be typed
Applied to files:
utilities/oadp.py
📚 Learning: 2026-01-06T20:32:10.845Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.845Z
Learning: Before writing new code, search the codebase for existing implementations in `utilities/`, `libs/`, and `tests/` directories — reuse existing code and patterns, extract duplicated logic to shared modules
Applied to files:
utilities/oadp.py
📚 Learning: 2025-06-03T12:36:36.982Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:68-74
Timestamp: 2025-06-03T12:36:36.982Z
Learning: In tests/storage/storage_migration/test_mtc_storage_class_migration.py, the broad Exception catching in the VM migration test is intentional - the user wants to catch every exception and save it for future triage rather than letting specific exceptions bubble up.
Applied to files:
utilities/oadp.py
📚 Learning: 2025-10-30T10:43:48.886Z
Learnt from: josemacassan
Repo: RedHatQE/openshift-virtualization-tests PR: 2300
File: tests/storage/online_resize/conftest.py:94-96
Timestamp: 2025-10-30T10:43:48.886Z
Learning: In tests/storage/online_resize/conftest.py, the `running_rhel_vm` fixture is intentionally designed to only invoke `running_vm(vm=rhel_vm_for_online_resize)` without returning or yielding a value. It's used as a dependency fixture to ensure the VM is running before other fixtures/tests execute, while the actual VM object is accessed via the `rhel_vm_for_online_resize` fixture.
Applied to files:
utilities/oadp.pytests/data_protection/oadp/test_velero.py
📚 Learning: 2025-09-03T07:23:37.045Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/test_windows_os.py:30-31
Timestamp: 2025-09-03T07:23:37.045Z
Learning: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, test_vm_deletion should depend on test_create_vm (not test_start_vm) to ensure resource cleanup happens even if the VM fails to start, preventing resource leaks in the test environment.
Applied to files:
utilities/oadp.pyutilities/unittests/test_oadp.py
📚 Learning: 2025-09-03T07:22:50.446Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/test_windows_os.py:27-28
Timestamp: 2025-09-03T07:22:50.446Z
Learning: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, Windows guest images are configured with SSH support (likely through WSL2), so running_vm() should keep the default check_ssh_connectivity=True to verify SSH connectivity is working properly in the Windows guest images.
Applied to files:
utilities/oadp.pyutilities/unittests/test_oadp.py
📚 Learning: 2025-09-29T20:33:51.007Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:65-72
Timestamp: 2025-09-29T20:33:51.007Z
Learning: In tests/virt/node/migration_and_maintenance/conftest.py, the added_vm_cpu_limit fixture doesn't require ResourceEditor as a context manager because it's the final test to modify the vm_for_multifd_test VM before teardown, so restoration of CPU limits is unnecessary overhead as confirmed by maintainer dshchedr.
Applied to files:
utilities/oadp.pytests/storage/storage_migration/utils.py
📚 Learning: 2025-12-29T10:07:12.804Z
Learnt from: hmeir
Repo: RedHatQE/openshift-virtualization-tests PR: 3081
File: tests/install_upgrade_operators/product_install/test_install_openshift_virtualization.py:189-191
Timestamp: 2025-12-29T10:07:12.804Z
Learning: In test_ocs_virt_default_storage_class in tests/install_upgrade_operators/product_install/test_install_openshift_virtualization.py, consecutive_checks_count=3 is sufficient for verify_boot_sources_reimported because datasources update quickly in the installation lane scenario after storage class changes.
Applied to files:
utilities/oadp.pytests/storage/storage_migration/utils.py
📚 Learning: 2025-12-16T20:11:03.645Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3062
File: conftest.py:333-333
Timestamp: 2025-12-16T20:11:03.645Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for changes affecting py_config["os_login_param"], follow this verification methodology: (1) Find all smoke tests: `rg "pytest.mark.smoke" --type=py -B2 | grep "def test_"`, (2) For each smoke test file, search for VM creation patterns: `rg "VirtualMachineForTests|running_vm|VirtualMachineForTestsFromTemplate|wait_for_ssh|check_ssh_connectivity"`, (3) Trace the dependency chain: smoke test → VirtualMachineForTests/running_vm() → wait_for_ssh_connectivity() (default enabled) → vm.login_params property → py_config["os_login_param"][vm.os_flavor], (4) Check utilities/virt.py for login_params usage: `rg "os_login_param|login_params" utilities/virt.py -C3`. Any smoke test creating VMs with default SSH connectivity checks (running_vm with check_ssh_connectivity=True) depends on os_login_param, even if the test doesn't directly reference it.
Applied to files:
utilities/oadp.pyutilities/unittests/test_oadp.pytests/data_protection/oadp/test_velero.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-09-07T13:16:32.011Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1678
File: utilities/oadp.py:1-5
Timestamp: 2025-09-07T13:16:32.011Z
Learning: In the openshift-virtualization-tests project utilities/oadp.py, DynamicClient instances are passed as parameters rather than created internally, so kubernetes.config import is not needed for client creation fallback patterns.
Applied to files:
utilities/oadp.pyutilities/unittests/test_oadp.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-08-06T13:57:34.740Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1028
File: utilities/infra.py:1257-1258
Timestamp: 2025-08-06T13:57:34.740Z
Learning: In the openshift-virtualization-tests repository, all existing calls to the `get_cluster_platform` function in utilities/infra.py already pass the `admin_client` parameter, so the breaking change requiring this parameter does not actually break any existing code.
Applied to files:
utilities/oadp.py
📚 Learning: 2026-01-06T20:32:10.845Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.845Z
Learning: Applies to **/*.py : Log format REQUIRED — structured key-value pairs for searchability using format like `LOGGER.info("VM created", extra={"name": vm_name, "namespace": ns})`
Applied to files:
utilities/oadp.py
📚 Learning: 2026-01-06T20:32:10.845Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.845Z
Learning: Applies to **/*.py : ERROR level logging REQUIRED for — exceptions with full context: what failed, expected vs actual values, resource state
Applied to files:
utilities/oadp.pyutilities/unittests/test_oadp.py
📚 Learning: 2026-01-06T20:32:10.845Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.845Z
Learning: Applies to **/*.py : Google-format docstrings are REQUIRED for all public functions with non-obvious return values OR side effects
Applied to files:
utilities/oadp.py
📚 Learning: 2026-01-07T11:51:57.253Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 3301
File: utilities/unittests/test_oadp.py:5-5
Timestamp: 2026-01-07T11:51:57.253Z
Learning: In utilities/unittests/test_oadp.py and similar test files in this repo, include a # flake8: noqa: E402 directive to suppress E402 warnings caused by pre-commit checks. This is needed because mock setup must precede imports to avoid circular dependencies, and Ruff may report unused imports. Apply this directive to all similar test files that require imports to guard against circular import issues.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-12-07T13:52:56.070Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 2920
File: utilities/network.py:31-31
Timestamp: 2025-12-07T13:52:56.070Z
Learning: In the openshift-virtualization-tests repository, it is acceptable for utilities/ modules to import from tests/network/libs/ (e.g., utilities/network.py importing from tests/network/libs/sriovnetworknode). The libs modules under tests/network/libs/ are library/tool modules, not test code, and this import direction does not violate architectural principles in this codebase.
Applied to files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-11-26T16:03:07.813Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 2119
File: tests/network/localnet/conftest.py:372-387
Timestamp: 2025-11-26T16:03:07.813Z
Learning: In the openshift-virtualization-tests repository, pytest fixtures that declare other fixtures as parameters purely for dependency ordering (without referencing them in the function body) should not be modified to silence Ruff ARG001 warnings. This is an idiomatic pytest pattern for ensuring setup order, and the team prefers to leave such fixtures unchanged rather than adding defensive comments or code to suppress linter warnings.
Applied to files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-12-16T10:28:54.212Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T10:28:54.212Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for test execution plans, only set "Run smoke tests: True" if there is a verified, traceable dependency path from smoke tests to the changed code. Session-scoped fixtures or infrastructure-sounding changes do NOT automatically imply smoke test impact - the dependency chain must be explicitly verified using shell scripts before recommending smoke test execution.
Applied to files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-12-07T13:52:56.070Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 2920
File: utilities/network.py:31-31
Timestamp: 2025-12-07T13:52:56.070Z
Learning: In the openshift-virtualization-tests repository, the utilities/ package is not intended to be used as a valid, structured module with clear focus and ownership. The team prefers using specialized library locations like tests/network/libs/ for network-related helper code rather than placing everything in utilities/.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-08-09T01:46:48.039Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/node/workload_density/test_swap.py:71-85
Timestamp: 2025-08-09T01:46:48.039Z
Learning: In the openshift-virtualization-tests repository, user dshchedr prefers that all test setup fixtures be explicitly declared as parameters in the test method itself rather than chaining fixtures through dependencies. This makes all setup steps visible in one place at the test method level, improving test readability and making dependencies explicit, even if a fixture isn't directly used within another fixture's implementation.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-06-05T11:36:15.214Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 906
File: tests/observability/metrics/conftest.py:19-19
Timestamp: 2025-06-05T11:36:15.214Z
Learning: The `pyhelper_utils` package is available in the test execution environment for the openshift-virtualization-tests repository, even though static analysis tools like Pylint may not be able to import it. This is a custom/internal package used throughout the test suite and should not be flagged as a missing dependency.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-12-16T15:09:49.597Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3062
File: conftest.py:333-333
Timestamp: 2025-12-16T15:09:49.597Z
Learning: In the openshift-virtualization-tests repository, when conftest.py or utilities/bitwarden.py changes affect py_config["os_login_param"], smoke test impact must be determined by: (1) finding all smoke tests using `rg "pytest.mark.smoke"`, (2) checking each for VM creation patterns (VirtualMachineForTests, running_vm, VirtualMachineForTestsFromTemplate), (3) tracing whether running_vm is called with default check_ssh_connectivity=True, which accesses vm.login_params property that reads py_config["os_login_param"][vm.os_flavor]. The dependency chain is: smoke test → VM creation → running_vm → wait_for_ssh_connectivity → vm.login_params → os_login_param. Any smoke test creating VMs with SSH connectivity (the default) depends on os_login_param.
Applied to files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/test_velero.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-09-15T06:49:53.478Z
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.
Applied to files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-09-28T14:43:07.181Z
Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds the OS_FLAVOR attribute to the Images.Cirros class, making Images.Cirros.OS_FLAVOR available for conditional logic in tests.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-09-28T14:43:07.181Z
Learnt from: vamsikrishna-siddu
Repo: RedHatQE/openshift-virtualization-tests PR: 2199
File: tests/storage/test_online_resize.py:108-113
Timestamp: 2025-09-28T14:43:07.181Z
Learning: In the openshift-virtualization-tests repo, PR #2199 depends on PR #2139 which adds architecture-specific OS_FLAVOR attributes to the Images.Cirros class (OS_FLAVOR_CIRROS for x86_64/ARM64, OS_FLAVOR_FEDORA for s390x), enabling conditional logic based on the underlying OS flavor in tests.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-08-09T01:52:26.683Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Applied to files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-06-03T14:34:04.796Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1105
File: tests/virt/utils.py:395-402
Timestamp: 2025-06-03T14:34:04.796Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, user akri3i prefers not to add docstrings to utility functions in tests/virt/utils.py, indicating that docstrings are not required for this codebase.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-08-20T23:43:28.117Z
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: libs/net/node_network.py:25-31
Timestamp: 2025-08-20T23:43:28.117Z
Learning: In the RedHatQE/openshift-virtualization-tests project, servolkov's team always uses bare metal (BM) clusters with IPv4 setup in their testing environment, making defensive checks for IPv4 data presence potentially redundant in their networking code.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-06-18T09:31:06.311Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:31:06.311Z
Learning: In tests/observability/metrics/conftest.py, ResourceEditor context managers automatically restore VM configuration when the context exits, including nodeSelector patches. The fixture pattern with `with ResourceEditor(patches={vm: {...}})` followed by `yield` properly restores the VM to its original state without requiring manual teardown logic.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2026-01-04T19:37:57.919Z
Learnt from: SamAlber
Repo: RedHatQE/openshift-virtualization-tests PR: 3027
File: tests/install_upgrade_operators/strict_reconciliation/test_hco_default_cpu_model.py:123-148
Timestamp: 2026-01-04T19:37:57.919Z
Learning: In tests/install_upgrade_operators/strict_reconciliation/test_hco_default_cpu_model.py, the test pattern intentionally uses class-scoped fixtures to modify HCO resources (e.g., hco_with_default_cpu_model_set patching hyperconverged_resource_scope_class) while function-scoped fixtures (e.g., hyperconverged_resource_scope_function) fetch fresh cluster state in tests. This verifies that HCO reconciliation has propagated correctly by comparing the fresh state with the expected value set by the class-scoped modification.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-05-18T09:24:43.335Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 990
File: utilities/virt.py:1704-1710
Timestamp: 2025-05-18T09:24:43.335Z
Learning: The `migrate_vm_and_verify` function in utilities/virt.py has inconsistent return behavior - it returns a VirtualMachineInstanceMigration object when wait_for_migration_success=False and returns None when wait_for_migration_success=True. This has been properly typed but should be refactored in a future PR for better API design.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-06-23T19:24:28.327Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/test_mass_machine_type_transition.py:97-104
Timestamp: 2025-06-23T19:24:28.327Z
Learning: In OpenShift Virtualization machine type transition tests, the test_machine_type_transition_without_restart method with restart_required=false parameter validates that VM machine types do NOT change when the lifecycle job runs with restart disabled, so the assertion should check against the original machine type rather than the target machine type.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-08-14T10:28:22.958Z
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 1785
File: tests/virt/cluster/common_templates/utils.py:65-66
Timestamp: 2025-08-14T10:28:22.958Z
Learning: The restart_qemu_guest_agent_service function in tests/virt/cluster/common_templates/utils.py is only called for RHEL7 VMs, with OS version checks handled by the calling test code, not within the function itself. Guest agent functionality is verified by subsequent tests in the test class.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-05-26T13:36:16.136Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 584
File: tests/observability/metrics/conftest.py:1136-1136
Timestamp: 2025-05-26T13:36:16.136Z
Learning: For Windows VM testing in metrics tests, the `verify_wsl2_guest_works(vm=vm)` call in the `windows_vm_for_test` fixture uses a 60-second timeout as a deliberate "fail fast" validation. This upfront check prevents more expensive failures later since Windows VM deployment is time-consuming, and WSL2 functionality is critical for the Windows VM tests to work properly.
Applied to files:
utilities/unittests/test_oadp.pytests/storage/storage_migration/utils.pytests/data_protection/oadp/test_velero.py
📚 Learning: 2025-05-27T11:44:14.859Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 584
File: tests/observability/metrics/test_network_metrics.py:62-76
Timestamp: 2025-05-27T11:44:14.859Z
Learning: The windows_vm_for_test fixture in tests/observability/metrics/conftest.py does not have a request argument, so it cannot be parametrized using pytest.mark.parametrize with indirect=True. This is different from vm_for_test fixture which accepts parameters through parametrization.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2026-01-06T20:32:10.845Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.845Z
Learning: Applies to **/*.py : No dead code — every function, variable, fixture MUST be used or removed. Code marked with `# skip-unused-code` is excluded from dead code analysis
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2026-01-06T20:32:10.845Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.845Z
Learning: Before committing, verification checks MUST pass: `pre-commit run --all-files` (linting and formatting) and `tox` (full CI checks) and `tox -e utilities-unittests` (utilities unit tests). Do not use `--no-verify` to bypass hooks
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2026-01-05T10:33:55.037Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3228
File: .coderabbit.yaml:30-41
Timestamp: 2026-01-05T10:33:55.037Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers minimal pre-merge checks in CodeRabbit configuration: only docstrings enforcement (80% threshold) is needed, not title or description checks.
Applied to files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-12-16T14:00:59.076Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:00:59.076Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, CodeRabbit must post ONLY an inline review comment on the Files Changed tab and then stop immediately without generating any follow-up comments in the PR discussion thread. No acknowledgment messages, no confirmation of posting, no explanation - silence after posting the inline review equals success. Additional comments create empty/meaningless reviews that clutter the PR.
Applied to files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-11-08T07:36:57.616Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Applied to files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-06-03T14:39:26.630Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1105
File: tests/virt/upgrade_custom/vgpu/test_vgpu_vm_upgrade.py:52-54
Timestamp: 2025-06-03T14:39:26.630Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, line lengths that pass ruff format checks in pre-commit are acceptable, even if they exceed Pylint's 100-character limit. The project uses ruff for formatting/linting with a more permissive line length configuration.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-12-22T15:56:00.157Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-22T15:56:00.157Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, do NOT use "REQUEST_CHANGES" review type if the PR author has already marked the PR as verified (e.g., with `/verified` command). Test execution plans are informational guides, not blocking requirements. Use COMMENT event for informational test plans, or only REQUEST_CHANGES if there are actual code issues that need to be addressed before merging.
Applied to files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-06-03T14:38:59.401Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1105
File: tests/virt/upgrade_custom/vgpu/test_vgpu_vm_upgrade.py:37-39
Timestamp: 2025-06-03T14:38:59.401Z
Learning: The openshift-virtualization-tests project uses ruff for code formatting in pre-commit hooks, which allows longer line lengths than Pylint's default 100-character limit.
Applied to files:
utilities/unittests/test_oadp.py
📚 Learning: 2025-06-10T09:41:58.035Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1168
File: tests/storage/storage_migration/utils.py:92-99
Timestamp: 2025-06-10T09:41:58.035Z
Learning: When CodeRabbit flags technical issues in code reviews, do not resolve the comments without first addressing them and getting a response from the user. Proper procedure is to discuss the technical issues and proposed solutions before marking comments as resolved.
Applied to files:
utilities/unittests/test_oadp.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-12-15T12:33:06.686Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 3024
File: tests/network/connectivity/utils.py:17-17
Timestamp: 2025-12-15T12:33:06.686Z
Learning: In the test suite, ensure the ipv6_network_data fixture returns a factory function (Callable) and that all call sites invoke it to obtain the actual data dict, i.e., use ipv6_network_data() at call sites. This enables future extensibility for configuring secondary interfaces' IP addresses without changing call sites. Apply this pattern to all Python test files under tests.
Applied to files:
tests/storage/storage_migration/utils.pytests/data_protection/oadp/test_velero.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-12-22T16:27:40.244Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 3196
File: tests/network/upgrade/test_upgrade_network.py:4-4
Timestamp: 2025-12-22T16:27:40.244Z
Learning: For PRs that remove tests, rely on pytest --collect-only to verify the test discovery results (which tests are selected/deselected) and ensure the removal is clean and the test module remains functional. Full test execution is not required for test deletion PRs. This guideline applies to test files anywhere under the tests/ directory (e.g., tests/network/upgrade/test_upgrade_network.py) and should be used for similar test-deletion scenarios across the repository.
Applied to files:
tests/storage/storage_migration/utils.pytests/data_protection/oadp/test_velero.pytests/data_protection/oadp/conftest.py
📚 Learning: 2026-01-07T09:52:12.342Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 3358
File: tests/network/sriov/test_sriov.py:21-21
Timestamp: 2026-01-07T09:52:12.342Z
Learning: When a PR only removes or modifies pytest markers in tests (e.g., removing pytest.mark.post_upgrade) and the test logic remains unchanged, prefer verifying with pytest --collect-only instead of running the full test suite. This validates that marker usage and test selection behavior are preserved. If the test logic changes, or markers affect behavior beyond collection, run the full test suite to confirm.
Applied to files:
tests/storage/storage_migration/utils.pytests/data_protection/oadp/test_velero.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-09-17T14:56:18.111Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1702
File: tests/chaos/oadp/conftest.py:23-45
Timestamp: 2025-09-17T14:56:18.111Z
Learning: In tests/chaos/oadp/conftest.py for the openshift-virtualization-tests repository, user qwang1 confirmed that py_config["latest_rhel_os_dict"]["data_source"] can be accessed directly without defensive .get() calls because the test configuration setup guarantees these keys will always be present in their test execution environment.
Applied to files:
utilities/constants.py
📚 Learning: 2025-10-16T12:47:04.521Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 2249
File: tests/install_upgrade_operators/must_gather/test_must_gather.py:428-441
Timestamp: 2025-10-16T12:47:04.521Z
Learning: In openshift-virtualization-tests repository, DataVolumes in the openshift-virtualization-os-images namespace are volatile resources managed by DataImportCron. They can be created/destroyed between must-gather collection listing and file retrieval, requiring FileNotFoundError exception handling in test_crd_resources to skip these volatile resources gracefully while still validating DataVolumes in other namespaces. There is no pytest_generate_tests hook that filters out datavolumes from test parametrization.
Applied to files:
tests/data_protection/oadp/test_velero.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-09-10T13:25:59.799Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 2014
File: tests/storage/cdi_clone/test_clone.py:0-0
Timestamp: 2025-09-10T13:25:59.799Z
Learning: In the openshift-virtualization-tests repository, when creating DataVolume objects using ocp_resources, use `api_name="storage"` rather than the default `api_name="pvc"`.
Applied to files:
tests/data_protection/oadp/test_velero.pytests/data_protection/oadp/conftest.py
📚 Learning: 2025-05-21T09:41:46.532Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/conftest.py:156-176
Timestamp: 2025-05-21T09:41:46.532Z
Learning: In the RedHatQE/openshift-virtualization-tests codebase, the pattern of calling to_dict() on a DataVolume object and then accessing dv.res to populate data_volume_template is the preferred approach rather than building the template dict explicitly.
Applied to files:
tests/data_protection/oadp/test_velero.py
📚 Learning: 2025-06-18T09:21:34.315Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:21:34.315Z
Learning: In tests/observability/metrics/conftest.py, when creating fixtures that modify shared Windows VM state (like changing nodeSelector), prefer using function scope rather than class scope to ensure ResourceEditor context managers properly restore the VM state after each test, maintaining test isolation while still reusing expensive Windows VM fixtures.
Applied to files:
tests/data_protection/oadp/conftest.py
📚 Learning: 2026-01-07T11:52:04.988Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 3301
File: utilities/unittests/test_oadp.py:5-5
Timestamp: 2026-01-07T11:52:04.988Z
Learning: In utilities/unittests/test_oadp.py (and similar test files in openshift-virtualization-tests), the `# flake8: noqa: E402` directive is required by pre-commit hooks, even when Ruff reports it as unused. This is needed because mock setup must precede imports to prevent circular dependencies, triggering E402 warnings that must be suppressed.
Applied to files:
tests/data_protection/oadp/conftest.py
📚 Learning: 2025-12-16T14:06:22.391Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:06:22.391Z
Learning: In the openshift-virtualization-tests repository, when posting test execution plan inline review comments using GitHub API, the full test execution plan content must go in the `comments[].body` field (which appears on Files Changed tab), NOT in the top-level `body` field (which appears in PR discussion thread). The top-level `body` field should be omitted or left empty to avoid posting redundant comments in the PR discussion thread.
Applied to files:
tests/data_protection/oadp/conftest.py
📚 Learning: 2025-08-27T12:38:26.496Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1723
File: conftest.py:496-502
Timestamp: 2025-08-27T12:38:26.496Z
Learning: In PR #1723, the user rnetser clarified that updating downstream string comparisons to handle DataVolume enums from StorageClassConfig is not part of the current PR scope, which focuses only on adding conformance storage class support.
Applied to files:
tests/data_protection/oadp/conftest.py
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: The test execution plan for PR #1904 focuses on cluster-type conditional logic where nmstate functionality is bypassed on cloud clusters (Azure/AWS) but fully functional on bare-metal/PSI clusters, requiring different test strategies for each environment type.
Applied to files:
tests/data_protection/oadp/conftest.py
📚 Learning: 2026-01-06T11:47:20.240Z
Learnt from: Ahmad-Hafe
Repo: RedHatQE/openshift-virtualization-tests PR: 3334
File: tests/storage/test_data_import_cron.py:212-215
Timestamp: 2026-01-06T11:47:20.240Z
Learning: In the openshift-virtualization-tests repository, when quarantining flaky tests, the team uses two separate Jira tickets: one for the quarantine action itself (referenced in the PR description) and another for tracking and investigating the underlying issue (referenced in the xfail marker reason). Both references are intentional and serve different purposes.
Applied to files:
tests/data_protection/oadp/conftest.py
🧬 Code graph analysis (3)
utilities/oadp.py (4)
libs/vm/vm.py (1)
console(61-66)utilities/infra.py (1)
unique_name(814-817)utilities/virt.py (1)
VirtualMachineForTests(203-1106)utilities/console.py (1)
Console(19-135)
tests/storage/storage_migration/utils.py (1)
utilities/oadp.py (1)
check_file_in_vm(261-313)
tests/data_protection/oadp/test_velero.py (3)
tests/data_protection/oadp/utils.py (1)
wait_for_restored_dv(13-15)utilities/oadp.py (1)
check_file_in_vm(261-313)tests/data_protection/oadp/conftest.py (3)
rhel_vm_with_data_volume_template(103-134)rhel_vm_from_existing_dv(166-182)velero_restore_second_namespace_with_datamover(248-259)
🪛 Ruff (0.14.10)
utilities/oadp.py
208-208: Boolean-typed positional argument in function definition
(FBT001)
208-208: Boolean default positional argument in function definition
(FBT002)
210-210: Boolean-typed positional argument in function definition
(FBT001)
210-210: Boolean default positional argument in function definition
(FBT002)
212-212: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
utilities/unittests/test_oadp.py
5-5: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
27-27: Unused noqa directive (non-enabled: E402)
Remove unused noqa directive
(RUF100)
701-701: Unused method argument: mock_delete_resource
(ARG002)
tests/data_protection/oadp/test_velero.py
98-98: Unused function argument: velero_restore_second_namespace_with_datamover
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build-container
- GitHub Check: tox
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
🔇 Additional comments (17)
utilities/constants.py (1)
985-985: LGTM - Constant value update.The
BACKUP_STORAGE_LOCATIONconstant has been updated from"dpa-1"to"ts-dpa-1". This aligns with the backup storage location naming used in the OADP fixtures.tests/storage/storage_migration/utils.py (1)
13-14: LGTM - Centralized utility import.The
check_file_in_vmfunction is now imported fromutilities.oadpinstead of being defined locally, aligning with the PR's goal to consolidate OADP utilities. The function signature usage at line 67 correctly passesfile_nameandfile_contentparameters.utilities/unittests/test_oadp.py (5)
5-5: Retain thenoqadirective as required by pre-commit.Ruff reports this directive as unused (RUF100), but per repository learnings, the
# flake8: noqa: E402is required by pre-commit hooks. The mock setup at lines 15-20 must precede the imports to prevent circular dependencies, which triggers E402 warnings that must be suppressed.
14-26: LGTM - Proper mock setup for circular import prevention.The mock setup correctly patches
utilities.virtandutilities.infrabefore importing fromutilities.oadp. ImportingLS_COMMANDandTIMEOUT_20SECfromutilities.constantsensures the tests validate against the actual constants used in production code.
700-724: Ignore Ruff ARG002 - mock parameter is required for patching.Ruff flags
mock_delete_resourceas unused (ARG002), but this is a false positive. The parameter is a@patchdecorator target that must be accepted to preventdelete_velero_resourcefrom being called during the test. The mock correctly verifies that deletion is skipped whenteardown=False.
773-793: LGTM - Well-structured test for check_file_in_vm.The test correctly validates the console interaction sequence:
- VM's
startis not called when VM is readyLS_COMMANDis sent to list filesfile_nameis expected in outputcat {file_name}is sentfile_contentis expected in outputThis matches the implementation in
utilities/oadp.py.
796-823: LGTM - Comprehensive tests for is_storage_class_support_volume_mode.Both positive and negative cases are covered:
test_volume_mode_supported: ReturnsTruewhen requested mode is in claim_property_setstest_volume_mode_not_supported: ReturnsFalsewhen requested mode is not presenttests/data_protection/oadp/conftest.py (2)
15-20: LGTM - Imports updated to use centralized utilities.The imports correctly reference
VeleroRestoreandis_storage_class_support_volume_modefromutilities.oadp, aligning with the PR's goal to consolidate OADP utilities outside the tests directory.
110-114: LGTM - Function call updated with named argument.The call to
is_storage_class_support_volume_modenow usesadmin_client=admin_clientas the first argument, matching the updated function signature inutilities/oadp.py. This follows the coding guideline to use named arguments for function calls with more than one argument.tests/data_protection/oadp/test_velero.py (3)
4-11: LGTM - Imports updated to use centralized utilities and constants.The test file now imports:
check_file_in_vmfromutilities.oadp(moved from local utils)FILE_NAME_FOR_BACKUP,TEXT_TO_TESTfromutilities.constantsThis aligns with the PR objective to consolidate OADP-related code in
utilities/oadp.py.
62-62: LGTM - Updated function call with explicit parameters.The
check_file_in_vmcall now correctly passesfile_name=FILE_NAME_FOR_BACKUPandfile_content=TEXT_TO_TEST, matching the new function signature and following the coding guideline to use named arguments.
98-99: Ignore Ruff ARG001 - fixture parameter is required for test ordering.Ruff flags
velero_restore_second_namespace_with_datamoveras unused (ARG001), but this is an idiomatic pytest pattern. The fixture is declared as a parameter purely for dependency ordering—it ensures the Velero restore operation completes before the test runs. Per repository learnings, such fixtures should not be modified to suppress linter warnings.utilities/oadp.py (5)
1-4: LGTM - Proper imports for new functionality.The new imports support the added functionality:
Generatorfor the context manager type hintSelffor the__enter__return type (PEP 673)Restore,StorageProfilefor the new classes/functionsconsole,LS_COMMAND,TIMEOUT_20SECforcheck_file_in_vm
37-40: LGTM - Type-annotated utility function.The
delete_velero_resourcefunction now has proper type hints as required for utilities. The union typeBackup | Restorecorrectly reflects that this function handles both resource types.
148-258: LGTM - Well-documented VeleroRestore class with proper type hints.The
VeleroRestoreclass:
- ✅ Has comprehensive Google-format docstring explaining behavior, args, and usage example
- ✅ Uses
Selfreturn type for__enter__(PEP 673)- ✅ Follows the same pattern as
VeleroBackupfor consistency- ✅ Uses structured logging with
extra={}per coding guidelines- ✅ Properly delegates to superclass in
finallyblockThe Ruff warnings (FBT001/FBT002 for boolean args, ANN401 for
**kwargs: Any) are acceptable here—they match the existingVeleroBackuppattern andAnyis appropriate for pass-through kwargs.
261-313: LGTM - Well-implemented check_file_in_vm function.The function:
- ✅ Has proper type hints as required for
utilities/- ✅ Documents the expected exceptions (
pexpect.TIMEOUT,pexpect.EOF) in the docstring- ✅ Uses structured logging with
extra={}at each step- ✅ Uses context manager for console session (proper resource cleanup)
- ✅ Uses constants
LS_COMMANDandTIMEOUT_20SECfromutilities.constants
316-344: LGTM - Properly typed is_storage_class_support_volume_mode function.The function:
- ✅ Has full type hints as required for
utilities/- ✅ Has Google-format docstring with args, returns, and example
- ✅ Simple and clear logic iterating over claim_property_sets
One minor observation: the function could use
any()for a more Pythonic approach, but the current implementation is clear and correct.
Contributor
Author
|
/verified Cluster Info: mtv81 with OCP 4.20.8, CNV 4.20.4, OADP 1.5.3, ODF 4.20.4 Unit test Result: Regression test Result： |
Contributor
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Your deliverable: Test Execution Plan
Guidelines:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL: Before setting "Run smoke tests: True", you MUST use shell scripts to: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2179 🧠 Learnings used |
sarahbx
approved these changes
Jan 8, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 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.
Short description:
For the upcoming oadp-chaos test cases, because
Import from tests is not allowed.More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
Summary by CodeRabbit
Refactor
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.