Skip to content

Conversation

@yossisegev
Copy link
Contributor

@yossisegev yossisegev commented Dec 22, 2025

Removing 3 network upgrade tests that are not contributing anything. The dependent fixtures and functions were also removed, except for those that are still used by other flows.
Polarion test cases deleted as well.

The rationale of removing each of these tests:

  • test_nmstate_bridge_before_upgrade: Needless to explain this is a pure nmstate flow, that doesn't involve CNV.
  • test_bridge_marker_before_upgrade: Bridge-marker is indirectly covered in any bridge test, which would fail if the bridge-marker is disfunctioning.
  • test_install_ovs_with_annotations_before_upgrade: Actively opting-in OVS CNI is not needed on OVN-k8s clusters, which is now the default.
jira-ticket:

https://issues.redhat.com/browse/CNV-70147

Summary by CodeRabbit

  • Tests
    • Removed several pre- and post-upgrade network validation tests related to NMState bridge creation, bridge-to-node markers, and OVS annotation checks.
    • Simplified test utilities by deleting bridge/node validation helpers and related assertions, leaving a smaller utility surface.
    • Retained core upgrade coverage: VM interface counts, connectivity (including macspoof), Linux bridge checks, and Kubemacpool status verifications.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 22, 2025

Walkthrough

Removed six upgrade test methods and three helper utilities related to NMState bridge creation, bridge marker validation, and OVS annotation verification; imports updated to remove the deleted utilities.

Changes

Cohort / File(s) Summary
Test removals
tests/network/upgrade/test_upgrade_network.py
Deleted six tests: test_nmstate_bridge_before_upgrade, test_bridge_marker_before_upgrade, test_nmstate_bridge_after_upgrade, test_bridge_marker_after_upgrade, test_install_ovs_with_annotations_before_upgrade, test_ovs_installed_with_annotations_after_upgrade. Removed their dependency/order markers and related assertions.
Import adjustments in tests
tests/network/upgrade/test_upgrade_network.py
Reduced imports from tests.network.upgrade.utils to only assert_label_in_namespace; removed verify_ovs_installed_with_annotations and other nmstate-related imports.
Utility function removals
tests/network/upgrade/utils.py
Deleted three helper functions: assert_bridge_and_vms_on_same_node(), assert_node_is_marked_by_bridge(), assert_nmstate_bridge_creation() (NodeNetworkState polling and bridge/node validation helpers removed).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

can-be-merged, tox:verify-tc-requirement-polarion:passed, tox:verify-bugs-are-open:passed

Suggested reviewers

  • rnetser
  • dshchedr
  • vsibirsk
  • RoniKishner
  • EdDev

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title '[upgrade/network] Remove redundant network upgrade tests' directly and accurately summarizes the main change: removal of redundant tests from the network upgrade test module.
Description check ✅ Passed The description is mostly complete, covering rationale for removals and the Jira ticket. However, it lacks details in template sections like 'Short description', 'More details', 'Which issue(s)', and 'Special notes for reviewer'.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between adc289d and 1792d80.

📒 Files selected for processing (2)
  • tests/network/upgrade/test_upgrade_network.py
  • tests/network/upgrade/utils.py
💤 Files with no reviewable changes (1)
  • tests/network/upgrade/utils.py
