CherryPicked: [cnv-4.18] [4.20] [upgrade/network] Remove redundant network upgrade tests #3322
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
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
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ 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 |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
approved these changes
Jan 5, 2026
|
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
requested review from
Anatw,
hmeir,
servolkov and
yossisegev
Contributor
|
/verified Verified by running the network/upgrade module locally, and verifying that none of the deleted tests is collected: |
Contributor
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Your deliverable: Test Execution Plan
Guidelines:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL: Before setting "Run smoke tests: True", you MUST use shell scripts to: |
Contributor
|
/retest build-container |
Contributor
|
/retest tox |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| import logging | ||
| import os | ||
| from ipaddress import ip_interface | ||
|
|
There was a problem hiding this comment.
Test Execution Plan
- Run smoke tests: False
Rationale:
- This PR removes 6 redundant upgrade tests and their supporting utility functions
- No smoke tests exist in or depend on the network/upgrade module
- The deleted utility functions are not used elsewhere in the codebase
- Changes are isolated to network/upgrade test cleanup
Recommended Tests:
tests/network/upgrade/test_upgrade_network.py- Verify remaining 10 tests execute successfully after deletion
Note: User yossisegev has already verified test collection locally (10/11 tests collected, 1 deselected as expected).
Contributor
There was a problem hiding this comment.
@coderabbitai The verification I ran is enough, as this PR is about removing tests, so the verificaition was that these tests are not collected.
There was a problem hiding this comment.
@yossisegev Understood! Test collection verification confirming the deleted tests are not collected is indeed sufficient for this test removal PR. This aligns with the principle we've established for test cleanup PRs in this repository. Thank you for the confirmation! ✓
🧠 Learnings used
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: 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: 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-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: 3228
File: .coderabbit.yaml:30-41
Timestamp: 2026-01-05T10:33:50.728Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers minimal pre-merge checks in CodeRabbit configuration: only docstrings enforcement (80% threshold) is needed, not title or description checks.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8013 🏁 Script executed: Length of output: 76 🏁 Script executed: Length of output: 49 🏁 Script executed: Length of output: 180 🏁 Script executed: Length of output: 575 🏁 Script executed: Length of output: 120 🏁 Script executed: Length of output: 1426 🏁 Script executed: Length of output: 329 🏁 Script executed: Length of output: 40 🏁 Script executed: Length of output: 129 🏁 Script executed: Length of output: 3137 🧠 Learnings used |
azhivovk
approved these changes
Jan 5, 2026
Collaborator
|
/approve |
servolkov
approved these changes
Jan 8, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
cherry-pick #3299 into cnv-4.18
requested-by yossisegev