[VIRT] Move virt-only fixtures closer to their usage #2978
+87
−83
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughRelocates and narrows VM-cloning fixtures/API, reintroduces cloning fixtures scoped to cluster vm_cloning, removes several public constants and fixtures (CPU/vendor and cloning hooks) from root conftest, adds session-scoped Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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
Author
|
/wip |
openshift-virtualization-qe-bot-3
changed the title
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-2
requested review from
RoniKishner,
SiboWang1997,
akri3i,
dshchedr,
jerry7z and
kbidarkar
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
approved these changes
Dec 2, 2025
SamAlber
changed the title
SamAlber
force-pushed
the
refactor-virt-only-fixtures
branch
from
d911a15 to
ae35029
Compare
SamAlber
force-pushed
the
refactor-virt-only-fixtures
branch
from
538894a to
0a4e3b8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py (1)
26-35: Docstring and formatting issues remain from past review.The docstring says "Skip test" but the fixture uses
pytest.xfail. Also, Line 33 is missing a space after the comma. These were flagged in previous reviews.@pytest.fixture(scope="session") def xfail_if_no_odf_cephfs_sc(cluster_storage_classes_names): """ - Skip test if no odf cephfs storage class available + Mark test as xfail if no odf cephfs storage class available """ if StorageClassNames.CEPHFS not in cluster_storage_classes_names: pytest.xfail( - f"Cannot execute test, {StorageClassNames.CEPHFS} storage class is not deployed," + f"Cannot execute test, {StorageClassNames.CEPHFS} storage class is not deployed, " f"deployed storage classes: {cluster_storage_classes_names}" )tests/conftest.py (1)
1147-1150: Missing fixture parameter will causeNameError.The fixture
skip_if_no_common_modern_cpureferencesnodes_cpu_architectureon Line 1149 but doesn't declare it as a parameter. This will raise aNameErrorat runtime.Apply this diff:
@pytest.fixture(scope="module") -def skip_if_no_common_modern_cpu(cluster_common_modern_node_cpu): +def skip_if_no_common_modern_cpu(cluster_common_modern_node_cpu, nodes_cpu_architecture): if not cluster_common_modern_node_cpu and nodes_cpu_architecture != ARM_64: pytest.skip("This is a heterogeneous cluster")tests/virt/cluster/vm_cloning/conftest.py (1)
51-54:target_vm_from_cloning_jobis a generator, not a context manager.This was flagged in a previous review. Looking at the relevant code snippet from
utilities/virt.py(lines 2407-2420),target_vm_from_cloning_jobis a generator function that usesyield, not a context manager. Usingwithwill raise aTypeError.Apply this diff:
@pytest.fixture() def target_vm_scope_function(unprivileged_client, cloning_job_scope_function): - with target_vm_from_cloning_job(client=unprivileged_client, cloning_job=cloning_job_scope_function) as target_vm: - yield target_vm + yield from target_vm_from_cloning_job(client=unprivileged_client, cloning_job=cloning_job_scope_function)
🧹 Nitpick comments (1)
tests/virt/conftest.py (1)
395-426: CPU model fixtures follow established pattern.Both
cluster_cpu_model_scope_classandcluster_cpu_model_scope_modulecorrectly use the context manager pattern withwait_for_kv_stabilizecalled after context exit.Note: Past review comments on this PR suggest that updating cluster-level CPU model is expensive (HCO update → wait for resources → teardown reversal). Consider passing
cpu_for_migrationdirectly to VM spec in tests where possible to improve test run time. This is a recommended improvement for follow-up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tests/conftest.py(1 hunks)tests/virt/cluster/vm_cloning/conftest.py(2 hunks)tests/virt/conftest.py(5 hunks)tests/virt/node/conftest.py(3 hunks)tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py(2 hunks)
🧰 Additional context used
🧠 Learnings (37)
📓 Common learnings
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.
Learnt from: 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.
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Learnt from: 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: 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: 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: 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: 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: 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.
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.
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.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 2119
File: tests/network/localnet/conftest.py:352-366
Timestamp: 2025-10-31T13:05:24.570Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, tests run sequentially by default on the same cluster (with module-scoped fixtures properly cleaned up between modules), and parallel test runs use different machines/clusters. This means cluster-scoped resources (like ClusterUserDefinedNetwork or NodeNetworkConfigurationPolicy) can safely use the same names across different test modules without risk of collision.
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/test_windows_os.py:30-31
Timestamp: 2025-09-03T07:23:37.045Z
Learning: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, test_vm_deletion should depend on test_create_vm (not test_start_vm) to ensure resource cleanup happens even if the VM fails to start, preventing resource leaks in the test environment.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1028
File: utilities/infra.py:1257-1258
Timestamp: 2025-08-06T13:57:34.740Z
Learning: In the openshift-virtualization-tests repository, all existing calls to the `get_cluster_platform` function in utilities/infra.py already pass the `admin_client` parameter, so the breaking change requiring this parameter does not actually break any existing code.
📚 Learning: 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/virt/node/conftest.pytests/virt/conftest.pytests/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/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/virt/node/conftest.pytests/virt/conftest.pytests/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/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/virt/node/conftest.pytests/virt/conftest.pytests/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/virt/node/conftest.pytests/virt/conftest.pytests/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-06-18T09:31:06.311Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:31:06.311Z
Learning: In tests/observability/metrics/conftest.py, ResourceEditor context managers automatically restore VM configuration when the context exits, including nodeSelector patches. The fixture pattern with `with ResourceEditor(patches={vm: {...}})` followed by `yield` properly restores the VM to its original state without requiring manual teardown logic.
Applied to files:
tests/virt/node/conftest.pytests/virt/conftest.pytests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-09-02T11:16:59.950Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/conftest.py:589-593
Timestamp: 2025-09-02T11:16:59.950Z
Learning: In tests/conftest.py, for non-baremetal/PSI clusters in the nodes_active_nics fixture, the user prefers to populate the "occupied" field with actual physical NICs from node_physical_nics rather than leaving it empty, to provide downstream consumers with visibility into physical NIC inventory even when NMState isn't managing them.
Applied to files:
tests/virt/node/conftest.pytests/virt/conftest.pytests/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/virt/node/conftest.pytests/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-06-13T01:08:18.579Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 890
File: tests/virt/node/descheduler/conftest.py:146-148
Timestamp: 2025-06-13T01:08:18.579Z
Learning: The fixture `vms_orig_nodes_before_node_drain` in tests/virt/node/descheduler/conftest.py is intentionally kept with the “node_drain” suffix because it represents the state directly before a node drain step; future refactor suggestions should preserve this name unless requirements change.
Applied to files:
tests/virt/node/conftest.pytests/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.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/virt/node/conftest.pytests/virt/conftest.pytests/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-06-22T13:47:35.014Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1183-1186
Timestamp: 2025-06-22T13:47:35.014Z
Learning: In tests/observability/metrics/conftest.py, the `stopped_windows_vm` fixture is designed to temporarily stop the Windows VM for a test, then restart it during teardown (after yield) because the Windows VM is module-scoped and needs to be available for other tests that depend on it being in a running state.
Applied to files:
tests/virt/node/conftest.py
📚 Learning: 2025-09-17T14:02:24.619Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1702
File: tests/chaos/oadp/conftest.py:23-45
Timestamp: 2025-09-17T14:02:24.619Z
Learning: In the openshift-virtualization-tests repository, VirtualMachineForTests class automatically generates unique VM names when generate_unique_name=True (default). It uses utilities.infra.unique_name() which appends a timestamp to create names like "vm-oadp-chaos-{timestamp}", so hardcoded VM names in fixtures don't cause collisions during parallel test execution.
Applied to files:
tests/virt/node/conftest.pytests/conftest.pytests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-10-31T13:05:24.570Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 2119
File: tests/network/localnet/conftest.py:352-366
Timestamp: 2025-10-31T13:05:24.570Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, tests run sequentially by default on the same cluster (with module-scoped fixtures properly cleaned up between modules), and parallel test runs use different machines/clusters. This means cluster-scoped resources (like ClusterUserDefinedNetwork or NodeNetworkConfigurationPolicy) can safely use the same names across different test modules without risk of collision.
Applied to files:
tests/virt/node/conftest.pytests/virt/conftest.pytests/conftest.pytests/virt/cluster/vm_cloning/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/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/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/virt/conftest.pytests/conftest.pytests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-06-23T19:18:12.275Z
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.
Applied to files:
tests/virt/conftest.pytests/conftest.pytests/virt/cluster/vm_cloning/conftest.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/virt/conftest.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/virt/conftest.py
📚 Learning: 2025-08-09T01:52:26.683Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Applied to files:
tests/virt/conftest.py
📚 Learning: 2025-09-08T21:34:28.924Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:57-64
Timestamp: 2025-09-08T21:34:28.924Z
Learning: In OpenShift Virtualization tests, MigrationPolicy fixtures should use static names rather than unique suffixes to enable collision detection. If parallel test runs collide on cluster-scoped resource names like MigrationPolicy, it's better to know about the collision rather than hide it with unique naming, as confirmed by maintainer dshchedr.
Applied to files:
tests/virt/conftest.pytests/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
📚 Learning: 2025-06-23T19:24:28.327Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/test_mass_machine_type_transition.py:97-104
Timestamp: 2025-06-23T19:24:28.327Z
Learning: In OpenShift Virtualization machine type transition tests, the test_machine_type_transition_without_restart method with restart_required=false parameter validates that VM machine types do NOT change when the lifecycle job runs with restart disabled, so the assertion should check against the original machine type rather than the target machine type.
Applied to files:
tests/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.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/virt/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/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-06-23T19:28:20.281Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:24-64
Timestamp: 2025-06-23T19:28:20.281Z
Learning: In OpenShift Virtualization mass machine type transition tests, the machine type glob pattern "pc-q35-rhel8.*.*" is intentionally hard-coded in the kubevirt_api_lifecycle_automation_job as it's used only once for this specific test case, with plans to update it in the future if the job needs to support other machine types.
Applied to files:
tests/virt/conftest.pytests/conftest.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/virt/conftest.pytests/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/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/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-12-07T14:51:53.484Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-07T14:51:53.484Z
Learning: In the openshift-virtualization-tests repository, the team has decided to avoid using predefined time constants (like TIMEOUT_2MIN, TIMEOUT_5SEC) and prefers using explicit numeric values for timeout parameters.
Applied to files:
tests/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/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-05-27T11:44:14.859Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 584
File: tests/observability/metrics/test_network_metrics.py:62-76
Timestamp: 2025-05-27T11:44:14.859Z
Learning: The windows_vm_for_test fixture in tests/observability/metrics/conftest.py does not have a request argument, so it cannot be parametrized using pytest.mark.parametrize with indirect=True. This is different from vm_for_test fixture which accepts parameters through parametrization.
Applied to files:
tests/conftest.pytests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-09-09T11:51:37.860Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 1984
File: tests/install_upgrade_operators/network_policy/test_network_policy_components.py:42-42
Timestamp: 2025-09-09T11:51:37.860Z
Learning: For SNO (Single Node OpenShift) deployments, connectivity tests that require multiple pods should be excluded at the pytest mark level rather than using runtime pytest.skip(), as SNO typically runs single replicas of components.
Applied to files:
tests/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.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/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.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/conftest.py
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: 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/virt/node/migration_and_maintenance/test_odf_vm_migration.py
📚 Learning: 2025-09-03T07:23:37.045Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/test_windows_os.py:30-31
Timestamp: 2025-09-03T07:23:37.045Z
Learning: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, test_vm_deletion should depend on test_create_vm (not test_start_vm) to ensure resource cleanup happens even if the VM fails to start, preventing resource leaks in the test environment.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
📚 Learning: 2025-11-05T01:28:39.322Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 2509
File: tests/virt/cluster/aaq/conftest.py:0-0
Timestamp: 2025-11-05T01:28:39.322Z
Learning: In pytest fixtures within the openshift-virtualization-tests repo, when using `yield from` to delegate to a generator function for setup/teardown (like `update_hco_memory_overcommit`), the delegated function should be a plain generator function without the `contextmanager` decorator. Using `contextmanager` causes a TypeError because it wraps the generator in a `_GeneratorContextManager` object, which is not iterable. The correct pattern is: remove `contextmanager` and use `yield from plain_generator_function(...)` in the fixture.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-05-18T09:24:43.335Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 990
File: utilities/virt.py:1704-1710
Timestamp: 2025-05-18T09:24:43.335Z
Learning: The `migrate_vm_and_verify` function in utilities/virt.py has inconsistent return behavior - it returns a VirtualMachineInstanceMigration object when wait_for_migration_success=False and returns None when wait_for_migration_success=True. This has been properly typed but should be refactored in a future PR for better API design.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-09-12T14:14:28.329Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 1984
File: tests/install_upgrade_operators/network_policy/test_network_policy_components.py:182-187
Timestamp: 2025-09-12T14:14:28.329Z
Learning: Network policy tests in the RedHatQE/openshift-virtualization-tests repository don't require unique resource names as there are no parallel runs on the same cluster in their test environment.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
🧬 Code graph analysis (3)
tests/virt/node/conftest.py (2)
tests/utils.py (1)
update_cluster_cpu_model(271-284)utilities/virt.py (1)
wait_for_kv_stabilize(2274-2276)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py (2)
tests/conftest.py (1)
cluster_storage_classes_names(1347-1348)utilities/constants.py (1)
StorageClassNames(693-709)
tests/virt/cluster/vm_cloning/conftest.py (2)
utilities/virt.py (3)
VirtualMachineForCloning(2376-2404)running_vm(1703-1771)target_vm_from_cloning_job(2408-2421)libs/infra/images.py (1)
Rhel(32-47)
🪛 Ruff (0.14.7)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
53-53: Unused function argument: xfail_test_if_no_odf_cephfs_sc
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: can-be-merged
- GitHub Check: build-container
- GitHub Check: tox
- GitHub Check: can-be-merged
🔇 Additional comments (4)
tests/virt/node/conftest.py (1)
147-178: LGTM - Fixtures correctly implement CPU model updates with stabilization.The new fixtures follow the established pattern: update CPU model within context manager, yield during update, then wait for KV stabilization after context exit. Scope alignment is correct (function-scoped fixture uses function-scoped HCO resource, class-scoped uses class-scoped).
Note: Past review comments on this PR suggest considering passing
cpu_for_migrationdirectly to VMs instead of updating cluster-level CPU model, as it's less expensive. This is an optional improvement for future consideration.tests/virt/cluster/vm_cloning/conftest.py (1)
36-48: LGTM - New RHEL VM fixture for cloning tests.The fixture correctly creates a VirtualMachineForCloning with instance type and preference, runs it, and yields. The use of
withstatement here is correct since VirtualMachineForCloning is a context manager (inherits from VirtualMachineForTests).tests/virt/conftest.py (2)
58-61: Good refactor: Dynamic cluster type detection.Replacing the
is_psi_clusterfixture dependency with a directInfrastructureAPI call simplifies the fixture signature and removes an unnecessary fixture dependency. The logic correctly checks if the platform is OpenStack.
429-431: LGTM - Simple and correct implementation.The
gpu_nodesfixture correctly delegates toget_nodes_with_labelwith the appropriate GPU label.
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
Outdated
Show resolved
Hide resolved
SamAlber
force-pushed
the
refactor-virt-only-fixtures
branch
from
0a4e3b8 to
0037a4c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/conftest.py (1)
1148-1151: Missing fixture parameter causes NameError.The fixture references
nodes_cpu_architectureon line 1150 but does not declare it as a parameter, which will raise aNameErrorat runtime.Apply this diff to add the missing parameter:
@pytest.fixture(scope="module") -def skip_if_no_common_modern_cpu(cluster_common_modern_node_cpu): +def skip_if_no_common_modern_cpu(cluster_common_modern_node_cpu, nodes_cpu_architecture): if not cluster_common_modern_node_cpu and nodes_cpu_architecture != ARM_64: pytest.skip("This is a heterogeneous cluster")
🧹 Nitpick comments (1)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py (1)
26-35: Update docstring to reflect xfail behavior.The docstring says "Skip test" but the fixture uses
pytest.xfail()rather thanpytest.skip(). Update the docstring to accurately describe the behavior.Apply this diff:
@pytest.fixture(scope="session") def xfail_if_no_odf_cephfs_sc(cluster_storage_classes_names): """ - Skip test if no odf cephfs storage class available + Mark test as xfail if no odf cephfs storage class available """ if StorageClassNames.CEPHFS not in cluster_storage_classes_names: pytest.xfail( f"Cannot execute test, {StorageClassNames.CEPHFS} storage class is not deployed," f"deployed storage classes: {cluster_storage_classes_names}" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tests/conftest.py(1 hunks)tests/virt/cluster/vm_cloning/conftest.py(2 hunks)tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/virt/cluster/vm_cloning/conftest.py
🧰 Additional context used
🧠 Learnings (30)
📓 Common learnings
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.
Learnt from: 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.
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Learnt from: 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: 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: 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: 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.
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: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 2838
File: .github/workflows/net-utils-builder-staging.yml:37-37
Timestamp: 2025-11-25T01:56:54.902Z
Learning: In the openshift-virtualization-tests repository, when renaming container images that are built and used by GitHub Actions workflows, the changes must be done sequentially: first merge the workflow files (.github/workflows/) that update the image name in the CI/CD pipelines, then update the code references (like constants.py and manifest files) in a follow-up PR. This prevents the old workflow from running with mismatched image names during the transition.
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: 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.
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.
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.
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-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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/conftest.py
📚 Learning: 2025-09-08T21:34:28.924Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:57-64
Timestamp: 2025-09-08T21:34:28.924Z
Learning: In OpenShift Virtualization tests, MigrationPolicy fixtures should use static names rather than unique suffixes to enable collision detection. If parallel test runs collide on cluster-scoped resource names like MigrationPolicy, it's better to know about the collision rather than hide it with unique naming, as confirmed by maintainer dshchedr.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/conftest.py
📚 Learning: 2025-06-13T01:08:18.579Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 890
File: tests/virt/node/descheduler/conftest.py:146-148
Timestamp: 2025-06-13T01:08:18.579Z
Learning: The fixture `vms_orig_nodes_before_node_drain` in tests/virt/node/descheduler/conftest.py is intentionally kept with the “node_drain” suffix because it represents the state directly before a node drain step; future refactor suggestions should preserve this name unless requirements change.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/conftest.py
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: 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/virt/node/migration_and_maintenance/test_odf_vm_migration.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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/conftest.py
📚 Learning: 2025-06-03T12:36:36.982Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:68-74
Timestamp: 2025-06-03T12:36:36.982Z
Learning: In tests/storage/storage_migration/test_mtc_storage_class_migration.py, the broad Exception catching in the VM migration test is intentional - the user wants to catch every exception and save it for future triage rather than letting specific exceptions bubble up.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/conftest.py
📚 Learning: 2025-09-09T11:51:37.860Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 1984
File: tests/install_upgrade_operators/network_policy/test_network_policy_components.py:42-42
Timestamp: 2025-09-09T11:51:37.860Z
Learning: For SNO (Single Node OpenShift) deployments, connectivity tests that require multiple pods should be excluded at the pytest mark level rather than using runtime pytest.skip(), as SNO typically runs single replicas of components.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/conftest.py
📚 Learning: 2025-06-23T19:24:28.327Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/test_mass_machine_type_transition.py:97-104
Timestamp: 2025-06-23T19:24:28.327Z
Learning: In OpenShift Virtualization machine type transition tests, the test_machine_type_transition_without_restart method with restart_required=false parameter validates that VM machine types do NOT change when the lifecycle job runs with restart disabled, so the assertion should check against the original machine type rather than the target machine type.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
📚 Learning: 2025-09-03T07:23:37.045Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/test_windows_os.py:30-31
Timestamp: 2025-09-03T07:23:37.045Z
Learning: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, test_vm_deletion should depend on test_create_vm (not test_start_vm) to ensure resource cleanup happens even if the VM fails to start, preventing resource leaks in the test environment.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
📚 Learning: 2025-11-19T17:00:58.250Z
Learnt from: chandramerla
Repo: RedHatQE/openshift-virtualization-tests PR: 2577
File: tests/virt/node/hotplug/test_cpu_memory_hotplug.py:161-162
Timestamp: 2025-11-19T17:00:58.250Z
Learning: In the openshift-virtualization-tests repository, the s390x test execution strategy uses positive filtering: only tests explicitly marked with pytest.mark.s390x are executed on s390x clusters. Tests without the s390x marker are automatically excluded from s390x runs, so explicit skipif decorators are not needed to prevent execution on s390x.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.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/conftest.py
📚 Learning: 2025-06-23T19:18:12.275Z
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.
Applied to files:
tests/conftest.py
📚 Learning: 2025-06-18T09:31:06.311Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:31:06.311Z
Learning: In tests/observability/metrics/conftest.py, ResourceEditor context managers automatically restore VM configuration when the context exits, including nodeSelector patches. The fixture pattern with `with ResourceEditor(patches={vm: {...}})` followed by `yield` properly restores the VM to its original state without requiring manual teardown logic.
Applied to files:
tests/conftest.py
📚 Learning: 2025-09-02T11:16:59.950Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/conftest.py:589-593
Timestamp: 2025-09-02T11:16:59.950Z
Learning: In tests/conftest.py, for non-baremetal/PSI clusters in the nodes_active_nics fixture, the user prefers to populate the "occupied" field with actual physical NICs from node_physical_nics rather than leaving it empty, to provide downstream consumers with visibility into physical NIC inventory even when NMState isn't managing them.
Applied to files:
tests/conftest.py
📚 Learning: 2025-12-07T14:51:53.484Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-07T14:51:53.484Z
Learning: In the openshift-virtualization-tests repository, the team has decided to avoid using predefined time constants (like TIMEOUT_2MIN, TIMEOUT_5SEC) and prefers using explicit numeric values for timeout parameters.
Applied to files:
tests/conftest.py
📚 Learning: 2025-09-17T14:02:24.619Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1702
File: tests/chaos/oadp/conftest.py:23-45
Timestamp: 2025-09-17T14:02:24.619Z
Learning: In the openshift-virtualization-tests repository, VirtualMachineForTests class automatically generates unique VM names when generate_unique_name=True (default). It uses utilities.infra.unique_name() which appends a timestamp to create names like "vm-oadp-chaos-{timestamp}", so hardcoded VM names in fixtures don't cause collisions during parallel test execution.
Applied to files:
tests/conftest.py
📚 Learning: 2025-10-31T13:05:24.570Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 2119
File: tests/network/localnet/conftest.py:352-366
Timestamp: 2025-10-31T13:05:24.570Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, tests run sequentially by default on the same cluster (with module-scoped fixtures properly cleaned up between modules), and parallel test runs use different machines/clusters. This means cluster-scoped resources (like ClusterUserDefinedNetwork or NodeNetworkConfigurationPolicy) can safely use the same names across different test modules without risk of collision.
Applied to files:
tests/conftest.py
📚 Learning: 2025-05-27T11:44:14.859Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 584
File: tests/observability/metrics/test_network_metrics.py:62-76
Timestamp: 2025-05-27T11:44:14.859Z
Learning: The windows_vm_for_test fixture in tests/observability/metrics/conftest.py does not have a request argument, so it cannot be parametrized using pytest.mark.parametrize with indirect=True. This is different from vm_for_test fixture which accepts parameters through parametrization.
Applied to files:
tests/conftest.py
🧬 Code graph analysis (1)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py (1)
utilities/constants.py (1)
StorageClassNames(693-709)
🪛 Ruff (0.14.7)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
52-52: Unused function argument: xfail_if_no_odf_cephfs_sc
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: build-container
- GitHub Check: tox
🔇 Additional comments (1)
tests/conftest.py (1)
2610-2624: LGTM: Fixture scope refactoring.The rename from
cluster_cpu_model_scope_classtocluster_cpu_model_scope_functionwith updated scope and dependencies is consistent with the PR's objective to consolidate fixtures. The pattern matches other similar fixtures in this file.
openshift-virtualization-qe-bot
changed the title
SamAlber
force-pushed
the
refactor-virt-only-fixtures
branch
from
0037a4c to
272dd9b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
tests/conftest.py (1)
1148-1151: Add missing fixture parameter to avoid NameError.The fixture references
nodes_cpu_architectureon line 1150 but doesn't declare it as a parameter, which will raise aNameErrorat runtime. Compare withskip_if_no_common_cpu(line 1143) which correctly includesnodes_cpu_architecturein its signature.Apply this diff:
@pytest.fixture(scope="module") -def skip_if_no_common_modern_cpu(cluster_common_modern_node_cpu): +def skip_if_no_common_modern_cpu(cluster_common_modern_node_cpu, nodes_cpu_architecture): if not cluster_common_modern_node_cpu and nodes_cpu_architecture != ARM_64: pytest.skip("This is a heterogeneous cluster")tests/virt/cluster/vm_cloning/conftest.py (1)
51-54: Fix incorrect usage of generator as context manager.
target_vm_from_cloning_jobis a generator function (usesyield), not a context manager (no__enter__/__exit__methods). Usingwithon it will raise aTypeErrorat runtime. Useyield fromto properly delegate to the generator.Apply this diff:
@pytest.fixture() def target_vm_scope_function(unprivileged_client, cloning_job_scope_function): - with target_vm_from_cloning_job(client=unprivileged_client, cloning_job=cloning_job_scope_function) as target_vm: - yield target_vm + yield from target_vm_from_cloning_job(client=unprivileged_client, cloning_job=cloning_job_scope_function)
🧹 Nitpick comments (1)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py (1)
26-35: Update docstring to reflect xfail behavior.The docstring states "Skip test" but the fixture uses
pytest.xfail(), notpytest.skip(). Update the docstring to accurately describe the behavior.Apply this diff:
@pytest.fixture(scope="session") def xfail_if_no_odf_cephfs_sc(cluster_storage_classes_names): """ - Skip test if no odf cephfs storage class available + Mark test as xfail if no odf cephfs storage class available """ if StorageClassNames.CEPHFS not in cluster_storage_classes_names: pytest.xfail( f"Cannot execute test, {StorageClassNames.CEPHFS} storage class is not deployed," f"deployed storage classes: {cluster_storage_classes_names}" )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/conftest.py(1 hunks)tests/virt/cluster/vm_cloning/conftest.py(2 hunks)tests/virt/conftest.py(5 hunks)tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py(2 hunks)
🧰 Additional context used
🧠 Learnings (39)
📓 Common learnings
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.
Learnt from: 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.
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: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Learnt from: 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: 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.
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: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 2838
File: .github/workflows/net-utils-builder-staging.yml:37-37
Timestamp: 2025-11-25T01:56:54.902Z
Learning: In the openshift-virtualization-tests repository, when renaming container images that are built and used by GitHub Actions workflows, the changes must be done sequentially: first merge the workflow files (.github/workflows/) that update the image name in the CI/CD pipelines, then update the code references (like constants.py and manifest files) in a follow-up PR. This prevents the old workflow from running with mismatched image names during the transition.
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: 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.
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 2119
File: tests/network/localnet/conftest.py:352-366
Timestamp: 2025-10-31T13:05:24.570Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, tests run sequentially by default on the same cluster (with module-scoped fixtures properly cleaned up between modules), and parallel test runs use different machines/clusters. This means cluster-scoped resources (like ClusterUserDefinedNetwork or NodeNetworkConfigurationPolicy) can safely use the same names across different test modules without risk of collision.
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: 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.
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.
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-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/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/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/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/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/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/conftest.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/virt/conftest.py
📚 Learning: 2025-06-18T09:31:06.311Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:31:06.311Z
Learning: In tests/observability/metrics/conftest.py, ResourceEditor context managers automatically restore VM configuration when the context exits, including nodeSelector patches. The fixture pattern with `with ResourceEditor(patches={vm: {...}})` followed by `yield` properly restores the VM to its original state without requiring manual teardown logic.
Applied to files:
tests/virt/conftest.pytests/virt/cluster/vm_cloning/conftest.pytests/conftest.py
📚 Learning: 2025-09-02T11:16:59.950Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/conftest.py:589-593
Timestamp: 2025-09-02T11:16:59.950Z
Learning: In tests/conftest.py, for non-baremetal/PSI clusters in the nodes_active_nics fixture, the user prefers to populate the "occupied" field with actual physical NICs from node_physical_nics rather than leaving it empty, to provide downstream consumers with visibility into physical NIC inventory even when NMState isn't managing them.
Applied to files:
tests/virt/conftest.pytests/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/virt/conftest.pytests/virt/cluster/vm_cloning/conftest.pytests/conftest.py
📚 Learning: 2025-12-07T13:52:56.070Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 2920
File: utilities/network.py:31-31
Timestamp: 2025-12-07T13:52:56.070Z
Learning: In the openshift-virtualization-tests repository, it is acceptable for utilities/ modules to import from tests/network/libs/ (e.g., utilities/network.py importing from tests/network/libs/sriovnetworknode). The libs modules under tests/network/libs/ are library/tool modules, not test code, and this import direction does not violate architectural principles in this codebase.
Applied to files:
tests/virt/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/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/conftest.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/virt/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/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/conftest.py
📚 Learning: 2025-06-23T19:18:12.275Z
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.
Applied to files:
tests/virt/conftest.pytests/virt/cluster/vm_cloning/conftest.pytests/conftest.py
📚 Learning: 2025-08-09T01:52:26.683Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Applied to files:
tests/virt/conftest.py
📚 Learning: 2025-06-23T19:24:28.327Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/test_mass_machine_type_transition.py:97-104
Timestamp: 2025-06-23T19:24:28.327Z
Learning: In OpenShift Virtualization machine type transition tests, the test_machine_type_transition_without_restart method with restart_required=false parameter validates that VM machine types do NOT change when the lifecycle job runs with restart disabled, so the assertion should check against the original machine type rather than the target machine type.
Applied to files:
tests/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
📚 Learning: 2025-09-08T21:34:28.924Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:57-64
Timestamp: 2025-09-08T21:34:28.924Z
Learning: In OpenShift Virtualization tests, MigrationPolicy fixtures should use static names rather than unique suffixes to enable collision detection. If parallel test runs collide on cluster-scoped resource names like MigrationPolicy, it's better to know about the collision rather than hide it with unique naming, as confirmed by maintainer dshchedr.
Applied to files:
tests/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/conftest.py
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: The test execution plan for PR #1904 focuses on cluster-type conditional logic where nmstate functionality is bypassed on cloud clusters (Azure/AWS) but fully functional on bare-metal/PSI clusters, requiring different test strategies for each environment type.
Applied to files:
tests/virt/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/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/conftest.py
📚 Learning: 2025-06-23T19:28:20.281Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/conftest.py:24-64
Timestamp: 2025-06-23T19:28:20.281Z
Learning: In OpenShift Virtualization mass machine type transition tests, the machine type glob pattern "pc-q35-rhel8.*.*" is intentionally hard-coded in the kubevirt_api_lifecycle_automation_job as it's used only once for this specific test case, with plans to update it in the future if the job needs to support other machine types.
Applied to files:
tests/virt/conftest.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/virt/conftest.pytests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/conftest.py
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: 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/virt/node/migration_and_maintenance/test_odf_vm_migration.py
📚 Learning: 2025-06-13T01:08:18.579Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 890
File: tests/virt/node/descheduler/conftest.py:146-148
Timestamp: 2025-06-13T01:08:18.579Z
Learning: The fixture `vms_orig_nodes_before_node_drain` in tests/virt/node/descheduler/conftest.py is intentionally kept with the “node_drain” suffix because it represents the state directly before a node drain step; future refactor suggestions should preserve this name unless requirements change.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/conftest.py
📚 Learning: 2025-09-09T11:51:37.860Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 1984
File: tests/install_upgrade_operators/network_policy/test_network_policy_components.py:42-42
Timestamp: 2025-09-09T11:51:37.860Z
Learning: For SNO (Single Node OpenShift) deployments, connectivity tests that require multiple pods should be excluded at the pytest mark level rather than using runtime pytest.skip(), as SNO typically runs single replicas of components.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/conftest.py
📚 Learning: 2025-09-03T07:23:37.045Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/test_windows_os.py:30-31
Timestamp: 2025-09-03T07:23:37.045Z
Learning: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, test_vm_deletion should depend on test_create_vm (not test_start_vm) to ensure resource cleanup happens even if the VM fails to start, preventing resource leaks in the test environment.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/conftest.py
📚 Learning: 2025-06-03T12:36:36.982Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:68-74
Timestamp: 2025-06-03T12:36:36.982Z
Learning: In tests/storage/storage_migration/test_mtc_storage_class_migration.py, the broad Exception catching in the VM migration test is intentional - the user wants to catch every exception and save it for future triage rather than letting specific exceptions bubble up.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.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/virt/cluster/vm_cloning/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/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-09-17T14:02:24.619Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1702
File: tests/chaos/oadp/conftest.py:23-45
Timestamp: 2025-09-17T14:02:24.619Z
Learning: In the openshift-virtualization-tests repository, VirtualMachineForTests class automatically generates unique VM names when generate_unique_name=True (default). It uses utilities.infra.unique_name() which appends a timestamp to create names like "vm-oadp-chaos-{timestamp}", so hardcoded VM names in fixtures don't cause collisions during parallel test execution.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.pytests/conftest.py
📚 Learning: 2025-11-05T01:28:39.322Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 2509
File: tests/virt/cluster/aaq/conftest.py:0-0
Timestamp: 2025-11-05T01:28:39.322Z
Learning: In pytest fixtures within the openshift-virtualization-tests repo, when using `yield from` to delegate to a generator function for setup/teardown (like `update_hco_memory_overcommit`), the delegated function should be a plain generator function without the `contextmanager` decorator. Using `contextmanager` causes a TypeError because it wraps the generator in a `_GeneratorContextManager` object, which is not iterable. The correct pattern is: remove `contextmanager` and use `yield from plain_generator_function(...)` in the fixture.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-05-18T09:24:43.335Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 990
File: utilities/virt.py:1704-1710
Timestamp: 2025-05-18T09:24:43.335Z
Learning: The `migrate_vm_and_verify` function in utilities/virt.py has inconsistent return behavior - it returns a VirtualMachineInstanceMigration object when wait_for_migration_success=False and returns None when wait_for_migration_success=True. This has been properly typed but should be refactored in a future PR for better API design.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-05-27T11:44:14.859Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 584
File: tests/observability/metrics/test_network_metrics.py:62-76
Timestamp: 2025-05-27T11:44:14.859Z
Learning: The windows_vm_for_test fixture in tests/observability/metrics/conftest.py does not have a request argument, so it cannot be parametrized using pytest.mark.parametrize with indirect=True. This is different from vm_for_test fixture which accepts parameters through parametrization.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.pytests/conftest.py
📚 Learning: 2025-10-31T13:05:24.570Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 2119
File: tests/network/localnet/conftest.py:352-366
Timestamp: 2025-10-31T13:05:24.570Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, tests run sequentially by default on the same cluster (with module-scoped fixtures properly cleaned up between modules), and parallel test runs use different machines/clusters. This means cluster-scoped resources (like ClusterUserDefinedNetwork or NodeNetworkConfigurationPolicy) can safely use the same names across different test modules without risk of collision.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.pytests/conftest.py
📚 Learning: 2025-09-12T14:14:28.329Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 1984
File: tests/install_upgrade_operators/network_policy/test_network_policy_components.py:182-187
Timestamp: 2025-09-12T14:14:28.329Z
Learning: Network policy tests in the RedHatQE/openshift-virtualization-tests repository don't require unique resource names as there are no parallel runs on the same cluster in their test environment.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-12-07T14:51:53.484Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-07T14:51:53.484Z
Learning: In the openshift-virtualization-tests repository, the team has decided to avoid using predefined time constants (like TIMEOUT_2MIN, TIMEOUT_5SEC) and prefers using explicit numeric values for timeout parameters.
Applied to files:
tests/conftest.py
🧬 Code graph analysis (3)
tests/virt/conftest.py (2)
utilities/infra.py (2)
get_nodes_with_label(788-789)label_nodes(668-675)tests/conftest.py (1)
nodes(420-421)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py (2)
tests/conftest.py (1)
cluster_storage_classes_names(1348-1349)utilities/constants.py (1)
StorageClassNames(693-709)
tests/conftest.py (1)
tests/utils.py (1)
update_cluster_cpu_model(271-284)
🪛 Ruff (0.14.7)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
52-52: Unused function argument: xfail_if_no_odf_cephfs_sc
(ARG001)
🔇 Additional comments (5)
tests/virt/conftest.py (2)
10-10: LGTM! Infrastructure detection refactoring is clean.The refactoring of
_verify_not_psi_clusterto directly queryInfrastructure(name="cluster").instance.status.platforminstead of accepting a parameter improves code clarity and removes unnecessary coupling. The added imports are used correctly.Also applies to: 34-34, 57-60, 146-146
394-396: LGTM! GPU nodes fixture properly relocated.The
gpu_nodesfixture is correctly implemented with session scope and properly filters nodes using the importedget_nodes_with_labelutility. This aligns with the PR objective of moving virt-only fixtures closer to their usage.tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py (1)
52-52: Note: Unused fixture parameter is intentional.The
xfail_if_no_odf_cephfs_scparameter appears unused but is intentionally included to ensure the fixture executes before the test runs, controlling the execution order. This is an idiomatic pytest pattern.Based on learnings, the team prefers to leave such fixtures unchanged rather than suppressing linter warnings.
tests/virt/cluster/vm_cloning/conftest.py (1)
36-48: LGTM! RHEL cloning fixture properly implemented.The
rhel_vm_with_instancetype_and_preference_for_cloningfixture correctly usesVirtualMachineForCloningas a context manager with proper resource configuration for RHEL-based cloning scenarios.tests/conftest.py (1)
2610-2624: LGTM! Fixture scope change from class to function.The
cluster_cpu_model_scope_functionfixture has been correctly updated to use function scope withhyperconverged_resource_scope_function, and includes proper cleanup withwait_for_kv_stabilizeat teardown. This aligns with the PR's refactoring objectives.
SamAlber
force-pushed
the
refactor-virt-only-fixtures
branch
from
272dd9b to
d74dbbf
Compare
Move fixtures that are used exclusively by tests/virt from the main tests/conftest.py to tests/virt/conftest.py for better organization and proximity to their usage.
SamAlber
force-pushed
the
refactor-virt-only-fixtures
branch
from
d74dbbf to
dddd30d
Compare
SamAlber
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py (1)
26-35: Minor wording and spacing tweaks in xfail fixture.Behavior is correct, but two tiny text issues remain:
- Docstring still says “Skip test” while the fixture uses
pytest.xfail; consider mentioning xfail explicitly.- The concatenated f-strings produce
"...is not deployed,deployed storage classes..."(missing space after the comma).Suggested diff:
@pytest.fixture(scope="session") def xfail_if_no_odf_cephfs_sc(cluster_storage_classes_names): - """ - Skip test if no odf cephfs storage class available - """ + """ + Mark test as xfail if no odf cephfs storage class available + """ if StorageClassNames.CEPHFS not in cluster_storage_classes_names: pytest.xfail( - f"Cannot execute test, {StorageClassNames.CEPHFS} storage class is not deployed," - f"deployed storage classes: {cluster_storage_classes_names}" + f"Cannot execute test, {StorageClassNames.CEPHFS} storage class is not deployed, " + f"deployed storage classes: {cluster_storage_classes_names}" )tests/virt/cluster/vm_cloning/conftest.py (1)
70-73: Fixtarget_vm_scope_function:target_vm_from_cloning_jobis also a generatorAs noted previously,
target_vm_from_cloning_jobis a generator (it callsyield target_vmthen cleans up), not a context manager. Usingwith target_vm_from_cloning_job(...) as target_vm:will raiseTypeError: __enter__.Mirror the same
yield frompattern as other generator-backed fixtures:@pytest.fixture() def target_vm_scope_function(unprivileged_client, cloning_job_scope_function): - with target_vm_from_cloning_job(client=unprivileged_client, cloning_job=cloning_job_scope_function) as target_vm: - yield target_vm + yield from target_vm_from_cloning_job( + client=unprivileged_client, + cloning_job=cloning_job_scope_function, + )Based on learnings, delegating directly to generator helpers avoids the context-manager/type mismatch and preserves their teardown behavior.
🧹 Nitpick comments (1)
tests/virt/conftest.py (1)
34-35: GPU/CPU helper fixtures are reasonable; consider slightly more defensive CPU-vendor detection
gpu_nodesreusesget_nodes_with_labelto derive GPU nodes and integrates cleanly with existing GPU-related fixtures.gpu_nodes_labeled_with_vm_vgpucontinues to uselabel_nodesas before, now with the shared import.For
nodes_cpu_vendor, you currently infer vendor fromschedulable_nodes[0]:if schedulable_nodes[0].labels.get(f"cpu-vendor.node.kubevirt.io/{AMD}"): return AMD elif schedulable_nodes[0].labels.get(f"cpu-vendor.node.kubevirt.io/{INTEL}"): return INTELThis works if:
- there is at least one schedulable node, and
- all schedulable nodes share the same CPU-vendor label, and
- the first node is labeled.
If you ever hit clusters with mixed vendors or partially-labeled nodes, this could silently return
Noneeven when some nodes are usable. A slightly more defensive option (scan all schedulable_nodes and, e.g., pick the first labeled vendor or assert they are consistent) would make the behavior more robust without changing the public fixture shape.Given this PR is mainly organizational/refactoring, it’s fine to keep as is for now; consider tightening it in a follow-up if you foresee heterogeneous clusters.
Also applies to: 233-240, 394-406
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/conftest.py(0 hunks)tests/virt/cluster/vm_cloning/conftest.py(2 hunks)tests/virt/conftest.py(5 hunks)tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py(2 hunks)
💤 Files with no reviewable changes (1)
- tests/conftest.py
🧰 Additional context used
🧠 Learnings (37)
📓 Common learnings
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 2045
File: tests/virt/cluster/vm_lifecycle/conftest.py:46-47
Timestamp: 2025-09-15T06:49:53.478Z
Learning: In the openshift-virtualization-tests repo, large fixture refactoring efforts like the golden image data source migration are handled incrementally by directory/team ownership. The virt/cluster directory is handled separately from virt/node, tests/infra, tests/storage, etc., with each area managed by relevant teams in follow-up PRs.
Learnt from: 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.
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Learnt from: 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: 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.
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: 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: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 2838
File: .github/workflows/net-utils-builder-staging.yml:37-37
Timestamp: 2025-11-25T01:56:54.902Z
Learning: In the openshift-virtualization-tests repository, when renaming container images that are built and used by GitHub Actions workflows, the changes must be done sequentially: first merge the workflow files (.github/workflows/) that update the image name in the CI/CD pipelines, then update the code references (like constants.py and manifest files) in a follow-up PR. This prevents the old workflow from running with mismatched image names during the transition.
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: 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.
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:57-64
Timestamp: 2025-09-08T21:34:28.924Z
Learning: In OpenShift Virtualization tests, MigrationPolicy fixtures should use static names rather than unique suffixes to enable collision detection. If parallel test runs collide on cluster-scoped resource names like MigrationPolicy, it's better to know about the collision rather than hide it with unique naming, as confirmed by maintainer dshchedr.
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.
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: 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.
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.
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: 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.
📚 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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/virt/conftest.py
📚 Learning: 2025-09-08T21:34:28.924Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1932
File: tests/virt/node/migration_and_maintenance/conftest.py:57-64
Timestamp: 2025-09-08T21:34:28.924Z
Learning: In OpenShift Virtualization tests, MigrationPolicy fixtures should use static names rather than unique suffixes to enable collision detection. If parallel test runs collide on cluster-scoped resource names like MigrationPolicy, it's better to know about the collision rather than hide it with unique naming, as confirmed by maintainer dshchedr.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/virt/conftest.py
📚 Learning: 2025-06-13T01:08:18.579Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 890
File: tests/virt/node/descheduler/conftest.py:146-148
Timestamp: 2025-06-13T01:08:18.579Z
Learning: The fixture `vms_orig_nodes_before_node_drain` in tests/virt/node/descheduler/conftest.py is intentionally kept with the “node_drain” suffix because it represents the state directly before a node drain step; future refactor suggestions should preserve this name unless requirements change.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/conftest.py
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: 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/virt/node/migration_and_maintenance/test_odf_vm_migration.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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/virt/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/virt/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/virt/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/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/virt/node/migration_and_maintenance/test_odf_vm_migration.py
📚 Learning: 2025-09-09T11:51:37.860Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 1984
File: tests/install_upgrade_operators/network_policy/test_network_policy_components.py:42-42
Timestamp: 2025-09-09T11:51:37.860Z
Learning: For SNO (Single Node OpenShift) deployments, connectivity tests that require multiple pods should be excluded at the pytest mark level rather than using runtime pytest.skip(), as SNO typically runs single replicas of components.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
📚 Learning: 2025-06-23T19:24:28.327Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/test_mass_machine_type_transition.py:97-104
Timestamp: 2025-06-23T19:24:28.327Z
Learning: In OpenShift Virtualization machine type transition tests, the test_machine_type_transition_without_restart method with restart_required=false parameter validates that VM machine types do NOT change when the lifecycle job runs with restart disabled, so the assertion should check against the original machine type rather than the target machine type.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/conftest.py
📚 Learning: 2025-09-03T07:23:37.045Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/test_windows_os.py:30-31
Timestamp: 2025-09-03T07:23:37.045Z
Learning: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, test_vm_deletion should depend on test_create_vm (not test_start_vm) to ensure resource cleanup happens even if the VM fails to start, preventing resource leaks in the test environment.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/conftest.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/virt/node/migration_and_maintenance/test_odf_vm_migration.pytests/virt/cluster/vm_cloning/conftest.pytests/virt/conftest.py
📚 Learning: 2025-06-03T12:36:36.982Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 954
File: tests/storage/storage_migration/test_mtc_storage_class_migration.py:68-74
Timestamp: 2025-06-03T12:36:36.982Z
Learning: In tests/storage/storage_migration/test_mtc_storage_class_migration.py, the broad Exception catching in the VM migration test is intentional - the user wants to catch every exception and save it for future triage rather than letting specific exceptions bubble up.
Applied to files:
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
📚 Learning: 2025-09-17T14:02:24.619Z
Learnt from: qwang1
Repo: RedHatQE/openshift-virtualization-tests PR: 1702
File: tests/chaos/oadp/conftest.py:23-45
Timestamp: 2025-09-17T14:02:24.619Z
Learning: In the openshift-virtualization-tests repository, VirtualMachineForTests class automatically generates unique VM names when generate_unique_name=True (default). It uses utilities.infra.unique_name() which appends a timestamp to create names like "vm-oadp-chaos-{timestamp}", so hardcoded VM names in fixtures don't cause collisions during parallel test execution.
Applied to files:
tests/virt/cluster/vm_cloning/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/virt/cluster/vm_cloning/conftest.pytests/virt/conftest.py
📚 Learning: 2025-06-23T19:18:12.275Z
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.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.pytests/virt/conftest.py
📚 Learning: 2025-06-18T09:31:06.311Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:31:06.311Z
Learning: In tests/observability/metrics/conftest.py, ResourceEditor context managers automatically restore VM configuration when the context exits, including nodeSelector patches. The fixture pattern with `with ResourceEditor(patches={vm: {...}})` followed by `yield` properly restores the VM to its original state without requiring manual teardown logic.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.pytests/virt/conftest.py
📚 Learning: 2025-11-05T01:28:39.322Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 2509
File: tests/virt/cluster/aaq/conftest.py:0-0
Timestamp: 2025-11-05T01:28:39.322Z
Learning: In pytest fixtures within the openshift-virtualization-tests repo, when using `yield from` to delegate to a generator function for setup/teardown (like `update_hco_memory_overcommit`), the delegated function should be a plain generator function without the `contextmanager` decorator. Using `contextmanager` causes a TypeError because it wraps the generator in a `_GeneratorContextManager` object, which is not iterable. The correct pattern is: remove `contextmanager` and use `yield from plain_generator_function(...)` in the fixture.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-05-18T09:24:43.335Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 990
File: utilities/virt.py:1704-1710
Timestamp: 2025-05-18T09:24:43.335Z
Learning: The `migrate_vm_and_verify` function in utilities/virt.py has inconsistent return behavior - it returns a VirtualMachineInstanceMigration object when wait_for_migration_success=False and returns None when wait_for_migration_success=True. This has been properly typed but should be refactored in a future PR for better API design.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-05-27T11:44:14.859Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 584
File: tests/observability/metrics/test_network_metrics.py:62-76
Timestamp: 2025-05-27T11:44:14.859Z
Learning: The windows_vm_for_test fixture in tests/observability/metrics/conftest.py does not have a request argument, so it cannot be parametrized using pytest.mark.parametrize with indirect=True. This is different from vm_for_test fixture which accepts parameters through parametrization.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-10-31T13:05:24.570Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 2119
File: tests/network/localnet/conftest.py:352-366
Timestamp: 2025-10-31T13:05:24.570Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, tests run sequentially by default on the same cluster (with module-scoped fixtures properly cleaned up between modules), and parallel test runs use different machines/clusters. This means cluster-scoped resources (like ClusterUserDefinedNetwork or NodeNetworkConfigurationPolicy) can safely use the same names across different test modules without risk of collision.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-09-12T14:14:28.329Z
Learnt from: rlobillo
Repo: RedHatQE/openshift-virtualization-tests PR: 1984
File: tests/install_upgrade_operators/network_policy/test_network_policy_components.py:182-187
Timestamp: 2025-09-12T14:14:28.329Z
Learning: Network policy tests in the RedHatQE/openshift-virtualization-tests repository don't require unique resource names as there are no parallel runs on the same cluster in their test environment.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-08-14T10:28:22.958Z
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 1785
File: tests/virt/cluster/common_templates/utils.py:65-66
Timestamp: 2025-08-14T10:28:22.958Z
Learning: The restart_qemu_guest_agent_service function in tests/virt/cluster/common_templates/utils.py is only called for RHEL7 VMs, with OS version checks handled by the calling test code, not within the function itself. Guest agent functionality is verified by subsequent tests in the test class.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.py
📚 Learning: 2025-08-20T23:57:48.380Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1840
File: tests/virt/node/workload_density/test_swap.py:88-89
Timestamp: 2025-08-20T23:57:48.380Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, CNV installation and health are verified by sanity tests before other tests run, so hco_namespace is guaranteed to exist in the testing environment. Defensive programming against nil hco_namespace scenarios is not needed in fixtures.
Applied to files:
tests/virt/cluster/vm_cloning/conftest.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/virt/conftest.py
📚 Learning: 2025-09-02T11:16:59.950Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/conftest.py:589-593
Timestamp: 2025-09-02T11:16:59.950Z
Learning: In tests/conftest.py, for non-baremetal/PSI clusters in the nodes_active_nics fixture, the user prefers to populate the "occupied" field with actual physical NICs from node_physical_nics rather than leaving it empty, to provide downstream consumers with visibility into physical NIC inventory even when NMState isn't managing them.
Applied to files:
tests/virt/conftest.py
📚 Learning: 2025-12-07T13:52:56.070Z
Learnt from: EdDev
Repo: RedHatQE/openshift-virtualization-tests PR: 2920
File: utilities/network.py:31-31
Timestamp: 2025-12-07T13:52:56.070Z
Learning: In the openshift-virtualization-tests repository, it is acceptable for utilities/ modules to import from tests/network/libs/ (e.g., utilities/network.py importing from tests/network/libs/sriovnetworknode). The libs modules under tests/network/libs/ are library/tool modules, not test code, and this import direction does not violate architectural principles in this codebase.
Applied to files:
tests/virt/conftest.py
📚 Learning: 2025-08-09T01:52:26.683Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 1716
File: tests/virt/conftest.py:289-297
Timestamp: 2025-08-09T01:52:26.683Z
Learning: When user dshchedr moves working code from one location to another in the openshift-virtualization-tests repository, they prefer not to modify it unless there's a real issue, maintaining the original implementation to avoid introducing unnecessary changes.
Applied to files:
tests/virt/conftest.py
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: The test execution plan for PR #1904 focuses on cluster-type conditional logic where nmstate functionality is bypassed on cloud clusters (Azure/AWS) but fully functional on bare-metal/PSI clusters, requiring different test strategies for each environment type.
Applied to files:
tests/virt/conftest.py
📚 Learning: 2025-06-13T00:47:10.948Z
Learnt from: dshchedr
Repo: RedHatQE/openshift-virtualization-tests PR: 890
File: tests/virt/node/descheduler/conftest.py:394-404
Timestamp: 2025-06-13T00:47:10.948Z
Learning: In descheduler test fixtures (`stressed_vms_on_one_node`), explicit termination of `stress-ng` processes isn’t needed because the VMs themselves are deleted during teardown, making additional cleanup redundant.
Applied to files:
tests/virt/conftest.py
🧬 Code graph analysis (2)
tests/virt/cluster/vm_cloning/conftest.py (3)
utilities/virt.py (5)
VirtualMachineForCloning(2376-2404)create_vm_cloning_job(2020-2058)fedora_vm_body(1423-1445)running_vm(1703-1771)target_vm_from_cloning_job(2408-2421)tests/conftest.py (2)
namespace(654-667)unprivileged_client(384-416)libs/infra/images.py (1)
Rhel(32-47)
tests/virt/conftest.py (1)
utilities/infra.py (3)
ExecCommandOnPod(449-502)get_nodes_with_label(788-789)label_nodes(668-675)
🪛 Ruff (0.14.7)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py
52-52: Unused function argument: xfail_if_no_odf_cephfs_sc
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-container
- GitHub Check: tox
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
🔇 Additional comments (4)
tests/virt/node/migration_and_maintenance/test_odf_vm_migration.py (1)
52-52: Gating via explicit xfail fixture parameter looks good (ARG001 is expected).Using
xfail_if_no_odf_cephfs_scas an explicit test parameter cleanly gates the test on ODF CephFS availability and aligns with the team’s preference for declaring fixtures directly in the test signature. The fact that the parameter is unused in the body (Ruff ARG001) is intentional and acceptable here, since the fixture is used purely for setup/ordering. Based on learnings, this pattern is preferred over chaining fixtures.tests/virt/cluster/vm_cloning/conftest.py (2)
2-22: New imports for instancetype/preference and cloning utilities look correctThe added imports align with the new cloning fixtures and existing utilities/constants usage; no issues spotted here.
42-55: RHEL VM cloning fixture wiring looks consistentThe
rhel_vm_with_instancetype_and_preference_for_cloningfixture correctly wiresVirtualMachineForCloningwith the RHEL registry guest image, instancetype, preference, and OS flavor, mirroring existing Fedora patterns while targeting the new use case.tests/virt/conftest.py (1)
57-60: Infrastructure-based psi-cluster detection invirt_special_infra_sanitylooks fineSwitching
_verify_not_psi_clusterto useInfrastructure(name="cluster").instance.status.platformand updating the call site keeps the check localized and avoids plumbing anis_psi_clusterflag through the fixture. The logic and call pattern remain clear and low-cost.Also applies to: 146-146
Comment on lines
+57
to
+67
| @pytest.fixture() | ||
| def cloning_job_scope_function(request, unprivileged_client, namespace): | ||
| with create_vm_cloning_job( | ||
| name=f"clone-job-{request.param['source_name']}", | ||
| client=unprivileged_client, | ||
| namespace=namespace.name, | ||
| source_name=request.param["source_name"], | ||
| label_filters=request.param.get("label_filters"), | ||
| annotation_filters=request.param.get("annotation_filters"), | ||
| ) as vmc: | ||
| yield vmc |
There was a problem hiding this comment.
Fix cloning_job_scope_function: generator misused as a context manager
create_vm_cloning_job is implemented as a generator (it uses yield inside a with VirtualMachineClone(...) block), not as a context manager. Using it in a with statement will raise TypeError: __enter__ and break all tests using this fixture.
Delegate directly to the generator with yield from so that setup/teardown from create_vm_cloning_job are preserved. Apply this diff:
@pytest.fixture()
def cloning_job_scope_function(request, unprivileged_client, namespace):
- with create_vm_cloning_job(
- name=f"clone-job-{request.param['source_name']}",
- client=unprivileged_client,
- namespace=namespace.name,
- source_name=request.param["source_name"],
- label_filters=request.param.get("label_filters"),
- annotation_filters=request.param.get("annotation_filters"),
- ) as vmc:
- yield vmc
+ yield from create_vm_cloning_job(
+ name=f"clone-job-{request.param['source_name']}",
+ client=unprivileged_client,
+ namespace=namespace.name,
+ source_name=request.param["source_name"],
+ label_filters=request.param.get("label_filters"),
+ annotation_filters=request.param.get("annotation_filters"),
+ )Based on learnings, using yield from to delegate to generator-style helpers is the preferred fixture pattern here.
🤖 Prompt for AI Agents
In tests/virt/cluster/vm_cloning/conftest.py around lines 57 to 67, the fixture
uses "with create_vm_cloning_job(...)" but create_vm_cloning_job is a
generator-based helper (it yields) not a context manager, causing a TypeError;
replace the context-manager usage by delegating to the generator using "yield
from create_vm_cloning_job(...)" so the helper's setup/teardown run correctly
and the fixture yields the cloned VM object.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 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.
This PR Moves virt-only fixtures closer to their usage from the main tests/conftest.py to tests/virt/conftest.py for better organization and proximity to their usage.
Short description:
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
https://issues.redhat.com/browse/CNV-72171
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.