🧰 Additional context used
🧠 Learnings (27)
📓 Common learnings
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 3196
File: tests/network/upgrade/test_upgrade_network.py:4-4
Timestamp: 2025-12-22T16:27:44.327Z
Learning: For PRs that remove test cases (especially redundant test cleanup PRs in openshift-virtualization-tests), test collection verification (pytest --collect-only showing selected/deselected counts) is sufficient to confirm the removal was clean and the test module remains functional. Full test execution is not required for test deletion PRs.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-22T15:56:00.157Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, do NOT use "REQUEST_CHANGES" review type if the PR author has already marked the PR as verified (e.g., with `/verified` command). Test execution plans are informational guides, not blocking requirements. Use COMMENT event for informational test plans, or only REQUEST_CHANGES if there are actual code issues that need to be addressed before merging.
Learnt from: HarshithaMS005
Repo: RedHatQE/openshift-virtualization-tests PR: 2027
File: tests/network/nmstate/test_connectivity_after_nmstate_changes.py:231-233
Timestamp: 2025-10-08T07:16:46.347Z
Learning: Network tests in the openshift-virtualization-tests repository have a sanity check in tests/network/conftest.py (around line 237) that validates len(hosts_common_available_ports) > 1 before tests run. This ensures multinic tests requiring hosts_common_available_ports[-2] won't encounter IndexError, making fixture-level guards redundant.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/network/conftest.py:348-362
Timestamp: 2025-10-27T15:30:06.412Z
Learning: In tests/network/conftest.py, the _verify_nmstate_running_pods function currently runs unconditionally in network_sanity, but rnetser plans to implement marker-based conditional checking (following the pattern of _verify_dpdk, _verify_sriov, etc.) in a future PR after adding nmstate markers to the relevant tests.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:00:59.076Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, CodeRabbit must post ONLY an inline review comment on the Files Changed tab and then stop immediately without generating any follow-up comments in the PR discussion thread. No acknowledgment messages, no confirmation of posting, no explanation - silence after posting the inline review equals success. Additional comments create empty/meaningless reviews that clutter the PR.
Learnt from: 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: 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: 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.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T10:28:54.212Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for test execution plans, only set "Run smoke tests: True" if there is a verified, traceable dependency path from smoke tests to the changed code. Session-scoped fixtures or infrastructure-sounding changes do NOT automatically imply smoke test impact - the dependency chain must be explicitly verified using shell scripts before recommending smoke test execution.
📚 Learning: 2025-10-27T15:30:06.412Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/network/conftest.py:348-362
Timestamp: 2025-10-27T15:30:06.412Z
Learning: In tests/network/conftest.py, the _verify_nmstate_running_pods function currently runs unconditionally in network_sanity, but rnetser plans to implement marker-based conditional checking (following the pattern of _verify_dpdk, _verify_sriov, etc.) in a future PR after adding nmstate markers to the relevant tests.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-10-08T07:16:46.347Z
Learnt from: HarshithaMS005
Repo: RedHatQE/openshift-virtualization-tests PR: 2027
File: tests/network/nmstate/test_connectivity_after_nmstate_changes.py:231-233
Timestamp: 2025-10-08T07:16:46.347Z
Learning: Network tests in the openshift-virtualization-tests repository have a sanity check in tests/network/conftest.py (around line 237) that validates len(hosts_common_available_ports) > 1 before tests run. This ensures multinic tests requiring hosts_common_available_ports[-2] won't encounter IndexError, making fixture-level guards redundant.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.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/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-08-28T12:30:40.692Z
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: tests/network/bgp/conftest.py:35-54
Timestamp: 2025-08-28T12:30:40.692Z
Learning: The BGP test suite in tests/network/bgp/ relies on session-scoped validation in tests/network/conftest.py via the network_sanity fixture's _verify_bgp() function, which validates required environment variables (VLAN_TAG, EXTERNAL_FRR_STATIC_IPV4, BGP_CLUSTER_DOMAIN_GROUP) before any BGP tests run, making individual fixture-level validation redundant.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.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/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-12-16T10:28:54.212Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T10:28:54.212Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for test execution plans, only set "Run smoke tests: True" if there is a verified, traceable dependency path from smoke tests to the changed code. Session-scoped fixtures or infrastructure-sounding changes do NOT automatically imply smoke test impact - the dependency chain must be explicitly verified using shell scripts before recommending smoke test execution.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.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/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-12-22T16:27:40.244Z
Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 3196
File: tests/network/upgrade/test_upgrade_network.py:4-4
Timestamp: 2025-12-22T16:27:40.244Z
Learning: For PRs that remove tests, rely on pytest --collect-only to verify the test discovery results (which tests are selected/deselected) and ensure the removal is clean and the test module remains functional. Full test execution is not required for test deletion PRs. This guideline applies to test files anywhere under the tests/ directory (e.g., tests/network/upgrade/test_upgrade_network.py) and should be used for similar test-deletion scenarios across the repository.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-12-16T20:11:03.645Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3062
File: conftest.py:333-333
Timestamp: 2025-12-16T20:11:03.645Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for changes affecting py_config["os_login_param"], follow this verification methodology: (1) Find all smoke tests: `rg "pytest.mark.smoke" --type=py -B2 | grep "def test_"`, (2) For each smoke test file, search for VM creation patterns: `rg "VirtualMachineForTests|running_vm|VirtualMachineForTestsFromTemplate|wait_for_ssh|check_ssh_connectivity"`, (3) Trace the dependency chain: smoke test → VirtualMachineForTests/running_vm() → wait_for_ssh_connectivity() (default enabled) → vm.login_params property → py_config["os_login_param"][vm.os_flavor], (4) Check utilities/virt.py for login_params usage: `rg "os_login_param|login_params" utilities/virt.py -C3`. Any smoke test creating VMs with default SSH connectivity checks (running_vm with check_ssh_connectivity=True) depends on os_login_param, even if the test doesn't directly reference it.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.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/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-12-16T15:09:49.597Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3062
File: conftest.py:333-333
Timestamp: 2025-12-16T15:09:49.597Z
Learning: In the openshift-virtualization-tests repository, when conftest.py or utilities/bitwarden.py changes affect py_config["os_login_param"], smoke test impact must be determined by: (1) finding all smoke tests using `rg "pytest.mark.smoke"`, (2) checking each for VM creation patterns (VirtualMachineForTests, running_vm, VirtualMachineForTestsFromTemplate), (3) tracing whether running_vm is called with default check_ssh_connectivity=True, which accesses vm.login_params property that reads py_config["os_login_param"][vm.os_flavor]. The dependency chain is: smoke test → VM creation → running_vm → wait_for_ssh_connectivity → vm.login_params → os_login_param. Any smoke test creating VMs with SSH connectivity (the default) depends on os_login_param.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-11-08T07:36:57.616Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-12-22T15:56:00.157Z
Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-22T15:56:00.157Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, do NOT use "REQUEST_CHANGES" review type if the PR author has already marked the PR as verified (e.g., with `/verified` command). Test execution plans are informational guides, not blocking requirements. Use COMMENT event for informational test plans, or only REQUEST_CHANGES if there are actual code issues that need to be addressed before merging.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-12-16T14:00:59.076Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:00:59.076Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, CodeRabbit must post ONLY an inline review comment on the Files Changed tab and then stop immediately without generating any follow-up comments in the PR discussion thread. No acknowledgment messages, no confirmation of posting, no explanation - silence after posting the inline review equals success. Additional comments create empty/meaningless reviews that clutter the PR.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.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/network/upgrade/test_upgrade_network.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/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-12-16T14:06:22.391Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:06:22.391Z
Learning: In the openshift-virtualization-tests repository, when posting test execution plan inline review comments using GitHub API, the full test execution plan content must go in the `comments[].body` field (which appears on Files Changed tab), NOT in the top-level `body` field (which appears in PR discussion thread). The top-level `body` field should be omitted or left empty to avoid posting redundant comments in the PR discussion thread.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.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/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-05-05T12:02:39.022Z
Learnt from: Anatw
Repo: RedHatQE/openshift-virtualization-tests PR: 900
File: tests/network/localnet/liblocalnet.py:25-33
Timestamp: 2025-05-05T12:02:39.022Z
Learning: The `lookup_iface_status` function in `libs/net/vmspec.py` already includes retry logic via its call to `_lookup_iface_status`, which handles polling for VM network information internally. This makes additional retry mechanisms unnecessary when retrieving VM IP addresses.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-05-05T12:02:39.022Z
Learnt from: Anatw
Repo: RedHatQE/openshift-virtualization-tests PR: 900
File: tests/network/localnet/liblocalnet.py:25-33
Timestamp: 2025-05-05T12:02:39.022Z
Learning: The `lookup_iface_status` function from libs.net.vmspec already includes retry logic via its call to _lookup_iface_status, which handles polling for VM network information internally, making additional retry mechanisms unnecessary when retrieving VM IP addresses.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-12-17T12:33:03.550Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 3072
File: tests/conftest.py:50-55
Timestamp: 2025-12-17T12:33:03.550Z
Learning: In the RedHatQE/openshift-virtualization-tests repository test environment, there is only one SR-IOV Node Network Policy (SNNP) deployed per cluster after deployment. The sriov_node_policy fixture implementation that retrieves the first policy from the namespace is intentional and correct for this single-policy environment. The team will adapt the test setup if the deployment strategy changes in the future.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.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/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-12-29T10:07:05.337Z
Learnt from: hmeir
Repo: RedHatQE/openshift-virtualization-tests PR: 3081
File: tests/install_upgrade_operators/product_install/test_install_openshift_virtualization.py:189-191
Timestamp: 2025-12-29T10:07:05.337Z
Learning: In test_ocs_virt_default_storage_class in tests/install_upgrade_operators/product_install/test_install_openshift_virtualization.py, consecutive_checks_count=3 is sufficient for verify_boot_sources_reimported because datasources update quickly in the installation lane scenario after storage class changes.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.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/network/upgrade/test_upgrade_network.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/network/upgrade/test_upgrade_network.py
📚 Learning: 2025-12-15T12:33:06.686Z
Learnt from: azhivovk
Repo: RedHatQE/openshift-virtualization-tests PR: 3024
File: tests/network/connectivity/utils.py:17-17
Timestamp: 2025-12-15T12:33:06.686Z
Learning: In the test suite, ensure the ipv6_network_data fixture returns a factory function (Callable) and that all call sites invoke it to obtain the actual data dict, i.e., use ipv6_network_data() at call sites. This enables future extensibility for configuring secondary interfaces' IP addresses without changing call sites. Apply this pattern to all Python test files under tests.

