use cleanup mechs on TestNetworkMigration #11945
Conversation
boring-cyborg
bot
added
component:integration-test
Python
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the test cleanup mechanism in test_migration.py to leverage the parent class's cleanup functionality. The changes move from a custom cleanup implementation to using the standard super() pattern inherited from cloudstackTestCase.
Key changes:
- Appending resources to cleanup lists immediately after creation instead of in bulk at the end
- Delegating cleanup to parent class methods via
super()calls - Initializing
self.cleanuplist at the beginning ofsetUp()method
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| zoneid=self.zone.id | ||
| ) | ||
| self.cleanup.append(deployVmResponse) | ||
|
|
There was a problem hiding this comment.
[nitpick] There's an extra blank line added after appending deployVmResponse to the cleanup list (line 215), but no blank line after other similar append operations (e.g., lines 194, 255, 271). For consistency, either remove this blank line or add blank lines after all similar operations throughout the file.
| domainid=self.account.domainid | ||
| ) | ||
| self.cleanup.append(vpc) | ||
|
|
There was a problem hiding this comment.
[nitpick] Similar to the previous comment, there's an extra blank line after appending vpc to the cleanup list. This inconsistency in spacing should be addressed to maintain uniform code style throughout the file.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #11945 +/- ##
=========================================
Coverage 16.18% 16.19%
- Complexity 13305 13307 +2
=========================================
Files 5657 5657
Lines 498466 498480 +14
Branches 60491 60494 +3
=========================================
+ Hits 80696 80704 +8
- Misses 408789 408795 +6
Partials 8981 8981
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15594 |
|
unfortunately |
The problem seems to be not in the test but in vlan allocation leakage in the system. This can be merged but the left over from the test will remain, the scenario is valid. |
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15619 |
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15621 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
As this is purely (test)cleanup, I don’t think this needs further testing , agree @vishesh92 @sudo87 @Pearl1594 ? |
Yes. Just need to wait for test results to ensure that teardown gets executed successfully. |
|
[SF] Trillian test result (tid-14767)
|
I don’t think there is much relevance in this, rerunning. |
|
[SF] Trillian test result (tid-14772)
|
4 participants
Description
This PR…
Fixes: #9026 , but shows that there is a genuine issue in the vlan allocation management in the system.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?