-
Notifications
You must be signed in to change notification settings - Fork 52
[upgrade/network] Remove redundant network upgrade tests #3196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughRemoved 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
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧠 Learnings (27)📓 Common learnings📚 Learning: 2025-10-27T15:30:06.412ZApplied to files:
📚 Learning: 2025-10-08T07:16:46.347ZApplied to files:
📚 Learning: 2025-09-29T19:05:24.987ZApplied to files:
📚 Learning: 2025-08-28T12:30:40.692ZApplied to files:
📚 Learning: 2025-05-28T10:50:56.122ZApplied to files:
📚 Learning: 2025-12-16T10:28:54.212ZApplied to files:
📚 Learning: 2025-06-13T01:08:18.579ZApplied to files:
📚 Learning: 2025-12-22T16:27:40.244ZApplied to files:
📚 Learning: 2025-12-16T20:11:03.645ZApplied to files:
📚 Learning: 2025-09-29T19:05:24.987ZApplied to files:
📚 Learning: 2025-12-16T15:09:49.597ZApplied to files:
📚 Learning: 2025-11-08T07:36:57.616ZApplied to files:
📚 Learning: 2025-12-22T15:56:00.157ZApplied to files:
📚 Learning: 2025-12-16T14:00:59.076ZApplied to files:
📚 Learning: 2025-09-15T06:49:53.478ZApplied to files:
📚 Learning: 2025-08-09T01:52:26.683ZApplied to files:
📚 Learning: 2025-12-16T14:06:22.391ZApplied to files:
📚 Learning: 2025-12-07T13:52:56.070ZApplied to files:
📚 Learning: 2025-05-05T12:02:39.022ZApplied to files:
📚 Learning: 2025-05-05T12:02:39.022ZApplied to files:
📚 Learning: 2025-12-17T12:33:03.550ZApplied to files:
📚 Learning: 2025-09-09T11:51:37.860ZApplied to files:
📚 Learning: 2025-12-29T10:07:05.337ZApplied to files:
📚 Learning: 2025-08-20T23:57:48.380ZApplied to files:
📚 Learning: 2025-06-18T09:31:06.311ZApplied to files:
📚 Learning: 2025-12-15T12:33:06.686ZApplied to files:
🧬 Code graph analysis (1)tests/network/upgrade/test_upgrade_network.py (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
|
/verified Verified by running the network/upgrade module locally, and verifying that none of the deleted tests is collected: |
Test execution plan request detailsCRITICAL: You MUST submit a formal pull request review using the "Request changes" review type. 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: |
There was a problem hiding this 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.upgrademodule - 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.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 815 🏁 Script executed: Length of output: 913 🏁 Script executed: Length of output: 2450 🏁 Script executed: Length of output: 1134 🏁 Script executed: Length of output: 2466 🧠 Learnings used |
32a78e2 to
adc289d
Compare
|
/verified Verified by running the network/upgrade module locally, and verifying that none of the deleted tests is collected: |
Test execution plan request detailsCRITICAL: You MUST submit a formal pull request review using the "Request changes" review type. 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: |
Test Execution PlanRun smoke tests: False Analysis:
Tests to execute:
Rationale: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5209 🏁 Script executed: Length of output: 403 🏁 Script executed: Length of output: 185 🧠 Learnings used |
azhivovk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
EdDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
|
/approve |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
|
/cherry-pick cnv-4.20 cnv-4.19 cnv-4.18 |
|
Manual cherry-pick is needed |
|
Manual cherry-pick is needed |
|
Manual cherry-pick is needed |
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
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
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
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:
jira-ticket:
https://issues.redhat.com/browse/CNV-70147
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.