Applied to files:

  • tests/network/upgrade/test_upgrade_network.py
🧬 Code graph analysis (1)
tests/network/upgrade/test_upgrade_network.py (1)
tests/network/upgrade/utils.py (1)
  • assert_label_in_namespace (1-6)
🔇 Additional comments (1)
tests/network/upgrade/test_upgrade_network.py (1)

8-8: LGTM! Clean import statement after test removal.

The import cleanup correctly retains only assert_label_in_namespace, which is used by the remaining Kubemacpool tests. The author's test collection verification (10 selected, 1 deselected) confirms the module structure remains intact after removing the redundant network upgrade tests.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-virtualization-qe-bot

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

  • Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
  • Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
  • Issue Creation: A tracking issue is created for this PR and will be closed when the PR is merged or closed
  • Pre-commit Checks: pre-commit runs automatically if .pre-commit-config.yaml exists
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified

📋 Available Commands

PR Status Management

  • /wip - Mark PR as work in progress (adds WIP: prefix to title)
  • /wip cancel - Remove work in progress status
  • /hold - Block PR merging (approvers only)
  • /hold cancel - Unblock PR merging
  • /verified - Mark PR as verified
  • /verified cancel - Remove verification status
  • /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
  • /assign-reviewers - Assign reviewers based on OWNERS file
  • /assign-reviewer @username - Assign specific reviewer
  • /check-can-merge - Check if PR meets merge requirements

Testing & Validation

  • /retest tox - Run Python test suite with tox
  • /retest build-container - Rebuild and test container image
  • /retest all - Run all available tests

Container Operations

  • /build-and-push-container - Build and push container image (tagged with PR number)
    • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

  • /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
    • Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

  • /<label-name> - Add a label to the PR
  • /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

  1. Approval: /approve from at least one approver
  2. LGTM Count: Minimum 2 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No WIP, hold, or conflict labels
  5. Verified: PR must be marked as verified (if verification is enabled)

📊 Review Process

Approvers and Reviewers

Approvers:

  • EdDev

Reviewers:

  • Anatw
  • EdDev
  • azhivovk
  • hmeir
  • rlobillo
  • servolkov
  • yossisegev
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is automatically removed on each new commit
  • Cherry-picking: Cherry-pick labels are processed when the PR is merged
  • Container Builds: Container images are automatically tagged with the PR number
  • Permission Levels: Some commands require approver permissions
  • Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

@yossisegev
Copy link
Contributor Author

/verified

Verified by running the network/upgrade module locally, and verifying that none of the deleted tests is collected:

collected 11 items / 1 deselected / 10 selected                                                                                                                                                                                                              

<Dir openshift-virtualization-tests>
  <Package tests>
    <Package network>
      <Package upgrade>
        <Module test_upgrade_network.py>
          <Class TestUpgradeNetwork>
            Pre-upgrade tests
            <Function test_vm_have_2_interfaces_before_upgrade>
            <Function test_linux_bridge_before_upgrade>
            <Function test_kubemacpool_disabled_ns_before_upgrade>
            <Function test_kubemacpool_before_upgrade>
            <Function test_vm_connectivity_with_macspoofing_before_upgrade>
            <Function test_vm_have_2_interfaces_after_upgrade>
            <Function test_linux_bridge_after_upgrade>
            <Function test_kubemacpool_after_upgrade>
            <Function test_kubemacpool_disabled_ns_after_upgrade>
            <Function test_vm_connectivity_with_macspoofing_after_upgrade>

@openshift-virtualization-qe-bot-3
Copy link
Contributor

@coderabbitai

Test execution plan request details

CRITICAL: You MUST submit a formal pull request review using the "Request changes" review type.
This will block the PR from merging until the author addresses the test execution plan.
Use the GitHub review submission feature (not inline comments) so it triggers the pull_request_review event.
The review body should contain the Test Execution Plan with the smoke tests recommendation.

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will submit a pull request review with the test execution plan in the review body.
If you fail to run or post a comment, retry.

Analysis Requirements:

  1. Examine code changes in each modified file
  2. Identify affected code paths, functions, and classes
  3. Analyze pytest-specific elements: fixtures (scope, dependencies), parametrization, markers, conftest changes
  4. Trace test dependencies through imports, shared utilities, and fixture inheritance
  5. Detect new tests introduced in the PR
  6. Smoke Test Impact Analysis: Determine if any changes could affect smoke tests by checking:
    • Changes to files/functions used by tests marked with @pytest.mark.smoke
    • Changes to fixtures or utilities imported by smoke tests
    • Changes to conftest.py files that may affect smoke test execution
    • Changes to core infrastructure code (utilities/, libs/) that smoke tests depend on

Your deliverable:
Your change request comment will be based on the following requirements:

Test Execution Plan

  • Run smoke tests: True / False (Based on your smoke test impact analysis - True if any changes may affect smoke tests)
  • path/to/test_file.py - When the entire test file needs verification
  • path/to/test_file.py::TestClass::test_method - When specific test(s) needed
  • path/to/test_file.py::test_function - When specific test(s) needed
  • -m marker - When specific marker(s) can be used to cover multiple cases.

Guidelines:

  • Include only tests directly affected by the changes
  • Use a full file path only if ALL tests in that file require verification
  • Use file path + test name if only specific tests are needed
  • If a test marker can cover multiple files/tests, provide the marker
  • Balance coverage vs over-testing - Keep descriptions minimal

CRITICAL WORKFLOW COMPLETION RULES:

