use cleanup mechs on TestNetworkMigration#11945
use cleanup mechs on TestNetworkMigration#11945DaanHoogland wants to merge 5 commits intoapache:4.20from
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.23% 17.09% +0.86%
Complexity 13377 13377
============================================
Files 5657 5255 -402
Lines 498865 466189 -32676
Branches 60545 54728 -5817
============================================
- Hits 80991 79703 -1288
+ Misses 408843 377608 -31235
+ Partials 9031 8878 -153
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:
|
|
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. |
|
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. |
I don’t think there is much relevance in this, rerunning. |
DaanHoogland
force-pushed
the
ghi9026-testNetworkMigrationCleanup
branch
from
a080eb8 to
74235ec
Compare
|
@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 15755 |
DaanHoogland
force-pushed
the
ghi9026-testNetworkMigrationCleanup
branch
from
74235ec to
f7fdaf8
Compare
DaanHoogland
force-pushed
the
ghi9026-testNetworkMigrationCleanup
branch
from
f7fdaf8 to
ec25c0f
Compare
|
@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 16143 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-15055)
|
|
maybe retry later |
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?