WIP: [T2] [storage] adjust pvc size to match minimumSupportedPvcSize in T2#3243
Closed
Ahmad-Hafe wants to merge 1 commit intoRedHatQE:mainfrom
Closed
WIP: [T2] [storage] adjust pvc size to match minimumSupportedPvcSize in T2#3243Ahmad-Hafe wants to merge 1 commit intoRedHatQE:mainfrom
Ahmad-Hafe wants to merge 1 commit intoRedHatQE:mainfrom
Conversation
Contributor
WalkthroughTests and utilities were modified to read PVC/DataVolume sizes from StorageProfile annotations using a new utility function; fixtures and helpers were updated to accept an admin/client parameter where needed and size-parsing now uses bitmath. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Contributor
|
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. |
openshift-virtualization-qe-bot-3
requested review from
dalia-frank,
duyanyan,
josemacassan and
kgoldbla
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
previously approved these changes
Dec 24, 2025
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/storage/cdi_upload/test_upload_virtctl.py (1)
272-296: Scope mismatch between fixture dependencies.The
empty_pvcfixture usesstorage_class_matrix__module__(line 275) for storage class configuration, butstorage_profile_minimum_supported_pvc_size(line 278) is function-scoped and reads fromstorage_class_matrix__function__. This means the minimum PVC size may be retrieved from a different storage class than the one used to create the PVC.Once the module-scoped fixture is added in
conftest.py, update this fixture to use it:🔎 Proposed fix
@pytest.fixture() def empty_pvc( namespace, storage_class_matrix__module__, storage_class_name_scope_module, worker_node1, - storage_profile_minimum_supported_pvc_size, + storage_profile_minimum_supported_pvc_size_scope_module, ): with PersistentVolumeClaim( name="empty-pvc", namespace=namespace.name, storage_class=storage_class_name_scope_module, volume_mode=storage_class_matrix__module__[storage_class_name_scope_module]["volume_mode"], accessmodes=storage_class_matrix__module__[storage_class_name_scope_module]["access_mode"], - size=storage_profile_minimum_supported_pvc_size, + size=storage_profile_minimum_supported_pvc_size_scope_module, hostpath_node=worker_node1.name if sc_is_hpp_with_immediate_volume_binding(sc=storage_class_name_scope_module) else None, ) as pvc:
🧹 Nitpick comments (1)
tests/storage/conftest.py (1)
593-593: Consider usingnext(iter(...))for clarity.Per static analysis (Ruff RUF015), prefer
next(iter(storage_class_matrix__function__))over the slice approach.🔎 Proposed fix
@pytest.fixture(scope="function") def storage_profile_minimum_supported_pvc_size(storage_class_matrix__function__): - storage_profile = StorageProfile(name=[*storage_class_matrix__function__][0]) + storage_profile = StorageProfile(name=next(iter(storage_class_matrix__function__))) return storage_profile.instance.metadata.annotations.get("cdi.kubevirt.io/minimumSupportedPvcSize", "1Gi")Note: The existing pattern
[*dict][0]is used elsewhere in this file (lines 557, 562, 567, 572), so maintaining consistency is also valid.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.py
🧰 Additional context used
🧠 Learnings (12)
📓 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: 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: 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.
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: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.
📚 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/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.py
📚 Learning: 2025-06-17T07:45:37.776Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.
Applied to files:
tests/storage/cdi_upload/test_upload_virtctl.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/storage/cdi_upload/test_upload_virtctl.pytests/storage/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:
tests/storage/cdi_upload/test_upload_virtctl.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:
tests/storage/cdi_upload/test_upload_virtctl.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/cdi_upload/test_upload_virtctl.pytests/storage/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/cdi_upload/test_upload_virtctl.pytests/storage/conftest.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/storage/conftest.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:
tests/storage/conftest.py
📚 Learning: 2025-08-13T06:27:29.727Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1641
File: tests/infrastructure/golden_images/test_common_templates_data_volumes.py:47-48
Timestamp: 2025-08-13T06:27:29.727Z
Learning: In tests/infrastructure/golden_images/test_common_templates_data_volumes.py, when set_storage_class is True but get_diff_storage_class_from_matrix returns None (indicating only one storage class is available), the test should continue using the default storage class rather than being skipped. This covers special cluster architectures that only have a single storage class available.
Applied to files:
tests/storage/conftest.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:
tests/storage/conftest.py
🪛 Ruff (0.14.10)
tests/storage/conftest.py
593-593: Prefer next(iter(storage_class_matrix__function__)) over single element slice
Replace with next(iter(storage_class_matrix__function__))
(RUF015)
⏰ 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). (9)
- 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: build-container
- GitHub Check: can-be-merged
- GitHub Check: tox
- GitHub Check: can-be-merged
🔇 Additional comments (1)
tests/storage/conftest.py (1)
24-24: LGTM on the import.The
StorageProfileimport is correctly added to support the new fixture.
Ahmad-Hafe
force-pushed
the
adjust_t2_minimumSupportedPvcSize_main
branch
from
1fc0605 to
f389bfd
Compare
openshift-virtualization-qe-bot
removed
the
tox:verify-tc-requirement-polarion:passed
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/storage/cdi_clone/test_clone.py (1)
285-293: Consider using max() for robustness across different image sizes.The current logic
dv_size = min_pvc_size or dv_sizeunconditionally usesmin_pvc_sizewhen present. For the cirros image used in this test, the minimum supported PVC size is typically larger than the overhead-calculated size, so this works correctly.However, for larger images or different storage configurations, the overhead-calculated size might exceed the storage profile minimum. Using
max()would ensure the DV is sized correctly in all cases.🔎 Proposed enhancement for general robustness
dv_size = overhead_size_for_dv( image_size=int(cirros_dv_with_block_volume_mode.size[:-2]), overhead_value=default_fs_overhead, ) min_pvc_size = get_storage_profile_minimum_supported_pvc_size( storage_class_name=storage_class_with_filesystem_volume_mode, admin_client=admin_client, ) -dv_size = min_pvc_size or dv_size +if min_pvc_size: + # Use the larger of overhead-calculated size or storage profile minimum + # Note: This requires both values to use compatible units (Mi) + dv_size_mi = int(dv_size.rstrip('Mi')) + min_size_mi = int(min_pvc_size.rstrip('MiGi')) if 'Mi' in min_pvc_size else int(min_pvc_size.rstrip('Gi')) * 1024 + dv_size = f"{max(dv_size_mi, min_size_mi)}Mi"Note: The current implementation is sufficient for this test's use case (cirros image with small overhead), as confirmed by your testing on multiple storage classes.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.pytests/storage/utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/storage/utils.py
🧰 Additional context used
🧠 Learnings (29)
📓 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: 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: 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: 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.
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: 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: 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: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.
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.
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.
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/node/workload_density/test_swap.py:120-124
Timestamp: 2025-08-09T01:47:21.485Z
Learning: In the openshift-virtualization-tests repository, float values for VM memory specifications (like those passed to VirtualMachineForTests memory parameters) are automatically converted to integers by webhooks, so explicit int conversion is not required when calculating memory sizes.
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.
📚 Learning: 2025-06-17T07:45:37.776Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.
Applied to files:
tests/storage/cdi_upload/test_upload_virtctl.pytests/storage/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/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.pytests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.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:
tests/storage/cdi_upload/test_upload_virtctl.pytests/storage/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:
tests/storage/cdi_upload/test_upload_virtctl.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:
tests/storage/cdi_upload/test_upload_virtctl.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:
tests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.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:
tests/storage/cdi_upload/test_upload_virtctl.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:
tests/storage/cdi_upload/test_upload_virtctl.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/cdi_upload/test_upload_virtctl.pytests/storage/conftest.pytests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.py
📚 Learning: 2025-05-28T10:50:56.122Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/conftest.py:264-269
Timestamp: 2025-05-28T10:50:56.122Z
Learning: In the openshift-virtualization-tests codebase, cleanup pytest fixtures like `deleted_old_dvs_of_stopped_vms`, `deleted_completed_virt_launcher_source_pod`, and `deleted_old_dvs_of_online_vms` do not require yield statements. These fixtures perform cleanup operations and work correctly without yielding values.
Applied to files:
tests/storage/cdi_upload/test_upload_virtctl.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/cdi_upload/test_upload_virtctl.pytests/storage/conftest.pytests/storage/cdi_clone/test_clone.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/storage/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:
tests/storage/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/storage/conftest.pytests/storage/cdi_clone/test_clone.py
📚 Learning: 2025-08-13T06:27:29.727Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1641
File: tests/infrastructure/golden_images/test_common_templates_data_volumes.py:47-48
Timestamp: 2025-08-13T06:27:29.727Z
Learning: In tests/infrastructure/golden_images/test_common_templates_data_volumes.py, when set_storage_class is True but get_diff_storage_class_from_matrix returns None (indicating only one storage class is available), the test should continue using the default storage class rather than being skipped. This covers special cluster architectures that only have a single storage class available.
Applied to files:
tests/storage/conftest.pytests/storage/cdi_clone/test_clone.py
📚 Learning: 2025-08-27T08:34:49.132Z
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.
Applied to files:
tests/storage/conftest.pytests/storage/cdi_clone/test_clone.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:
tests/storage/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 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:
tests/storage/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:
tests/storage/conftest.py
📚 Learning: 2025-11-19T08:13:30.263Z
Learnt from: SamAlber
Repo: RedHatQE/openshift-virtualization-tests PR: 2507
File: tests/virt/node/general/test_vmi_reset.py:26-29
Timestamp: 2025-11-19T08:13:30.263Z
Learning: In the openshift-virtualization-tests repository, user SamAlber prefers not to define fixture dependencies by chaining fixtures (adding one fixture as a parameter to another). Instead, all fixture dependencies should be explicitly declared as parameters in the test method itself, relying on parameter order to control execution sequence.
Applied to files:
tests/storage/conftest.py
📚 Learning: 2025-09-10T23:16:25.845Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/test_multifd_policy_behavior.py:44-52
Timestamp: 2025-09-10T23:16:25.845Z
Learning: In pytest, fixtures are executed in the order they appear as parameters in the test method signature. For the multifd CPU limit test in tests/virt/node/migration_and_maintenance/test_multifd_policy_behavior.py, the parameter order (vm_for_multifd_test, added_vm_cpu_limit, migrated_vm_source_pod) ensures CPU limits are applied before migration occurs.
Applied to files:
tests/storage/conftest.py
📚 Learning: 2025-08-04T15:27:14.175Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1082
Timestamp: 2025-08-04T15:27:14.175Z
Learning: In tests/observability/metrics/conftest.py, the `non_existent_node_windows_vm` fixture is used for tier3 Windows VM testing and must use Windows VMs rather than Linux VMs, even though it adds overhead, because it's specifically testing Windows VM status transition metrics as part of dedicated Windows VM test coverage.
Applied to files:
tests/storage/conftest.py
📚 Learning: 2025-07-01T10:09:27.213Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1338
File: tests/observability/metrics/conftest.py:1177-1187
Timestamp: 2025-07-01T10:09:27.213Z
Learning: In tests/observability/metrics/conftest.py, when implementing pytest fixtures that validate Prometheus query results, prefer simple assertions (like checking if the result list is not empty) over complex error handling structures. The user OhadRevah prefers this simpler approach for test fixtures in this codebase.
Applied to files:
tests/storage/conftest.py
📚 Learning: 2025-08-28T12:30:40.692Z
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: tests/network/bgp/conftest.py:35-54
Timestamp: 2025-08-28T12:30:40.692Z
Learning: The BGP test suite in tests/network/bgp/ relies on session-scoped validation in tests/network/conftest.py via the network_sanity fixture's _verify_bgp() function, which validates required environment variables (VLAN_TAG, EXTERNAL_FRR_STATIC_IPV4, BGP_CLUSTER_DOMAIN_GROUP) before any BGP tests run, making individual fixture-level validation redundant.
Applied to files:
tests/storage/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:
tests/storage/cdi_clone/test_clone.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/storage/cdi_clone/test_clone.py
🧬 Code graph analysis (3)
tests/storage/cdi_upload/test_upload_virtctl.py (1)
tests/storage/conftest.py (2)
storage_profile_minimum_supported_pvc_size(567-570)storage_class_name_scope_module(562-563)
tests/storage/conftest.py (2)
tests/storage/utils.py (1)
get_storage_profile_minimum_supported_pvc_size(411-414)tests/conftest.py (1)
admin_client(297-301)
tests/storage/cdi_clone/test_clone.py (4)
tests/storage/utils.py (1)
get_storage_profile_minimum_supported_pvc_size(411-414)tests/conftest.py (1)
admin_client(297-301)tests/storage/conftest.py (1)
default_fs_overhead(312-313)utilities/storage.py (1)
overhead_size_for_dv(588-595)
⏰ 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). (14)
- 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: 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 (2)
tests/storage/conftest.py (1)
566-570: LGTM! Fixture implementation is correct.The module-scoped fixture correctly delegates to the helper function and has appropriate dependencies. The scope matches the storage class fixture scope, resolving the previously identified scope mismatch.
tests/storage/cdi_upload/test_upload_virtctl.py (1)
278-286: LGTM! Empty PVC sizing logic is correct.The fixture correctly uses the storage profile's minimum supported PVC size when available, falling back to the default. This pattern is appropriate for creating empty PVCs and prevents failures on storage providers with minimum size requirements.
Contributor
Author
|
/verified test_virtctl_image_upload_with_exist_pvc on muiltiple storage class pass rate 100% |
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: |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 7721 🏁 Script executed: Length of output: 7244 🏁 Script executed: Length of output: 1388 🏁 Script executed: Length of output: 1728 🧠 Learnings used |
openshift-virtualization-qe-bot
approved these changes
Jan 4, 2026
openshift-virtualization-qe-bot
approved these changes
Jan 4, 2026
openshift-virtualization-qe-bot
approved these changes
Jan 4, 2026
Contributor
Author
|
verified test_virtctl_image_upload_with_exist_pvc on muiltiple storage class pass rate 100% |
Contributor
Author
|
/verified test_virtctl_image_upload_with_exist_pvc on muiltiple storage class pass rate 100% |
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: |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 9030 🏁 Script executed: Length of output: 1728 🧠 Learnings used |
openshift-virtualization-qe-bot
previously approved these changes
Jan 6, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @tests/storage/utils.py:
- Around line 411-414: The function
get_storage_profile_minimum_supported_pvc_size is defined twice; remove the
duplicate definition so only one implementation remains. Search the file for all
occurrences of def get_storage_profile_minimum_supported_pvc_size, delete one of
the redundant blocks (keep the correct/most complete copy), ensure there are no
leftover imports or references relying on the removed block, and run tests/lint
to confirm no regressions.
- Around line 411-414: The function
get_storage_profile_minimum_supported_pvc_size should catch errors when
instantiating StorageProfile to avoid unhandled exceptions; wrap the
StorageProfile(name=storage_class_name, client=admin_client) call in a
try/except that catches API/lookup errors (e.g., ApiException or a generic
Exception), log or attach a clear message identifying the storage_class_name and
admin_client context, and return None (or rethrow a more descriptive exception)
instead of letting the raw exception bubble up; ensure subsequent access to
storage_profile.instance.metadata.annotations only runs when instantiation
succeeded.
🧹 Nitpick comments (1)
tests/storage/utils.py (1)
411-414: Consider adding a docstring.The function lacks a docstring explaining its purpose, parameters, and return value. Adding documentation would improve maintainability.
🔎 Proposed docstring
def get_storage_profile_minimum_supported_pvc_size(storage_class_name, admin_client): + """ + Retrieve the minimum supported PVC size from a StorageProfile annotation. + + Args: + storage_class_name: Name of the storage class + admin_client: Admin client for API access + + Returns: + str or None: Minimum supported PVC size (e.g., "1Gi") if the annotation exists, None otherwise + """ storage_profile = StorageProfile(name=storage_class_name, client=admin_client) annotations = getattr(storage_profile.instance.metadata, "annotations", {}) or {} return annotations.get("cdi.kubevirt.io/minimumSupportedPvcSize")
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.pytests/storage/utils.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/storage/conftest.py
- tests/storage/cdi_upload/test_upload_virtctl.py
- tests/storage/cdi_clone/test_clone.py
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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: 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: 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: 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.
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: 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: 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: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3228
File: .coderabbit.yaml:30-41
Timestamp: 2026-01-05T10:33:50.728Z
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: 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.
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: 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.
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: 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.
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.
📚 Learning: 2025-08-27T08:34:49.132Z
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.
Applied to files:
tests/storage/utils.py
📚 Learning: 2025-08-13T06:27:29.727Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1641
File: tests/infrastructure/golden_images/test_common_templates_data_volumes.py:47-48
Timestamp: 2025-08-13T06:27:29.727Z
Learning: In tests/infrastructure/golden_images/test_common_templates_data_volumes.py, when set_storage_class is True but get_diff_storage_class_from_matrix returns None (indicating only one storage class is available), the test should continue using the default storage class rather than being skipped. This covers special cluster architectures that only have a single storage class available.
Applied to files:
tests/storage/utils.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/storage/utils.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:
tests/storage/utils.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/utils.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/utils.py
rnetser
requested changes
Jan 6, 2026
| from utilities.storage import data_volume | ||
|
|
||
|
|
||
| def get_storage_profile_minimum_supported_pvc_size(storage_class_name, admin_client): |
Collaborator
There was a problem hiding this comment.
do not include functions in conftest files
| def get_storage_profile_minimum_supported_pvc_size(storage_class_name, admin_client): | ||
| storage_profile = StorageProfile(name=storage_class_name, client=admin_client) | ||
| annotations = getattr(storage_profile.instance.metadata, "annotations", {}) or {} | ||
| return annotations.get("cdi.kubevirt.io/minimumSupportedPvcSize") |
Collaborator
There was a problem hiding this comment.
please use the api_group attribute
| default_fs_overhead, | ||
| ): | ||
| dv_size = overhead_size_for_dv( | ||
| image_size=int(cirros_dv_with_block_volume_mode.size[:-2]), |
Collaborator
There was a problem hiding this comment.
please use bitmath.parse_string to get the size
|
|
||
| @pytest.fixture(scope="module") | ||
| def storage_profile_minimum_supported_pvc_size(storage_class_name_scope_module, admin_client): | ||
| return get_storage_profile_minimum_supported_pvc_size( |
Collaborator
There was a problem hiding this comment.
this fixture only returns a value from another function; use directly where needed
|
|
||
|
|
||
| def get_storage_profile_minimum_supported_pvc_size(storage_class_name, admin_client): | ||
| storage_profile = StorageProfile(name=storage_class_name, client=admin_client) |
Collaborator
There was a problem hiding this comment.
add ensure_exists to make sure the storageProfile is there
jpeimer
reviewed
Jan 6, 2026
| storage_class_with_filesystem_volume_mode, | ||
| default_fs_overhead, | ||
| ): | ||
| dv_size = overhead_size_for_dv( |
Contributor
There was a problem hiding this comment.
I know we used this overhead_size_for_dv function before, but I'm a bit confused. Block -> FS clone should work even if we ask for the same size as Block pvc had (with storage api, cdi will increase the size automatically to accommodate the fs overhead)
Contributor
Author
|
/wip |
openshift-virtualization-qe-bot
previously approved these changes
Jan 7, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @tests/storage/utils.py:
- Around line 411-414: In get_storage_profile_minimum_supported_pvc_size,
instantiate StorageProfile with ensure_exists=True (i.e.,
StorageProfile(name=storage_class_name, client=admin_client,
ensure_exists=True)) so the code verifies the profile exists before accessing
instance.metadata.annotations; keep the rest of the function unchanged.
🧹 Nitpick comments (1)
tests/storage/cdi_clone/test_clone.py (1)
283-283: Usebitmath.parse_string()instead ofparse_string_unsafe().The code uses
parse_string_unsafe(), which skips validation of unit suffixes. Usebitmath.parse_string()for safer parsing with validation.♻️ Proposed fix
- pvc_size = bitmath.parse_string_unsafe(s=source_pvc.instance.spec.resources.requests.storage) + pvc_size = bitmath.parse_string(s=source_pvc.instance.spec.resources.requests.storage)Based on past review comments, rnetser recommended using
bitmath.parse_string.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.pytests/storage/utils.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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:
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.pytests/storage/utils.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:
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.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/storage/conftest.py
🧠 Learnings (45)
📓 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: 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: 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.
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: 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: 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: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3228
File: .coderabbit.yaml:30-41
Timestamp: 2026-01-05T10:33:50.728Z
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: 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.
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: 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.
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: 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.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.
📚 Learning: 2025-08-13T06:27:29.727Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1641
File: tests/infrastructure/golden_images/test_common_templates_data_volumes.py:47-48
Timestamp: 2025-08-13T06:27:29.727Z
Learning: In tests/infrastructure/golden_images/test_common_templates_data_volumes.py, when set_storage_class is True but get_diff_storage_class_from_matrix returns None (indicating only one storage class is available), the test should continue using the default storage class rather than being skipped. This covers special cluster architectures that only have a single storage class available.
Applied to files:
tests/storage/cdi_clone/test_clone.pytests/storage/conftest.pytests/storage/utils.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/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/utils.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/storage/cdi_clone/test_clone.pytests/storage/conftest.py
📚 Learning: 2025-08-27T08:34:49.132Z
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.
Applied to files:
tests/storage/cdi_clone/test_clone.pytests/storage/conftest.pytests/storage/utils.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:
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.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:
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.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:
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.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/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/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/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.pytests/storage/utils.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/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.pytests/storage/utils.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:
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.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: For PR #1904 test execution, the critical validation point is test_connectivity_over_migration_between_localnet_vms which should fail gracefully on cloud clusters but pass on bare-metal/PSI clusters, representing the core nmstate conditional logic functionality.
Applied to files:
tests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.py
📚 Learning: 2025-06-23T19:19:31.961Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:83-97
Timestamp: 2025-06-23T19:19:31.961Z
Learning: In OpenShift Virtualization mass machine type transition tests, the kubevirt_api_lifecycle_automation_job requires cluster-admin privileges to function properly, as confirmed by the test maintainer akri3i.
Applied to files:
tests/storage/cdi_clone/test_clone.py
📚 Learning: 2025-11-20T16:27:01.693Z
Learnt from: chandramerla
Repo: RedHatQE/openshift-virtualization-tests PR: 2577
File: tests/virt/node/workload_density/test_free_page_reporting.py:90-90
Timestamp: 2025-11-20T16:27:01.693Z
Learning: The virtio_balloon free page reporting issue on s390x architecture (tracked in OCPBUGS-51113) has been fixed. Free page reporting tests in tests/virt/node/workload_density/test_free_page_reporting.py are now safe to run on s390x clusters.
Applied to files:
tests/storage/cdi_clone/test_clone.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/storage/cdi_clone/test_clone.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/storage/cdi_clone/test_clone.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/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.pytests/storage/utils.py
📚 Learning: 2025-06-17T07:45:37.776Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 1160
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:165-176
Timestamp: 2025-06-17T07:45:37.776Z
Learning: In the openshift-virtualization-tests repository, user jpeimer prefers explicit fixture parameters over composite fixtures in test methods, even when there are many parameters, as they find this approach more readable and maintainable for understanding test dependencies.
Applied to files:
tests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.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:
tests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.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:
tests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.py
📚 Learning: 2025-06-18T09:19:05.769Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/test_vms_metrics.py:129-137
Timestamp: 2025-06-18T09:19:05.769Z
Learning: For Windows VM testing in tests/observability/metrics/test_vms_metrics.py, it's acceptable to have more fixture parameters than typical pylint recommendations when reusing expensive Windows VM fixtures for performance. Windows VMs take a long time to deploy, so reusing fixtures like windows_vm_for_test and adding labels via windows_vm_with_low_bandwidth_migration_policy is preferred over creating separate fixtures that would require additional VM deployments.
Applied to files:
tests/storage/cdi_upload/test_upload_virtctl.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:
tests/storage/cdi_upload/test_upload_virtctl.pytests/storage/conftest.py
📚 Learning: 2025-05-28T10:50:56.122Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/conftest.py:264-269
Timestamp: 2025-05-28T10:50:56.122Z
Learning: In the openshift-virtualization-tests codebase, cleanup pytest fixtures like `deleted_old_dvs_of_stopped_vms`, `deleted_completed_virt_launcher_source_pod`, and `deleted_old_dvs_of_online_vms` do not require yield statements. These fixtures perform cleanup operations and work correctly without yielding values.
Applied to files:
tests/storage/cdi_upload/test_upload_virtctl.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/storage/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:
tests/storage/conftest.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:
tests/storage/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:
tests/storage/conftest.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 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/storage/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 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:
tests/storage/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:
tests/storage/conftest.py
📚 Learning: 2026-01-05T10:33:50.728Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3228
File: .coderabbit.yaml:30-41
Timestamp: 2026-01-05T10:33:50.728Z
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:
tests/storage/conftest.py
📚 Learning: 2025-11-19T08:13:30.263Z
Learnt from: SamAlber
Repo: RedHatQE/openshift-virtualization-tests PR: 2507
File: tests/virt/node/general/test_vmi_reset.py:26-29
Timestamp: 2025-11-19T08:13:30.263Z
Learning: In the openshift-virtualization-tests repository, user SamAlber prefers not to define fixture dependencies by chaining fixtures (adding one fixture as a parameter to another). Instead, all fixture dependencies should be explicitly declared as parameters in the test method itself, relying on parameter order to control execution sequence.
Applied to files:
tests/storage/conftest.py
📚 Learning: 2026-01-06T20:32:10.834Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.834Z
Learning: Local fixtures location — place in `<feature_dir>/conftest.py`
Applied to files:
tests/storage/conftest.py
📚 Learning: 2026-01-06T20:32:10.833Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.833Z
Learning: Applies to **/conftest.py : Fixtures MUST do ONE action only — single responsibility principle required
Applied to files:
tests/storage/conftest.py
📚 Learning: 2026-01-06T20:32:10.834Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.834Z
Learning: Move fixtures and helpers to shared location — move to `utilities/` or `tests/conftest.py` ONLY when used by different team directories
Applied to files:
tests/storage/conftest.py
📚 Learning: 2026-01-06T20:32:10.833Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.833Z
Learning: Applies to **/conftest.py : Fixture ordering REQUIRED — pytest native fixtures first, then session-scoped, then module/class/function scoped
Applied to files:
tests/storage/conftest.py
📚 Learning: 2025-09-10T23:16:25.845Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/test_multifd_policy_behavior.py:44-52
Timestamp: 2025-09-10T23:16:25.845Z
Learning: In pytest, fixtures are executed in the order they appear as parameters in the test method signature. For the multifd CPU limit test in tests/virt/node/migration_and_maintenance/test_multifd_policy_behavior.py, the parameter order (vm_for_multifd_test, added_vm_cpu_limit, migrated_vm_source_pod) ensures CPU limits are applied before migration occurs.
Applied to files:
tests/storage/conftest.py
📚 Learning: 2026-01-06T20:32:10.833Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.833Z
Learning: Applies to **/conftest.py : Fixture scope rules — use `scope="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
Applied to files:
tests/storage/conftest.py
📚 Learning: 2025-08-04T15:27:14.175Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1082
Timestamp: 2025-08-04T15:27:14.175Z
Learning: In tests/observability/metrics/conftest.py, the `non_existent_node_windows_vm` fixture is used for tier3 Windows VM testing and must use Windows VMs rather than Linux VMs, even though it adds overhead, because it's specifically testing Windows VM status transition metrics as part of dedicated Windows VM test coverage.
Applied to files:
tests/storage/conftest.py
📚 Learning: 2026-01-06T20:32:10.833Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.833Z
Learning: Applies to **/test_*.py : ALWAYS use `pytest.mark.usefixtures` when fixture return value is not used by test
Applied to files:
tests/storage/conftest.py
📚 Learning: 2026-01-06T20:32:10.833Z
Learnt from: CR
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T20:32:10.833Z
Learning: Applies to **/conftest.py : Fixture naming REQUIRED — ALWAYS use NOUNS (what they provide), NEVER verbs. Use descriptive names like `vm_with_disk` instead of `create_vm_with_disk`
Applied to files:
tests/storage/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:
tests/storage/utils.py
🧬 Code graph analysis (3)
tests/storage/cdi_clone/test_clone.py (2)
tests/storage/cdi_clone/conftest.py (1)
cirros_dv_with_block_volume_mode(26-37)tests/conftest.py (2)
namespace(661-674)unprivileged_client(387-419)
tests/storage/cdi_upload/test_upload_virtctl.py (1)
tests/storage/conftest.py (2)
storage_profile_minimum_supported_pvc_size(567-570)storage_class_name_scope_module(562-563)
tests/storage/conftest.py (2)
tests/storage/utils.py (1)
get_storage_profile_minimum_supported_pvc_size(411-414)tests/conftest.py (1)
admin_client(297-301)
🔇 Additional comments (2)
tests/storage/cdi_clone/test_clone.py (1)
291-291: LGTM! Direct PVC size usage simplifies clone logic.Using the source PVC size directly is appropriate here. As mentioned in past review discussions, CDI automatically handles filesystem overhead during Block→FS cloning, eliminating the need for manual overhead calculations.
tests/storage/cdi_upload/test_upload_virtctl.py (1)
278-286: LGTM! PVC sizing now respects storage provider minimums.The fixture correctly uses the storage profile's minimum supported PVC size when available, with a sensible fallback to
DEFAULT_DV_SIZE. This prevents test failures on providers like GCP that enforce minimum PVC sizes.
Comment on lines
+411
to
+414
| def get_storage_profile_minimum_supported_pvc_size(storage_class_name, admin_client): | ||
| storage_profile = StorageProfile(name=storage_class_name, client=admin_client) | ||
| annotations = getattr(storage_profile.instance.metadata, "annotations", {}) or {} | ||
| return annotations.get("cdi.kubevirt.io/minimumSupportedPvcSize") |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add ensure_exists=True to StorageProfile instantiation.
The function doesn't verify that the StorageProfile exists, which could lead to unclear errors if the storage class doesn't have a corresponding profile.
♻️ Proposed fix
def get_storage_profile_minimum_supported_pvc_size(storage_class_name, admin_client):
- storage_profile = StorageProfile(name=storage_class_name, client=admin_client)
+ storage_profile = StorageProfile(name=storage_class_name, client=admin_client, ensure_exists=True)
annotations = getattr(storage_profile.instance.metadata, "annotations", {}) or {}
return annotations.get("cdi.kubevirt.io/minimumSupportedPvcSize")Based on past review comments, rnetser requested adding ensure_exists.
🤖 Prompt for AI Agents
In @tests/storage/utils.py around lines 411 - 414, In
get_storage_profile_minimum_supported_pvc_size, instantiate StorageProfile with
ensure_exists=True (i.e., StorageProfile(name=storage_class_name,
client=admin_client, ensure_exists=True)) so the code verifies the profile
exists before accessing instance.metadata.annotations; keep the rest of the
function unchanged.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3243 +/- ##
=======================================
Coverage 98.38% 98.38%
=======================================
Files 25 25
Lines 2166 2167 +1
=======================================
+ Hits 2131 2132 +1
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
previously approved these changes
Jan 7, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @utilities/storage.py:
- Around line 598-601: Add mandatory type hints to the public function
get_storage_profile_minimum_supported_pvc_size by annotating storage_class_name
as str, admin_client with the appropriate client type (or typing.Any if
unknown), and the return type as Optional[str]; also add a Google-style
docstring to describe parameters, return value (may return None or string), and
side effects (it instantiates StorageProfile with ensure_exists=True which can
call the API). Reference the function name
get_storage_profile_minimum_supported_pvc_size and the use of
StorageProfile(name=storage_class_name, client=admin_client, ensure_exists=True)
in the docstring so reviewers understand the API interaction and nullable
return.
- Around line 598-601: The get_storage_profile_minimum_supported_pvc_size
function currently constructs StorageProfile(name=storage_class_name,
client=admin_client, ensure_exists=True) which raises when the profile is
missing; change this to either use ensure_exists=False on StorageProfile or wrap
the creation/attribute access in a try/except that catches NotFoundError and
returns None so callers can use their fallback; ensure you still read
annotations via getattr(storage_profile.instance.metadata, "annotations", {}) or
{} and return annotations.get("cdi.kubevirt.io/minimumSupportedPvcSize") when
present.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/utils.pyutilities/storage.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/storage/cdi_upload/test_upload_virtctl.py
- tests/storage/cdi_clone/test_clone.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/storage.pytests/utils.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/storage.py
🧠 Learnings (5)
📓 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: 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.
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: 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: 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: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3228
File: .coderabbit.yaml:30-41
Timestamp: 2026-01-05T10:33:50.728Z
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: 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.
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.
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: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/node/workload_density/test_swap.py:120-124
Timestamp: 2025-08-09T01:47:21.485Z
Learning: In the openshift-virtualization-tests repository, float values for VM memory specifications (like those passed to VirtualMachineForTests memory parameters) are automatically converted to integers by webhooks, so explicit int conversion is not required when calculating memory sizes.
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:142-149
Timestamp: 2025-06-23T19:18:12.275Z
Learning: In OpenShift Virtualization machine type transition tests, the kubevirt_api_lifecycle_automation_job updates VM machine types to the latest version based on a MACHINE_TYPE_GLOB pattern, and subsequent fixtures may intentionally revert the machine type to test bidirectional transition behavior.
📚 Learning: 2025-06-05T11:36:25.219Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 906
File: tests/observability/metrics/conftest.py:19-19
Timestamp: 2025-06-05T11:36:25.219Z
Learning: In the openshift-virtualization-tests repository, the pyhelper_utils package (specifically pyhelper_utils.shell with run_ssh_commands) is installed and available in the test execution environment where the tests are run.
Applied to files:
tests/utils.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/utils.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/utils.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/utils.py
🧬 Code graph analysis (2)
utilities/storage.py (1)
tests/conftest.py (1)
admin_client(297-301)
tests/utils.py (3)
utilities/storage.py (1)
get_storage_profile_minimum_supported_pvc_size(598-601)tests/conftest.py (1)
admin_client(297-301)libs/infra/images.py (1)
Cirros(9-20)
⏰ 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). (5)
- GitHub Check: build-container
- GitHub Check: tox
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
🔇 Additional comments (1)
tests/utils.py (1)
49-49: LGTM: Import follows guidelines.The absolute import of the specific function follows the coding guidelines correctly.
Comment on lines
+598
to
+601
| def get_storage_profile_minimum_supported_pvc_size(storage_class_name, admin_client): | ||
| storage_profile = StorageProfile(name=storage_class_name, client=admin_client, ensure_exists=True) | ||
| annotations = getattr(storage_profile.instance.metadata, "annotations", {}) or {} | ||
| return annotations.get("cdi.kubevirt.io/minimumSupportedPvcSize") |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add mandatory type hints and required docstring.
This new public function under utilities/ violates coding guidelines:
-
Missing type hints: Per guidelines, type hints are MANDATORY for all new public functions under
utilities/. Add return type (Optional[str]) and parameter types. -
Missing docstring: Per guidelines, Google-format docstrings are REQUIRED for functions with non-obvious return values or side effects. This function has both (returns
Noneor string, makes API calls viaensure_exists=True).
✨ Proposed enhancement with type hints and docstring
+from typing import Optional
+
-def get_storage_profile_minimum_supported_pvc_size(storage_class_name, admin_client):
+def get_storage_profile_minimum_supported_pvc_size(
+ storage_class_name: str, admin_client: DynamicClient
+) -> Optional[str]:
+ """
+ Get minimum supported PVC size from StorageProfile annotation.
+
+ Args:
+ storage_class_name: Name of the storage class
+ admin_client: DynamicClient with privileges to read StorageProfile
+
+ Returns:
+ Minimum PVC size string (e.g., "1Gi") if annotation exists, None otherwise
+
+ Raises:
+ NotFoundError: If StorageProfile doesn't exist
+ """
storage_profile = StorageProfile(name=storage_class_name, client=admin_client, ensure_exists=True)
annotations = getattr(storage_profile.instance.metadata, "annotations", {}) or {}
return annotations.get("cdi.kubevirt.io/minimumSupportedPvcSize")Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @utilities/storage.py around lines 598 - 601, Add mandatory type hints to the
public function get_storage_profile_minimum_supported_pvc_size by annotating
storage_class_name as str, admin_client with the appropriate client type (or
typing.Any if unknown), and the return type as Optional[str]; also add a
Google-style docstring to describe parameters, return value (may return None or
string), and side effects (it instantiates StorageProfile with
ensure_exists=True which can call the API). Reference the function name
get_storage_profile_minimum_supported_pvc_size and the use of
StorageProfile(name=storage_class_name, client=admin_client, ensure_exists=True)
in the docstring so reviewers understand the API interaction and nullable
return.
🧩 Analysis chain
🏁 Script executed:
# Search for usages of the function get_storage_profile_minimum_supported_pvc_size
rg -n 'get_storage_profile_minimum_supported_pvc_size' --type pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 674
🏁 Script executed:
# Find the StorageProfile class definition
rg -n 'class StorageProfile' --type pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
# Search for the fallback pattern mentioned: Images.Cirros.DEFAULT_DV_SIZE
rg -n 'Images\.Cirros\.DEFAULT_DV_SIZE\|DEFAULT_DV_SIZE' --type py | head -20Repository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
# Check for NotFoundError usage in the codebase
rg -n 'NotFoundError' --type py | head -20Repository: RedHatQE/openshift-virtualization-tests
Length of output: 1629
🏁 Script executed:
# Look at the broader context of the storage.py file to understand error handling patterns
head -50 utilities/storage.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 1748
🏁 Script executed:
# Get context around the usage in tests/utils.py:525
sed -n '515,540p' tests/utils.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 1182
🏁 Script executed:
# Get context around the usage in test_upload_virtctl.py:284
sed -n '275,295p' tests/storage/cdi_upload/test_upload_virtctl.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 982
🏁 Script executed:
# Look at the complete get_storage_profile_minimum_supported_pvc_size function and surrounding code
sed -n '595,610p' utilities/storage.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 655
🏁 Script executed:
# Search for Images.Cirros.DEFAULT_DV_SIZE fallback usage
rg -B 3 -A 3 'Images\.Cirros\.DEFAULT_DV_SIZE' --type pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 13360
🏁 Script executed:
# Check how ensure_exists is used elsewhere in storage.py to understand its behavior
rg -B 2 -A 2 'ensure_exists=True' utilities/storage.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 428
🏁 Script executed:
# Check if StorageProfile is created automatically or if ensure_exists=True is needed
rg -B 3 -A 3 'StorageProfile.*ensure_exists' --type pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 577
🏁 Script executed:
# Look for exception handling around the function calls
sed -n '520,535p' tests/utils.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 717
🏁 Script executed:
# Check test_common_templates_data_volumes.py mentioned in learning (special cluster handling)
head -60 tests/infrastructure/golden_images/test_common_templates_data_volumes.py | tail -20Repository: RedHatQE/openshift-virtualization-tests
Length of output: 752
🏁 Script executed:
# Check how StorageProfile is used elsewhere - does ensure_exists=False exist?
rg 'StorageProfile' --type py -A 2 | head -30Repository: RedHatQE/openshift-virtualization-tests
Length of output: 2172
🏁 Script executed:
# Look for any exception handling specifically for NotFoundError in callers
rg -B 5 'get_storage_profile_minimum_supported_pvc_size' tests/utils.py | head -20Repository: RedHatQE/openshift-virtualization-tests
Length of output: 608
🏁 Script executed:
# Check what other functions in storage.py do when a resource might not exist
sed -n '240,260p' utilities/storage.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 978
🏁 Script executed:
# Search for patterns where ensure_exists is used with False to see the contrast
rg 'ensure_exists=False' utilities/ --type py | head -10Repository: RedHatQE/openshift-virtualization-tests
Length of output: 65
Catch NotFoundError to allow fallback pattern when StorageProfile doesn't exist.
The function uses ensure_exists=True, which raises an exception if the StorageProfile is missing. Both callers expect None to trigger their fallback pattern (or Images.Cirros.DEFAULT_DV_SIZE / or DEFAULT_DV_SIZE), and other StorageProfile usage in the codebase omits ensure_exists (defaulting to False). Change to ensure_exists=False or wrap with a try/except to catch NotFoundError and return None, allowing the fallback to work for clusters with limited StorageProfile availability.
🤖 Prompt for AI Agents
In @utilities/storage.py around lines 598 - 601, The
get_storage_profile_minimum_supported_pvc_size function currently constructs
StorageProfile(name=storage_class_name, client=admin_client, ensure_exists=True)
which raises when the profile is missing; change this to either use
ensure_exists=False on StorageProfile or wrap the creation/attribute access in a
try/except that catches NotFoundError and returns None so callers can use their
fallback; ensure you still read annotations via
getattr(storage_profile.instance.metadata, "annotations", {}) or {} and return
annotations.get("cdi.kubevirt.io/minimumSupportedPvcSize") when present.
|
D/S test |
openshift-virtualization-qe-bot
requested changes
Jan 7, 2026
openshift-virtualization-qe-bot
left a comment
openshift-virtualization-qe-bot
left a comment
There was a problem hiding this comment.
D/S test tox -e verify-bugs-are-open failed: cnv-tests-tox-executor/22930
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @tests/utils.py:
- Around line 525-526: The call to
get_storage_profile_minimum_supported_pvc_size is using an unprivileged client
but StorageProfile is cluster-scoped and requires admin rights; update the
enclosing function in tests/utils.py (the function that constructs size using
get_storage_profile_minimum_supported_pvc_size and
Images.Cirros.DEFAULT_DV_SIZE) to accept a separate admin_client parameter and
pass admin_client=admin_client into
get_storage_profile_minimum_supported_pvc_size, then update all callers
(tests/storage/conftest.py, tests/storage/hpp/conftest.py,
tests/storage/fs_overhead/test_fs_overhead.py) to provide an admin_client (not
unprivileged_client); alternatively, if you choose not to change callers, ensure
the function signature explicitly requires an admin_client and validate it
before calling get_storage_profile_minimum_supported_pvc_size.
🧹 Nitpick comments (3)
tests/storage/cdi_clone/test_clone.py (1)
282-283: Usebitmath.parse_stringinstead ofparse_string_unsafe.The code uses
bitmath.parse_string_unsafe, butbitmath.parse_stringis preferred as it validates input and raises exceptions for invalid formats. This provides better error messages if the PVC storage format is unexpected.🔧 Proposed fix
source_pvc = cirros_dv_with_block_volume_mode.pvc - pvc_size = bitmath.parse_string_unsafe(s=source_pvc.instance.spec.resources.requests.storage) + pvc_size = bitmath.parse_string(string=source_pvc.instance.spec.resources.requests.storage)Based on learnings, as per a previous review comment on this PR.
tests/storage/fs_overhead/test_fs_overhead.py (2)
22-22: Usebitmath.parse_stringinstead ofparse_string_unsafe.The function uses
bitmath.parse_string_unsafe, butbitmath.parse_stringis preferred as it validates input and raises exceptions for invalid formats. This provides better error handling when parsing PVC storage specifications.🔧 Proposed fix
def get_pvc_size_gib(pvc): - return bitmath.parse_string_unsafe(s=pvc.instance.spec.resources.requests.storage).to_GiB() + return bitmath.parse_string(string=pvc.instance.spec.resources.requests.storage).to_GiB()Based on learnings, as per a previous review comment on this PR.
123-123: Usebitmath.parse_stringinstead ofparse_string_unsafe.The code uses
bitmath.parse_string_unsafe, butbitmath.parse_stringis preferred as it validates input and raises exceptions for invalid formats.🔧 Proposed fix
assert_fs_overhead_added( actual_size=get_pvc_size_gib(pvc=uploaded_cirros_dv.pvc), - requested_size=bitmath.GiB(int(bitmath.parse_string_unsafe(s=requested_size_str).value)), + requested_size=bitmath.GiB(int(bitmath.parse_string(string=requested_size_str).value)), )Based on learnings, as per a previous review comment on this PR.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/fs_overhead/test_fs_overhead.pytests/utils.pyutilities/storage.py
🚧 Files skipped from review as they are similar to previous changes (2)
- utilities/storage.py
- tests/storage/cdi_upload/test_upload_virtctl.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:
tests/utils.pytests/storage/cdi_clone/test_clone.pytests/storage/fs_overhead/test_fs_overhead.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:
tests/storage/cdi_clone/test_clone.pytests/storage/fs_overhead/test_fs_overhead.py
🧠 Learnings (28)
📓 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: 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: 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: 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: 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.
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: 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: 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: 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.
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: 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.
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.
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/test_vms_metrics.py:129-137
Timestamp: 2025-06-18T09:19:05.769Z
Learning: For Windows VM testing in tests/observability/metrics/test_vms_metrics.py, it's acceptable to have more fixture parameters than typical pylint recommendations when reusing expensive Windows VM fixtures for performance. Windows VMs take a long time to deploy, so reusing fixtures like windows_vm_for_test and adding labels via windows_vm_with_low_bandwidth_migration_policy is preferred over creating separate fixtures that would require additional VM deployments.
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.
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.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1028
File: utilities/infra.py:1249-1254
Timestamp: 2025-08-06T13:57:51.928Z
Learning: User rnetser verified that all calls to get_infrastructure() function use the admin_client parameter, confirming that signature changes requiring this parameter don't cause breaking changes in the openshift-virtualization-tests codebase.
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.
📚 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/utils.pytests/storage/cdi_clone/test_clone.pytests/storage/fs_overhead/test_fs_overhead.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/utils.pytests/storage/cdi_clone/test_clone.pytests/storage/fs_overhead/test_fs_overhead.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:
tests/utils.py
📚 Learning: 2026-01-06T09:35:25.073Z
Learnt from: SiboWang1997
Repo: RedHatQE/openshift-virtualization-tests PR: 3328
File: utilities/virt.py:1509-1512
Timestamp: 2026-01-06T09:35:25.073Z
Learning: In the openshift-virtualization-tests repository, when ServiceForVirtualMachineForTests.service_ip() needs to access Node information (for NODE_PORT service type), it must use cache_admin_client() rather than self.vm.client because Node is a cluster-scoped resource requiring admin permissions, even if the VM was created with an unprivileged client.
Applied to files:
tests/utils.py
📚 Learning: 2025-06-23T19:19:31.961Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:83-97
Timestamp: 2025-06-23T19:19:31.961Z
Learning: In OpenShift Virtualization mass machine type transition tests, the kubevirt_api_lifecycle_automation_job requires cluster-admin privileges to function properly, as confirmed by the test maintainer akri3i.
Applied to files:
tests/utils.pytests/storage/cdi_clone/test_clone.py
📚 Learning: 2025-08-06T13:57:51.928Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1028
File: utilities/infra.py:1249-1254
Timestamp: 2025-08-06T13:57:51.928Z
Learning: User rnetser verified that all calls to get_infrastructure() function use the admin_client parameter, confirming that signature changes requiring this parameter don't cause breaking changes in the openshift-virtualization-tests codebase.
Applied to files:
tests/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:
tests/utils.pytests/storage/cdi_clone/test_clone.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:
tests/utils.pytests/storage/cdi_clone/test_clone.py
📚 Learning: 2025-06-23T19:18:51.402Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:72-80
Timestamp: 2025-06-23T19:18:51.402Z
Learning: In mass machine type transition tests for OpenShift Virtualization, the kubevirt_api_lifecycle_automation service account requires the "privileged" SCC as it's a requirement for the lifecycle automation jobs that perform VM machine type transitions at the cluster level.
Applied to files:
tests/utils.py
📚 Learning: 2025-08-27T08:34:49.132Z
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.
Applied to files:
tests/utils.pytests/storage/cdi_clone/test_clone.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:
tests/utils.py
📚 Learning: 2025-06-05T11:36:25.219Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 906
File: tests/observability/metrics/conftest.py:19-19
Timestamp: 2025-06-05T11:36:25.219Z
Learning: In the openshift-virtualization-tests repository, the pyhelper_utils package (specifically pyhelper_utils.shell with run_ssh_commands) is installed and available in the test execution environment where the tests are run.
Applied to files:
tests/utils.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/utils.pytests/storage/cdi_clone/test_clone.pytests/storage/fs_overhead/test_fs_overhead.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/utils.pytests/storage/cdi_clone/test_clone.pytests/storage/fs_overhead/test_fs_overhead.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/utils.pytests/storage/cdi_clone/test_clone.pytests/storage/fs_overhead/test_fs_overhead.py
📚 Learning: 2025-08-13T06:27:29.727Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1641
File: tests/infrastructure/golden_images/test_common_templates_data_volumes.py:47-48
Timestamp: 2025-08-13T06:27:29.727Z
Learning: In tests/infrastructure/golden_images/test_common_templates_data_volumes.py, when set_storage_class is True but get_diff_storage_class_from_matrix returns None (indicating only one storage class is available), the test should continue using the default storage class rather than being skipped. This covers special cluster architectures that only have a single storage class available.
Applied to files:
tests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.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/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.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: For PR #1904 test execution, the critical validation point is test_connectivity_over_migration_between_localnet_vms which should fail gracefully on cloud clusters but pass on bare-metal/PSI clusters, representing the core nmstate conditional logic functionality.
Applied to files:
tests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.py
📚 Learning: 2025-11-20T16:27:01.693Z
Learnt from: chandramerla
Repo: RedHatQE/openshift-virtualization-tests PR: 2577
File: tests/virt/node/workload_density/test_free_page_reporting.py:90-90
Timestamp: 2025-11-20T16:27:01.693Z
Learning: The virtio_balloon free page reporting issue on s390x architecture (tracked in OCPBUGS-51113) has been fixed. Free page reporting tests in tests/virt/node/workload_density/test_free_page_reporting.py are now safe to run on s390x clusters.
Applied to files:
tests/storage/cdi_clone/test_clone.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/storage/cdi_clone/test_clone.pytests/storage/fs_overhead/test_fs_overhead.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/storage/cdi_clone/test_clone.pytests/storage/fs_overhead/test_fs_overhead.py
📚 Learning: 2025-08-09T01:47:21.485Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/node/workload_density/test_swap.py:120-124
Timestamp: 2025-08-09T01:47:21.485Z
Learning: In the openshift-virtualization-tests repository, float values for VM memory specifications (like those passed to VirtualMachineForTests memory parameters) are automatically converted to integers by webhooks, so explicit int conversion is not required when calculating memory sizes.
Applied to files:
tests/storage/fs_overhead/test_fs_overhead.py
⏰ 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: tox
- GitHub Check: build-container
- GitHub Check: can-be-merged
🔇 Additional comments (2)
tests/storage/fs_overhead/test_fs_overhead.py (2)
83-83: LGTM - explicit access mode improves clarity.Adding the explicit
access_mode=DataVolume.AccessMode.RWOparameter improves code clarity and makes the access mode requirement explicit.
110-119: LGTM - dynamic PVC sizing with proper fallback.The test now correctly uses the storage profile minimum size helper with fallback to the default, ensuring compatibility with storage providers that enforce minimum PVC sizes.
|
D/S test |
openshift-virtualization-qe-bot
requested changes
Jan 7, 2026
openshift-virtualization-qe-bot
left a comment
openshift-virtualization-qe-bot
left a comment
There was a problem hiding this comment.
D/S test tox -e verify-bugs-are-open failed: cnv-tests-tox-executor/22932
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @tests/storage/cdi_clone/test_clone.py:
- Around line 282-283: Replace the two-line sequence that creates an unnecessary
intermediate source_pvc and calls bitmath.parse_string_unsafe with a single
direct call to bitmath.parse_string using the PVC storage string; specifically,
remove the source_pvc variable and set pvc_size =
bitmath.parse_string(s=cirros_dv_with_block_volume_mode.pvc.instance.spec.resources.requests.storage)
so you use the safe parser and access the attribute directly.
- Around line 282-291: The size calculation truncates fractional GiB by using
int(pvc_size.to_GiB().value), potentially making the target smaller than the
source; update the logic around pvc_size and the
create_vm_from_clone_dv_template call to round up instead (use
math.ceil(pvc_size.to_GiB().value) and convert that to int or str as needed) so
the passed size f"{...}Gi" is always >= source, and add the math import if
missing; reference pvc_size and the create_vm_from_clone_dv_template invocation
to locate where to change the conversion.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tests/storage/cdi_clone/test_clone.pytests/storage/cdi_upload/test_upload_virtctl.pytests/storage/fs_overhead/test_fs_overhead.pytests/utils.pyutilities/storage.py
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/storage/fs_overhead/test_fs_overhead.py
- utilities/storage.py
- tests/storage/cdi_upload/test_upload_virtctl.py
- tests/utils.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:
tests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.py
🧠 Learnings (21)
📓 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: 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: 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: 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.
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: 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: 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: 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: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/node/workload_density/test_swap.py:120-124
Timestamp: 2025-08-09T01:47:21.485Z
Learning: In the openshift-virtualization-tests repository, float values for VM memory specifications (like those passed to VirtualMachineForTests memory parameters) are automatically converted to integers by webhooks, so explicit int conversion is not required when calculating memory sizes.
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.
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-13T06:27:29.727Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1641
File: tests/infrastructure/golden_images/test_common_templates_data_volumes.py:47-48
Timestamp: 2025-08-13T06:27:29.727Z
Learning: In tests/infrastructure/golden_images/test_common_templates_data_volumes.py, when set_storage_class is True but get_diff_storage_class_from_matrix returns None (indicating only one storage class is available), the test should continue using the default storage class rather than being skipped. This covers special cluster architectures that only have a single storage class available.
Applied to files:
tests/storage/cdi_clone/test_clone.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/storage/cdi_clone/test_clone.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/storage/cdi_clone/test_clone.py
📚 Learning: 2025-08-27T08:34:49.132Z
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.
Applied to files:
tests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.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/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.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/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.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/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.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: For PR #1904 test execution, the critical validation point is test_connectivity_over_migration_between_localnet_vms which should fail gracefully on cloud clusters but pass on bare-metal/PSI clusters, representing the core nmstate conditional logic functionality.
Applied to files:
tests/storage/cdi_clone/test_clone.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:
tests/storage/cdi_clone/test_clone.py
📚 Learning: 2025-06-23T19:19:31.961Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:83-97
Timestamp: 2025-06-23T19:19:31.961Z
Learning: In OpenShift Virtualization mass machine type transition tests, the kubevirt_api_lifecycle_automation_job requires cluster-admin privileges to function properly, as confirmed by the test maintainer akri3i.
Applied to files:
tests/storage/cdi_clone/test_clone.py
📚 Learning: 2025-11-20T16:27:01.693Z
Learnt from: chandramerla
Repo: RedHatQE/openshift-virtualization-tests PR: 2577
File: tests/virt/node/workload_density/test_free_page_reporting.py:90-90
Timestamp: 2025-11-20T16:27:01.693Z
Learning: The virtio_balloon free page reporting issue on s390x architecture (tracked in OCPBUGS-51113) has been fixed. Free page reporting tests in tests/virt/node/workload_density/test_free_page_reporting.py are now safe to run on s390x clusters.
Applied to files:
tests/storage/cdi_clone/test_clone.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/storage/cdi_clone/test_clone.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/storage/cdi_clone/test_clone.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/cdi_clone/test_clone.py
🧬 Code graph analysis (1)
tests/storage/cdi_clone/test_clone.py (1)
tests/storage/cdi_clone/conftest.py (1)
cirros_dv_with_block_volume_mode(26-37)
⏰ 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: build-container
- GitHub Check: tox
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
Comment on lines
+282
to
+283
| source_pvc = cirros_dv_with_block_volume_mode.pvc | ||
| pvc_size = bitmath.parse_string_unsafe(s=source_pvc.instance.spec.resources.requests.storage) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use parse_string and eliminate unnecessary intermediate variable.
Two improvements:
- Use
bitmath.parse_stringinstead ofparse_string_unsafe(as suggested in past reviews) - Eliminate the
source_pvcintermediate variable since it's used only once
♻️ Proposed refactor
- source_pvc = cirros_dv_with_block_volume_mode.pvc
- pvc_size = bitmath.parse_string_unsafe(s=source_pvc.instance.spec.resources.requests.storage)
+ pvc_size = bitmath.parse_string(s=cirros_dv_with_block_volume_mode.pvc.instance.spec.resources.requests.storage)Based on coding guidelines preferring direct attribute access and past review feedback from rnetser.
🤖 Prompt for AI Agents
In @tests/storage/cdi_clone/test_clone.py around lines 282 - 283, Replace the
two-line sequence that creates an unnecessary intermediate source_pvc and calls
bitmath.parse_string_unsafe with a single direct call to bitmath.parse_string
using the PVC storage string; specifically, remove the source_pvc variable and
set pvc_size =
bitmath.parse_string(s=cirros_dv_with_block_volume_mode.pvc.instance.spec.resources.requests.storage)
so you use the safe parser and access the attribute directly.
Comment on lines
+282
to
+291
| source_pvc = cirros_dv_with_block_volume_mode.pvc | ||
| pvc_size = bitmath.parse_string_unsafe(s=source_pvc.instance.spec.resources.requests.storage) | ||
| create_vm_from_clone_dv_template( | ||
| vm_name="vm-5608", | ||
| dv_name="dv-5608", | ||
| namespace_name=namespace.name, | ||
| source_dv=cirros_dv_with_block_volume_mode, | ||
| client=unprivileged_client, | ||
| volume_mode=DataVolume.VolumeMode.FILE, | ||
| # add fs overhead and round up the result | ||
| size=overhead_size_for_dv( | ||
| image_size=int(cirros_dv_with_block_volume_mode.size[:-2]), | ||
| overhead_value=default_fs_overhead, | ||
| ), | ||
| size=f"{int(pvc_size.to_GiB().value)}Gi", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the create_vm_from_clone_dv_template function and understand size handling
fd -e py -x rg -l 'def create_vm_from_clone_dv_template' {} \;Repository: RedHatQE/openshift-virtualization-tests
Length of output: 122
🏁 Script executed:
#!/bin/bash
# Examine the function signature and implementation
rg -n 'def create_vm_from_clone_dv_template' -A 30Repository: RedHatQE/openshift-virtualization-tests
Length of output: 2175
🏁 Script executed:
#!/bin/bash
# Check how size is used when creating data volumes
rg -n 'data_volume_template_dict|api_name.*storage' -C 3Repository: RedHatQE/openshift-virtualization-tests
Length of output: 19832
🏁 Script executed:
#!/bin/bash
# Examine the data_volume_template_dict implementation
sed -n '542,600p' utilities/storage.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 2210
🏁 Script executed:
#!/bin/bash
# Check surrounding context for size handling
sed -n '530,590p' utilities/storage.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 2038
🏁 Script executed:
#!/bin/bash
# Find the cirros_dv_with_block_volume_mode fixture
rg -n 'def cirros_dv_with_block_volume_mode' -A 20Repository: RedHatQE/openshift-virtualization-tests
Length of output: 1478
🏁 Script executed:
#!/bin/bash
# Search for how cirros data volumes are created
rg -n 'cirros.*block.*volume|block_volume_mode.*cirros' -B 5 -A 10Repository: RedHatQE/openshift-virtualization-tests
Length of output: 2769
🏁 Script executed:
#!/bin/bash
# Find the create_cirros_dv function to check api_name
rg -n 'def create_cirros_dv' -A 30Repository: RedHatQE/openshift-virtualization-tests
Length of output: 1880
🏁 Script executed:
#!/bin/bash
# Check the test context - what is the actual source PVC size?
# Look at the entire test function to understand the flow
sed -n '274,294p' tests/storage/cdi_clone/test_clone.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 850
🏁 Script executed:
#!/bin/bash
# Find Images.Cirros.DEFAULT_DV_SIZE value
rg -n 'class Cirros|DEFAULT_DV_SIZE' utilities/constants.py -A 2 -B 2Repository: RedHatQE/openshift-virtualization-tests
Length of output: 473
🏁 Script executed:
#!/bin/bash
# Find the create_dv function to check what api_name is used
rg -n 'def create_dv' -A 25 tests/storage/utils.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
#!/bin/bash
# Find Cirros image constants
rg -n 'class Cirros' utilities/constants.py -A 10Repository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
#!/bin/bash
# Find create_dv function in storage utils
rg -n 'def create_dv' -A 30 tests/storage/utils.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
#!/bin/bash
# Find all references to Cirros constants
fd -e py -x rg -l 'Cirros' {} \; | head -5Repository: RedHatQE/openshift-virtualization-tests
Length of output: 196
🏁 Script executed:
#!/bin/bash
# Check what's imported in the test file and conftest
head -30 tests/storage/cdi_clone/test_clone.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 770
🏁 Script executed:
#!/bin/bash
# Check conftest imports and find create_dv
head -50 tests/storage/cdi_clone/conftest.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 1452
🏁 Script executed:
#!/bin/bash
# Find create_dv in utilities/storage.py
rg -n 'def create_dv' utilities/storage.py -A 40Repository: RedHatQE/openshift-virtualization-tests
Length of output: 1379
🏁 Script executed:
#!/bin/bash
# Find Images.Cirros constants
rg -n 'Cirros.*=' utilities/constants.py -B 2 -A 8Repository: RedHatQE/openshift-virtualization-tests
Length of output: 1866
🏁 Script executed:
#!/bin/bash
# Find Cirros DEFAULT_DV_SIZE
rg -n 'DEFAULT_DV_SIZE|DEFAULT_MEMORY_SIZE' utilities/constants.py | grep -i cirros -A 2 -B 2Repository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
#!/bin/bash
# Search more broadly for Cirros DEFAULT_DV_SIZE
rg -n 'DEFAULT_DV_SIZE' utilities/constants.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 162
🏁 Script executed:
#!/bin/bash
# Look at the Cirros namedtuple definition
rg -n 'Cirros = namedtuple|class Cirros' utilities/constants.py -B 5 -A 15Repository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
#!/bin/bash
# Check what DEFAULT_DV_SIZE is used in create_cirros_dv
grep -n 'DEFAULT_DV_SIZE' tests/storage/utils.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 225
🏁 Script executed:
#!/bin/bash
# Search for Cirros namedtuple definition
rg -n 'Cirros.*namedtuple' utilities/constants.py -B 2 -A 10Repository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
#!/bin/bash
# Check line 395 context in tests/storage/utils.py to see full definition
sed -n '385,410p' tests/storage/utils.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 687
🏁 Script executed:
#!/bin/bash
# Find the full Cirros namedtuple definition
rg -n 'Cirros = namedtuple' utilities/constants.py -A 20Repository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
#!/bin/bash
# Check if DEFAULT_DV_SIZE is defined in Cirros class
sed -n '40,100p' utilities/constants.py | grep -A 20 'class.*Cirros\|Cirros = 'Repository: RedHatQE/openshift-virtualization-tests
Length of output: 831
🏁 Script executed:
#!/bin/bash
# Look for where DEFAULT_DV_SIZE is added to Cirros
rg -n 'DEFAULT_DV_SIZE.*Cirros|Cirros.*DEFAULT_DV_SIZE' utilities/constants.py -B 3 -A 3Repository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
#!/bin/bash
# Check the entire Images class structure
sed -n '1,250p' utilities/constants.py | tail -100Repository: RedHatQE/openshift-virtualization-tests
Length of output: 3621
🏁 Script executed:
#!/bin/bash
# Find where DEFAULT_DV_SIZE is actually defined for x86_64 Cirros
rg -n 'class X86_64|X86_64:' utilities/constants.py -A 80 | head -100Repository: RedHatQE/openshift-virtualization-tests
Length of output: 3553
🏁 Script executed:
#!/bin/bash
# Look for Cirros definition in globals or module level
rg -n '^Cirros = ' utilities/constants.py -B 5 -A 10Repository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
#!/bin/bash
# Check the namedtuple definition of Cirros
rg -n 'Cirros = namedtuple' utilities/constants.py -B 2 -A 5Repository: RedHatQE/openshift-virtualization-tests
Length of output: 65
🏁 Script executed:
#!/bin/bash
# Check what happens when DEFAULT_DV_SIZE is accessed
python3 << 'EOF'
# Simulate checking if Cirros has DEFAULT_DV_SIZE
# Let's see if it's a dynamically added attribute
import sys
sys.path.insert(0, '.')
# Try importing to see what happens
try:
from utilities.constants import Images
print("Images.Cirros attributes:")
print(dir(Images.Cirros))
if hasattr(Images.Cirros, 'DEFAULT_DV_SIZE'):
print(f"DEFAULT_DV_SIZE = {Images.Cirros.DEFAULT_DV_SIZE}")
else:
print("DEFAULT_DV_SIZE not found")
except Exception as e:
print(f"Error: {e}")
EOFRepository: RedHatQE/openshift-virtualization-tests
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Let me search for how DEFAULT_DV_SIZE is actually defined/used
rg -n 'DEFAULT_DV_SIZE' utilities/constants.py -B 3 -A 1Repository: RedHatQE/openshift-virtualization-tests
Length of output: 612
🏁 Script executed:
#!/bin/bash
# Check if there's a factory function or property that adds DEFAULT_DV_SIZE
rg -n 'DEFAULT_DV_SIZE|setattr.*Cirros' utilities/constants.pyRepository: RedHatQE/openshift-virtualization-tests
Length of output: 162
🏁 Script executed:
#!/bin/bash
# Let me check what the source_dv.size property returns
rg -n 'def.*size|property.*size' utilities/storage.py -A 3 | head -30Repository: RedHatQE/openshift-virtualization-tests
Length of output: 872
Critical: Size truncation makes target smaller than source.
The int() conversion truncates fractional GiB values, resulting in a target PVC smaller than the source. For example:
- Source: "1500Mi"
- Conversion:
1500Mi → ~1.46 GiB → int(1.46) = 1 → "1Gi" = 1024Mi - Result: Target (1024Mi) < Source (1500Mi) ❌
For clone operations, the target must be at least as large as the source.
🐛 Proposed fix using math.ceil()
+import math
+
import bitmath
import pytest
from ocp_resources.datavolume import DataVolume source_pvc = cirros_dv_with_block_volume_mode.pvc
pvc_size = bitmath.parse_string_unsafe(s=source_pvc.instance.spec.resources.requests.storage)
create_vm_from_clone_dv_template(
vm_name="vm-5608",
dv_name="dv-5608",
namespace_name=namespace.name,
source_dv=cirros_dv_with_block_volume_mode,
client=unprivileged_client,
volume_mode=DataVolume.VolumeMode.FILE,
- size=f"{int(pvc_size.to_GiB().value)}Gi",
+ size=f"{math.ceil(pvc_size.to_GiB().value)}Gi",
storage_class=storage_class_with_filesystem_volume_mode,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| source_pvc = cirros_dv_with_block_volume_mode.pvc | |
| pvc_size = bitmath.parse_string_unsafe(s=source_pvc.instance.spec.resources.requests.storage) | |
| create_vm_from_clone_dv_template( | |
| vm_name="vm-5608", | |
| dv_name="dv-5608", | |
| namespace_name=namespace.name, | |
| source_dv=cirros_dv_with_block_volume_mode, | |
| client=unprivileged_client, | |
| volume_mode=DataVolume.VolumeMode.FILE, | |
| # add fs overhead and round up the result | |
| size=overhead_size_for_dv( | |
| image_size=int(cirros_dv_with_block_volume_mode.size[:-2]), | |
| overhead_value=default_fs_overhead, | |
| ), | |
| size=f"{int(pvc_size.to_GiB().value)}Gi", | |
| import math | |
| import bitmath | |
| import pytest | |
| from ocp_resources.datavolume import DataVolume | |
| source_pvc = cirros_dv_with_block_volume_mode.pvc | |
| pvc_size = bitmath.parse_string_unsafe(s=source_pvc.instance.spec.resources.requests.storage) | |
| create_vm_from_clone_dv_template( | |
| vm_name="vm-5608", | |
| dv_name="dv-5608", | |
| namespace_name=namespace.name, | |
| source_dv=cirros_dv_with_block_volume_mode, | |
| client=unprivileged_client, | |
| volume_mode=DataVolume.VolumeMode.FILE, | |
| size=f"{math.ceil(pvc_size.to_GiB().value)}Gi", | |
| storage_class=storage_class_with_filesystem_volume_mode, | |
| ) |
🤖 Prompt for AI Agents
In @tests/storage/cdi_clone/test_clone.py around lines 282 - 291, The size
calculation truncates fractional GiB by using int(pvc_size.to_GiB().value),
potentially making the target smaller than the source; update the logic around
pvc_size and the create_vm_from_clone_dv_template call to round up instead (use
math.ceil(pvc_size.to_GiB().value) and convert that to int or str as needed) so
the passed size f"{...}Gi" is always >= source, and add the math import if
missing; reference pvc_size and the create_vm_from_clone_dv_template invocation
to locate where to change the conversion.
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
10 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:
Adjust minimum supported PVC size for T2 storage configuration
More details:
This PR updates the minimum supported PVC size handling for T2 storage profiles. The change ensures that tests correctly use the minimum supported PVC size as defined in the storage profile annotations, with a fallback to the default value when not specified.
What this PR does / why we need it:
cdi.kubevirt.io/minimumSupportedPvcSizeannotation from storage profilesWhich issue(s) this PR fixes:
in GCP as well as other cloud providor storage sloution are having minimum supported PVC size, where are few tests
that are creating much smaller pvc size which will lead to unbound/ never bound pvc
current affected tests:
test_clone_from_block_to_fs_using_dv_template
test_virtctl_image_upload_with_exist_pvc
test_virtctl_image_upload_dv_with_exist_pvc
test_import_vm_with_specify_fs_overhead
test_upload_dv_with_specify_fs_overhead
Special notes for reviewer:
jira-ticket:
https://issues.redhat.com/browse/CNV-75716
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.