When responding to this test execution plan request, you MUST follow these rules EXACTLY:

  1. YOUR ONLY DELIVERABLE: Submit a formal pull request review with the test execution plan in the review body
  2. THEN STOP IMMEDIATELY - Do NOT generate any additional response
  3. FALLBACK ONLY: If submitting the review fails after retrying, post as a regular PR comment
  4. SILENCE = SUCCESS: After successfully submitting the review, your task is complete. No confirmation needed.

ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):

  • ❌ Do NOT post acknowledgment messages like "Test execution plan posted", "Review posted successfully", "I've successfully posted"
  • ❌ Do NOT mention review IDs, URLs, or confirmation of posting in the PR thread
  • ❌ Do NOT add any follow-up comments after submitting the review
  • ❌ Do NOT reply to confirm task completion
  • ❌ Do NOT explain what you did - just do it and stop

Remember: The pull request review is visible to users. Additional comments are redundant noise.

CRITICAL: Before setting "Run smoke tests: True", you MUST use shell scripts to:
- Trace the actual fixture dependency chain from smoke tests to changed fixtures
- Verify that smoke tests actually import/use changed utilities or functions
- Confirm the dependency path exists; do NOT assume based on scope or semantics
- Be conservative: Session-scoped fixtures or infrastructure-sounding names do NOT
automatically mean smoke test impact. Only flag smoke test impact when you can
demonstrate a concrete dependency path.
WRONG: "This session-scoped storage fixture might affect smoke tests"
RIGHT: "Smoke test X uses fixture Y, which depends on the changed fixture Z"

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test Execution Plan

Run smoke tests: False

Based on dependency analysis:

  • No smoke tests import from tests.network.upgrade module
  • Removed utility functions are not referenced elsewhere in the codebase
  • No changes to fixtures, conftest.py, or infrastructure code affecting smoke tests
  • No traceable dependency chain from smoke tests to removed code

Tests to execute:

  • tests/network/upgrade/test_upgrade_network.py - Verify remaining upgrade tests function correctly after test removals

Rationale:
This PR removes non-contributory network upgrade tests. The remaining tests in the upgrade module should be verified to ensure the module continues to function properly after the deletions.

@coderabbitai
Copy link

coderabbitai bot commented Dec 22, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Check if the removed utility functions are used anywhere else in the codebase
echo "=== Checking usage of removed utility functions ==="
rg -n "assert_bridge_and_vms_on_same_node|assert_node_is_marked_by_bridge|assert_nmstate_bridge_creation" --type=py -g '!tests/network/upgrade/utils.py'

echo -e "\n=== Checking if any smoke tests import from tests.network.upgrade ==="
# Find all smoke tests
rg -n "pytest.mark.smoke" --type=py -B2 | grep "def test_" | cut -d: -f1 | sort -u > /tmp/smoke_test_files.txt

# Check if any of these smoke test files import from tests.network.upgrade
if [ -s /tmp/smoke_test_files.txt ]; then
    echo "Smoke test files found, checking imports:"
    while read -r file; do
        if [ -f "$file" ]; then
            if rg -q "from tests\.network\.upgrade|import.*tests\.network\.upgrade" "$file"; then
                echo "FOUND: $file imports from tests.network.upgrade"
                rg -n "from tests\.network\.upgrade|import.*tests\.network\.upgrade" "$file"
            fi
        fi
    done < /tmp/smoke_test_files.txt
else
    echo "No smoke test files found with pattern search"
fi

echo -e "\n=== Checking remaining tests in test_upgrade_network.py ==="
# Verify what tests remain in the file
rg -n "def test_" tests/network/upgrade/test_upgrade_network.py | head -20

Length of output: 815


🏁 Script executed:

#!/bin/bash

# Check if verify_ovs_installed_with_annotations is used elsewhere (it was removed from imports)
echo "=== Checking usage of verify_ovs_installed_with_annotations ==="
rg -n "verify_ovs_installed_with_annotations" --type=py

echo -e "\n=== Checking if any test imports assert functions from tests.network.upgrade.utils ==="
rg -n "from tests\.network\.upgrade\.utils import|from tests\.network\.upgrade import" --type=py -C2

echo -e "\n=== Verifying conftest.py changes in network/upgrade ==="
# Check if there's a conftest.py that might have been affected
if [ -f "tests/network/upgrade/conftest.py" ]; then
    echo "Found conftest.py, checking for removed fixture references:"
    rg -n "bridge_on_one_node|upgrade_bridge_marker_nad|ovs_daemonset|hyperconverged_ovs_annotations" tests/network/upgrade/conftest.py
else
    echo "No conftest.py found in tests/network/upgrade/"
fi

Length of output: 913


🏁 Script executed:

#!/bin/bash

# Check which fixtures were removed/remain and their usage
echo "=== Checking fixtures in network/upgrade/conftest.py ==="
if [ -f "tests/network/upgrade/conftest.py" ]; then
    rg -n "@pytest.fixture" tests/network/upgrade/conftest.py -A1
fi

echo -e "\n=== Checking if removed test fixtures are still referenced ==="
# Fixtures that might have been used by removed tests
rg -n "bridge_on_one_node|upgrade_bridge_marker_nad|ovs_daemonset|hyperconverged_ovs_annotations_fetched" tests/network/upgrade/ --type=py

echo -e "\n=== Verify smoke tests pattern search ==="
# Try a different approach to find smoke tests
rg -l "@pytest.mark.smoke|pytest.mark.smoke" --type=py | head -10

echo -e "\n=== Check if any tests in network/upgrade module affect other network tests ==="
# Check if network upgrade tests are imported/referenced elsewhere
rg -n "from tests\.network\.upgrade\.test_upgrade_network import|test_upgrade_network" --type=py -g '!tests/network/upgrade/test_upgrade_network.py'

Length of output: 2450


🏁 Script executed:

#!/bin/bash

# Let's check the actual current state of conftest.py to see what fixtures exist
echo "=== Full fixture list in network/upgrade/conftest.py ==="
if [ -f "tests/network/upgrade/conftest.py" ]; then
    rg -n "^def " tests/network/upgrade/conftest.py | grep -v "    def "
fi

echo -e "\n=== Check git diff to see what was actually changed in conftest.py ==="
git diff HEAD tests/network/upgrade/conftest.py | head -100

echo -e "\n=== Check smoke test files for any network.upgrade imports ==="
for file in tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py \
            tests/install_upgrade_operators/csv/test_csv.py \
            tests/storage/cdi_upload/test_upload_virtctl.py \
            tests/virt/node/general/test_container_disk_vm.py; do
    if [ -f "$file" ]; then
        echo "Checking $file:"
        rg -n "^import |^from " "$file" | grep -i "network\|upgrade" || echo "  No network/upgrade imports"
    fi
done

echo -e "\n=== Verify network upgrade module isolation ==="
# Check if any other test modules depend on network upgrade tests
rg -n "tests\.network\.upgrade" --type=py -g '!tests/network/upgrade/*' | head -20

Length of output: 1134


🏁 Script executed:

#!/bin/bash

# Submit PR review with test execution plan using GitHub CLI
gh api \
  --method POST \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  /repos/RedHatQE/openshift-virtualization-tests/pulls/3196/reviews \
  -f event='REQUEST_CHANGES' \
  -f body='## Test Execution Plan

**Run smoke tests: False**

Based on dependency analysis:
- No smoke tests import from `tests.network.upgrade` module
- Removed utility functions are not referenced elsewhere in the codebase
- No changes to fixtures, conftest.py, or infrastructure code affecting smoke tests
- No traceable dependency chain from smoke tests to removed code

**Tests to execute:**

- `tests/network/upgrade/test_upgrade_network.py` - Verify remaining upgrade tests function correctly after test removals

**Rationale:**
This PR removes non-contributory network upgrade tests. The remaining tests in the upgrade module should be verified to ensure the module continues to function properly after the deletions.'

Length of output: 2466



🧠 Learnings used
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T10:28:54.212Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for test execution plans, only set "Run smoke tests: True" if there is a verified, traceable dependency path from smoke tests to the changed code. Session-scoped fixtures or infrastructure-sounding changes do NOT automatically imply smoke test impact - the dependency chain must be explicitly verified using shell scripts before recommending smoke test execution.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3062
File: conftest.py:333-333
Timestamp: 2025-12-16T20:11:03.645Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for changes affecting py_config["os_login_param"], follow this verification methodology: (1) Find all smoke tests: `rg "pytest.mark.smoke" --type=py -B2 | grep "def test_"`, (2) For each smoke test file, search for VM creation patterns: `rg "VirtualMachineForTests|running_vm|VirtualMachineForTestsFromTemplate|wait_for_ssh|check_ssh_connectivity"`, (3) Trace the dependency chain: smoke test → VirtualMachineForTests/running_vm() → wait_for_ssh_connectivity() (default enabled) → vm.login_params property → py_config["os_login_param"][vm.os_flavor], (4) Check utilities/virt.py for login_params usage: `rg "os_login_param|login_params" utilities/virt.py -C3`. Any smoke test creating VMs with default SSH connectivity checks (running_vm with check_ssh_connectivity=True) depends on os_login_param, even if the test doesn't directly reference it.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:00:59.076Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, CodeRabbit must post ONLY an inline review comment on the Files Changed tab and then stop immediately without generating any follow-up comments in the PR discussion thread. No acknowledgment messages, no confirmation of posting, no explanation - silence after posting the inline review equals success. Additional comments create empty/meaningless reviews that clutter the PR.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3062
File: conftest.py:333-333
Timestamp: 2025-12-16T15:09:49.597Z
Learning: In the openshift-virtualization-tests repository, when conftest.py or utilities/bitwarden.py changes affect py_config["os_login_param"], smoke test impact must be determined by: (1) finding all smoke tests using `rg "pytest.mark.smoke"`, (2) checking each for VM creation patterns (VirtualMachineForTests, running_vm, VirtualMachineForTestsFromTemplate), (3) tracing whether running_vm is called with default check_ssh_connectivity=True, which accesses vm.login_params property that reads py_config["os_login_param"][vm.os_flavor]. The dependency chain is: smoke test → VM creation → running_vm → wait_for_ssh_connectivity → vm.login_params → os_login_param. Any smoke test creating VMs with SSH connectivity (the default) depends on os_login_param.

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: 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: 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: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/network/conftest.py:348-362
Timestamp: 2025-10-27T15:30:06.412Z
Learning: In tests/network/conftest.py, the _verify_nmstate_running_pods function currently runs unconditionally in network_sanity, but rnetser plans to implement marker-based conditional checking (following the pattern of _verify_dpdk, _verify_sriov, etc.) in a future PR after adding nmstate markers to the relevant tests.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:06:22.391Z
Learning: In the openshift-virtualization-tests repository, when posting test execution plan inline review comments using GitHub API, the full test execution plan content must go in the `comments[].body` field (which appears on Files Changed tab), NOT in the top-level `body` field (which appears in PR discussion thread). The top-level `body` field should be omitted or left empty to avoid posting redundant comments in the PR discussion thread.

Learnt from: 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: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: libs/net/node_network.py:25-31
Timestamp: 2025-08-20T23:43:28.117Z
Learning: In the RedHatQE/openshift-virtualization-tests project, servolkov's team always uses bare metal (BM) clusters with IPv4 setup in their testing environment, making defensive checks for IPv4 data presence potentially redundant in their networking code.

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: 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.

Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1411
File: utilities/os_utils.py:246-279
Timestamp: 2025-07-22T17:13:59.166Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, CentOS preferences follow the format "centos-stream<version>" (e.g., "centos-stream9", "centos-stream10"). The generate_instance_type_centos_os_matrix function correctly uses regex to extract numeric versions and constructs the latest version string in the same format as the input preferences for proper comparison.

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.

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.

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: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2701
File: .github/scripts/coderabbit_workflow.py:45-46
Timestamp: 2025-11-27T11:45:54.224Z
Learning: In the openshift-virtualization-tests repository, when using the PyGithub library or similar API clients, rnetser prefers to rely on API-level validation rather than adding redundant client-side input validation, since the API will handle validation and raise appropriate errors.

Learnt from: HarshithaMS005
Repo: RedHatQE/openshift-virtualization-tests PR: 2027
File: tests/network/nmstate/test_connectivity_after_nmstate_changes.py:231-233
Timestamp: 2025-10-08T07:16:46.347Z
Learning: Network tests in the openshift-virtualization-tests repository have a sanity check in tests/network/conftest.py (around line 237) that validates len(hosts_common_available_ports) > 1 before tests run. This ensures multinic tests requiring hosts_common_available_ports[-2] won't encounter IndexError, making fixture-level guards redundant.

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.

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: 3072
File: tests/conftest.py:50-55
Timestamp: 2025-12-17T12:33:03.550Z
Learning: In the RedHatQE/openshift-virtualization-tests repository test environment, there is only one SR-IOV Node Network Policy (SNNP) deployed per cluster after deployment. The sriov_node_policy fixture implementation that retrieves the first policy from the namespace is intentional and correct for this single-policy environment. The team will adapt the test setup if the deployment strategy changes in the future.

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: 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.

@yossisegev
Copy link
Contributor Author

/verified

Verified by running the network/upgrade module locally, and verifying that none of the deleted tests is collected:

collected 11 items / 1 deselected / 10 selected                                                                                                                                                                                                              

<Dir openshift-virtualization-tests>
  <Package tests>
    <Package network>
      <Package upgrade>
        <Module test_upgrade_network.py>
          <Class TestUpgradeNetwork>
            Pre-upgrade tests
            <Function test_vm_have_2_interfaces_before_upgrade>
            <Function test_linux_bridge_before_upgrade>
            <Function test_kubemacpool_disabled_ns_before_upgrade>
            <Function test_kubemacpool_before_upgrade>
            <Function test_vm_connectivity_with_macspoofing_before_upgrade>
            <Function test_vm_have_2_interfaces_after_upgrade>
            <Function test_linux_bridge_after_upgrade>
            <Function test_kubemacpool_after_upgrade>
            <Function test_kubemacpool_disabled_ns_after_upgrade>
            <Function test_vm_connectivity_with_macspoofing_after_upgrade>

@openshift-virtualization-qe-bot-3
Copy link
Contributor

@coderabbitai

Test execution plan request details

CRITICAL: You MUST submit a formal pull request review using the "Request changes" review type.
This will block the PR from merging until the author addresses the test execution plan.
Use the GitHub review submission feature (not inline comments) so it triggers the pull_request_review event.
The review body should contain the Test Execution Plan with the smoke tests recommendation.

As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan.
You will submit a pull request review with the test execution plan in the review body.
If you fail to run or post a comment, retry.

Analysis Requirements:

  1. Examine code changes in each modified file
  2. Identify affected code paths, functions, and classes
  3. Analyze pytest-specific elements: fixtures (scope, dependencies), parametrization, markers, conftest changes
  4. Trace test dependencies through imports, shared utilities, and fixture inheritance
  5. Detect new tests introduced in the PR
  6. Smoke Test Impact Analysis: Determine if any changes could affect smoke tests by checking:
    • Changes to files/functions used by tests marked with @pytest.mark.smoke
    • Changes to fixtures or utilities imported by smoke tests
    • Changes to conftest.py files that may affect smoke test execution
    • Changes to core infrastructure code (utilities/, libs/) that smoke tests depend on

Your deliverable:
Your change request comment will be based on the following requirements:

Test Execution Plan

  • Run smoke tests: True / False (Based on your smoke test impact analysis - True if any changes may affect smoke tests)
  • path/to/test_file.py - When the entire test file needs verification
  • path/to/test_file.py::TestClass::test_method - When specific test(s) needed
  • path/to/test_file.py::test_function - When specific test(s) needed
  • -m marker - When specific marker(s) can be used to cover multiple cases.

Guidelines:

  • Include only tests directly affected by the changes
  • Use a full file path only if ALL tests in that file require verification
  • Use file path + test name if only specific tests are needed
  • If a test marker can cover multiple files/tests, provide the marker
  • Balance coverage vs over-testing - Keep descriptions minimal

CRITICAL WORKFLOW COMPLETION RULES:

When responding to this test execution plan request, you MUST follow these rules EXACTLY:

  1. YOUR ONLY DELIVERABLE: Submit a formal pull request review with the test execution plan in the review body
  2. THEN STOP IMMEDIATELY - Do NOT generate any additional response
  3. FALLBACK ONLY: If submitting the review fails after retrying, post as a regular PR comment
  4. SILENCE = SUCCESS: After successfully submitting the review, your task is complete. No confirmation needed.

ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):

  • ❌ Do NOT post acknowledgment messages like "Test execution plan posted", "Review posted successfully", "I've successfully posted"
  • ❌ Do NOT mention review IDs, URLs, or confirmation of posting in the PR thread
  • ❌ Do NOT add any follow-up comments after submitting the review
  • ❌ Do NOT reply to confirm task completion
  • ❌ Do NOT explain what you did - just do it and stop

Remember: The pull request review is visible to users. Additional comments are redundant noise.

CRITICAL: Before setting "Run smoke tests: True", you MUST use shell scripts to:
- Trace the actual fixture dependency chain from smoke tests to changed fixtures
- Verify that smoke tests actually import/use changed utilities or functions
- Confirm the dependency path exists; do NOT assume based on scope or semantics
- Be conservative: Session-scoped fixtures or infrastructure-sounding names do NOT
automatically mean smoke test impact. Only flag smoke test impact when you can
demonstrate a concrete dependency path.
WRONG: "This session-scoped storage fixture might affect smoke tests"
RIGHT: "Smoke test X uses fixture Y, which depends on the changed fixture Z"

@coderabbitai
Copy link

coderabbitai bot commented Dec 29, 2025

Test Execution Plan

Run smoke tests: False

Analysis:

  • No smoke tests import from tests.network.upgrade module (verified via dependency tracing)
  • Removed utility functions are not referenced elsewhere in the codebase
  • No changes to fixtures, conftest.py, or infrastructure code
  • No traceable dependency chain from smoke tests to removed code
  • PR removes only test cases and their unused helper functions

Tests to execute:

  • tests/network/upgrade/test_upgrade_network.py - Verify all 10 remaining upgrade tests function correctly after deletions

Rationale:
This PR removes 3 redundant network upgrade tests (test_nmstate_bridge_before_upgrade, test_bridge_marker_before_upgrade, test_install_ovs_with_annotations_before_upgrade) and their post-upgrade counterparts, along with 3 unused utility functions. The remaining tests cover VM interfaces, Linux bridge, kubemacpool, and macspoof connectivity scenarios and should be verified to ensure the module continues to function properly.

@coderabbitai
Copy link

coderabbitai bot commented Dec 29, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# 1. Verify smoke test impact - check if any smoke tests import from network.upgrade
echo "=== Finding all smoke tests ==="
rg -l "pytest.mark.smoke" --type=py | head -20 > /tmp/smoke_files.txt
cat /tmp/smoke_files.txt

echo -e "\n=== Checking if smoke tests import from tests.network.upgrade ==="
while read -r file; do
    if [ -f "$file" ]; then
        if rg -q "from tests\.network\.upgrade|import.*tests\.network\.upgrade" "$file" 2>/dev/null; then
            echo "FOUND: $file imports from tests.network.upgrade"
            rg -n "from tests\.network\.upgrade|import.*tests\.network\.upgrade" "$file"
        fi
    fi
done < /tmp/smoke_files.txt
if [ $? -eq 0 ]; then
    echo "No smoke tests import from tests.network.upgrade"
fi

echo -e "\n=== Verifying removed functions are not used elsewhere ==="
rg -n "assert_bridge_and_vms_on_same_node|assert_node_is_marked_by_bridge|assert_nmstate_bridge_creation" --type=py -g '!tests/network/upgrade/utils.py' || echo "Not used elsewhere"

echo -e "\n=== Get PR diff to identify first file and line ==="
gh pr diff 3196 --patch | head -100

echo -e "\n=== Get commit SHA for inline comment ==="
COMMIT_SHA=$(gh pr view 3196 --json commits --jq '.commits[-1].oid')
echo "Latest commit SHA: $COMMIT_SHA"

echo -e "\n=== Get changed files list ==="
gh pr view 3196 --json files --jq '.files[] | .path'

Length of output: 5209


🏁 Script executed:

#!/bin/bash

# Post inline review comment with Test Execution Plan
COMMIT_SHA=$(gh pr view 3196 --json commits --jq '.commits[-1].oid')

# Create the TEP comment payload
cat > /tmp/tep_comment.json <<'EOF'
{
  "body": "## Test Execution Plan\n\n**Run smoke tests: False**\n\n**Analysis:**\n- No smoke tests import from `tests.network.upgrade` module (verified via dependency tracing)\n- Removed utility functions are not referenced elsewhere in the codebase\n- No changes to fixtures, conftest.py, or infrastructure code\n- No traceable dependency chain from smoke tests to removed code\n- PR removes only test cases and their unused helper functions\n\n**Tests to execute:**\n\n- `tests/network/upgrade/test_upgrade_network.py` - Verify all 10 remaining upgrade tests function correctly after deletions\n\n**Rationale:**\nThis PR removes 3 redundant network upgrade tests (`test_nmstate_bridge_before_upgrade`, `test_bridge_marker_before_upgrade`, `test_install_ovs_with_annotations_before_upgrade`) and their post-upgrade counterparts, along with 3 unused utility functions. The remaining tests cover VM interfaces, Linux bridge, kubemacpool, and macspoof connectivity scenarios and should be verified to ensure the module continues to function properly.",
  "commit_id": "",
  "path": "tests/network/upgrade/test_upgrade_network.py",
  "line": 1
}
EOF

# Add commit SHA
jq --arg sha "$COMMIT_SHA" '.commit_id = $sha' /tmp/tep_comment.json > /tmp/tep_final.json

echo "Posting inline review comment..."
gh api \
  --method POST \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  /repos/RedHatQE/openshift-virtualization-tests/pulls/3196/comments \
  --input /tmp/tep_final.json

echo "Inline comment posted with exit code: $?"

Length of output: 403


🏁 Script executed:

#!/bin/bash

# The inline comment API requires position in diff, not line number
# Let's get the diff and calculate position, or fall back to regular comment

echo "=== Attempting to post as regular PR comment (fallback) ==="
gh pr comment 3196 --body "## Test Execution Plan

**Run smoke tests: False**

**Analysis:**
- No smoke tests import from \`tests.network.upgrade\` module (verified via dependency tracing)
- Removed utility functions are not referenced elsewhere in the codebase
- No changes to fixtures, conftest.py, or infrastructure code
- No traceable dependency chain from smoke tests to removed code
- PR removes only test cases and their unused helper functions

**Tests to execute:**

- \`tests/network/upgrade/test_upgrade_network.py\` - Verify all 10 remaining upgrade tests function correctly after deletions

**Rationale:**
This PR removes 3 redundant network upgrade tests (\`test_nmstate_bridge_before_upgrade\`, \`test_bridge_marker_before_upgrade\`, \`test_install_ovs_with_annotations_before_upgrade\`) and their post-upgrade counterparts, along with 3 unused utility functions. The remaining tests cover VM interfaces, Linux bridge, kubemacpool, and macspoof connectivity scenarios and should be verified to ensure the module continues to function properly."

echo "Comment posted with exit code: $?"

Length of output: 185



🧠 Learnings used
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.

Learnt from: jpeimer
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-22T15:56:00.157Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, do NOT use "REQUEST_CHANGES" review type if the PR author has already marked the PR as verified (e.g., with `/verified` command). Test execution plans are informational guides, not blocking requirements. Use COMMENT event for informational test plans, or only REQUEST_CHANGES if there are actual code issues that need to be addressed before merging.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:00:59.076Z
Learning: In the openshift-virtualization-tests repository, when responding to test execution plan requests from openshift-virtualization-qe-bot-3, CodeRabbit must post ONLY an inline review comment on the Files Changed tab and then stop immediately without generating any follow-up comments in the PR discussion thread. No acknowledgment messages, no confirmation of posting, no explanation - silence after posting the inline review equals success. Additional comments create empty/meaningless reviews that clutter the PR.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 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: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3062
File: conftest.py:333-333
Timestamp: 2025-12-16T20:11:03.645Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for changes affecting py_config["os_login_param"], follow this verification methodology: (1) Find all smoke tests: `rg "pytest.mark.smoke" --type=py -B2 | grep "def test_"`, (2) For each smoke test file, search for VM creation patterns: `rg "VirtualMachineForTests|running_vm|VirtualMachineForTestsFromTemplate|wait_for_ssh|check_ssh_connectivity"`, (3) Trace the dependency chain: smoke test → VirtualMachineForTests/running_vm() → wait_for_ssh_connectivity() (default enabled) → vm.login_params property → py_config["os_login_param"][vm.os_flavor], (4) Check utilities/virt.py for login_params usage: `rg "os_login_param|login_params" utilities/virt.py -C3`. Any smoke test creating VMs with default SSH connectivity checks (running_vm with check_ssh_connectivity=True) depends on os_login_param, even if the test doesn't directly reference it.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T10:28:54.212Z
Learning: In the openshift-virtualization-tests repository, when determining smoke test impact for test execution plans, only set "Run smoke tests: True" if there is a verified, traceable dependency path from smoke tests to the changed code. Session-scoped fixtures or infrastructure-sounding changes do NOT automatically imply smoke test impact - the dependency chain must be explicitly verified using shell scripts before recommending smoke test execution.

Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 3062
File: conftest.py:333-333
Timestamp: 2025-12-16T15:09:49.597Z
Learning: In the openshift-virtualization-tests repository, when conftest.py or utilities/bitwarden.py changes affect py_config["os_login_param"], smoke test impact must be determined by: (1) finding all smoke tests using `rg "pytest.mark.smoke"`, (2) checking each for VM creation patterns (VirtualMachineForTests, running_vm, VirtualMachineForTestsFromTemplate), (3) tracing whether running_vm is called with default check_ssh_connectivity=True, which accesses vm.login_params property that reads py_config["os_login_param"][vm.os_flavor]. The dependency chain is: smoke test → VM creation → running_vm → wait_for_ssh_connectivity → vm.login_params → os_login_param. Any smoke test creating VMs with SSH connectivity (the default) depends on os_login_param.

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: 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: 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: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-12-16T14:06:22.391Z
Learning: In the openshift-virtualization-tests repository, when posting test execution plan inline review comments using GitHub API, the full test execution plan content must go in the `comments[].body` field (which appears on Files Changed tab), NOT in the top-level `body` field (which appears in PR discussion thread). The top-level `body` field should be omitted or left empty to avoid posting redundant comments in the PR discussion thread.

Learnt from: yossisegev
Repo: RedHatQE/openshift-virtualization-tests PR: 3196
File: tests/network/upgrade/test_upgrade_network.py:4-4
Timestamp: 2025-12-22T16:27:44.327Z
Learning: For PRs that remove test cases (especially redundant test cleanup PRs in openshift-virtualization-tests), test collection verification (pytest --collect-only showing selected/deselected counts) is sufficient to confirm the removal was clean and the test module remains functional. Full test execution is not required for test deletion PRs.

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: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1904
File: tests/network/conftest.py:348-362
Timestamp: 2025-10-27T15:30:06.412Z
Learning: In tests/network/conftest.py, the _verify_nmstate_running_pods function currently runs unconditionally in network_sanity, but rnetser plans to implement marker-based conditional checking (following the pattern of _verify_dpdk, _verify_sriov, etc.) in a future PR after adding nmstate markers to the relevant tests.

Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: libs/net/node_network.py:25-31
Timestamp: 2025-08-20T23:43:28.117Z
Learning: In the RedHatQE/openshift-virtualization-tests project, servolkov's team always uses bare metal (BM) clusters with IPv4 setup in their testing environment, making defensive checks for IPv4 data presence potentially redundant in their networking code.

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: 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.

Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1411
File: utilities/os_utils.py:246-279
Timestamp: 2025-07-22T17:13:59.166Z
Learning: In the RedHatQE/openshift-virtualization-tests repository, CentOS preferences follow the format "centos-stream<version>" (e.g., "centos-stream9", "centos-stream10"). The generate_instance_type_centos_os_matrix function correctly uses regex to extract numeric versions and constructs the latest version string in the same format as the input preferences for proper comparison.

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.

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: 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: 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.

Copy link
Contributor

@azhivovk azhivovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Copy link
Contributor

@EdDev EdDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@rnetser
Copy link
Collaborator

rnetser commented Dec 29, 2025

/approve
/lgtm

@rnetser rnetser merged commit ed4809e into RedHatQE:main Dec 29, 2025
23 checks passed
@openshift-virtualization-qe-bot-3
Copy link
Contributor

New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published

@yossisegev
Copy link
Contributor Author

/cherry-pick cnv-4.20 cnv-4.19 cnv-4.18

@openshift-virtualization-qe-bot

Manual cherry-pick is needed
Cherry pick failed for ed4809e to cnv-4.18:
To cherry-pick run:

git remote update
git checkout cnv-4.18
git pull origin cnv-4.18
git checkout -b rm-redund-net-upg-cnv-4.18
git cherry-pick ed4809e81d6d2de9daae744bed4ef5638a83f7fc
git push origin rm-redund-net-upg-cnv-4.18

@openshift-virtualization-qe-bot

Manual cherry-pick is needed
Cherry pick failed for ed4809e to cnv-4.20:
To cherry-pick run:

git remote update
git checkout cnv-4.20
git pull origin cnv-4.20
git checkout -b rm-redund-net-upg-cnv-4.20
git cherry-pick ed4809e81d6d2de9daae744bed4ef5638a83f7fc
git push origin rm-redund-net-upg-cnv-4.20

@openshift-virtualization-qe-bot

Manual cherry-pick is needed
Cherry pick failed for ed4809e to cnv-4.19:
To cherry-pick run:

git remote update
git checkout cnv-4.19
git pull origin cnv-4.19
git checkout -b rm-redund-net-upg-cnv-4.19
git cherry-pick ed4809e81d6d2de9daae744bed4ef5638a83f7fc
git push origin rm-redund-net-upg-cnv-4.19

rnetser pushed a commit that referenced this pull request Jan 5, 2026
Removing 3 network upgrade tests that are not contributing anything. The
dependent fixtures and functions were also removed, except for those
that are still used by other flows.
Polarion test cases deleted as well.

The rationale of removing each of these tests:
- test_nmstate_bridge_before_upgrade: Needless to explain this is a pure
nmstate flow, that doesn't involve CNV.
- test_bridge_marker_before_upgrade: Bridge-marker is indirectly covered
in any bridge test, which would fail if the bridge-marker is
disfunctioning.
- test_install_ovs_with_annotations_before_upgrade: Actively opting-in
OVS CNI is not needed on OVN-k8s clusters, which is now the default.

Backported from
#3196

##### jira-ticket:
https://issues.redhat.com/browse/CNV-70147
openshift-virtualization-qe-bot-5 pushed a commit that referenced this pull request Jan 5, 2026
Removing 3 network upgrade tests that are not contributing anything. The
dependent fixtures and functions were also removed, except for those
that are still used by other flows.
Polarion test cases deleted as well.

The rationale of removing each of these tests:
- test_nmstate_bridge_before_upgrade: Needless to explain this is a pure
nmstate flow, that doesn't involve CNV.
- test_bridge_marker_before_upgrade: Bridge-marker is indirectly covered
in any bridge test, which would fail if the bridge-marker is
disfunctioning.
- test_install_ovs_with_annotations_before_upgrade: Actively opting-in
OVS CNI is not needed on OVN-k8s clusters, which is now the default.

Backported from
#3196

##### jira-ticket:
https://issues.redhat.com/browse/CNV-70147
openshift-virtualization-qe-bot-5 pushed a commit that referenced this pull request Jan 5, 2026
Removing 3 network upgrade tests that are not contributing anything. The
dependent fixtures and functions were also removed, except for those
that are still used by other flows.
Polarion test cases deleted as well.

The rationale of removing each of these tests:
- test_nmstate_bridge_before_upgrade: Needless to explain this is a pure
nmstate flow, that doesn't involve CNV.
- test_bridge_marker_before_upgrade: Bridge-marker is indirectly covered
in any bridge test, which would fail if the bridge-marker is
disfunctioning.
- test_install_ovs_with_annotations_before_upgrade: Actively opting-in
OVS CNI is not needed on OVN-k8s clusters, which is now the default.

Backported from
#3196

##### jira-ticket:
https://issues.redhat.com/browse/CNV-70147
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.