Skip to content

test: cleanup resources in test_deploy_vm_iso, use base class tearDown#13136

Open
DaanHoogland wants to merge 4 commits into
mainfrom
cleanup-test-deploy-vm-iso-resources
Open

test: cleanup resources in test_deploy_vm_iso, use base class tearDown#13136
DaanHoogland wants to merge 4 commits into
mainfrom
cleanup-test-deploy-vm-iso-resources

Conversation

@DaanHoogland

Copy link
Copy Markdown
Contributor

The test was creating an ISO and a VirtualMachine without adding them to self.cleanup, so they were never deleted after each test run.

  • Add iso and virtual_machine to self.cleanup (using append) so the base-class tearDown deletes them after each test.
  • Remove the explicit tearDown override — the base class cloudstackTestCase.tearDown already calls cleanup_resources(self.apiclient, reversed(self.cleanup)).
  • Remove the explicit tearDownClass override — the base class cloudstackTestCase.tearDownClass already handles cls._cleanup for both the apiclient and api_client attribute names.
  • Rename cls.api_client → cls.apiclient in setUpClass so the base-class tearDownClass can find the client.
  • Remove the now-unused cleanup_resources import from marvin.lib.utils.

Relates to #3693

Description

This PR...

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@boring-cyborg boring-cyborg Bot added component:integration-test Python Warning... Python code Ahead! labels May 8, 2026
@codecov

codecov Bot commented May 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 19.33%. Comparing base (72b99a3) to head (00c2bbe).
⚠️ Report is 52 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13136      +/-   ##
============================================
+ Coverage     18.08%   19.33%   +1.24%     
- Complexity    16718    18751    +2033     
============================================
  Files          6037     6160     +123     
  Lines        542546   569460   +26914     
  Branches      66432    74092    +7660     
============================================
+ Hits          98146   110131   +11985     
- Misses       433378   447349   +13971     
- Partials      11022    11980     +958     
Flag Coverage Δ
uitests 3.87% <ø> (+0.35%) ⬆️
unittests 20.52% <ø> (+1.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to ensure test_deploy_vm_iso properly cleans up created resources by relying on the base cloudstackTestCase tearDown/tearDownClass cleanup mechanisms, so test runs don’t leak ISOs/VMs/accounts into the environment.

Changes:

  • Remove the test-specific tearDown/tearDownClass and rely on cloudstackTestCase cleanup handling.
  • Standardize the class client attribute name to cls.apiclient so base-class cleanup can find it.
  • Add newly created resources (ISO, VM) to self.cleanup so they’re deleted after each test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/integration/smoke/test_deploy_vm_iso.py Outdated
Comment thread test/integration/smoke/test_deploy_vm_iso.py
Comment thread test/integration/smoke/test_deploy_vm_iso.py Outdated
Comment thread test/integration/smoke/test_deploy_vm_iso.py
@DaanHoogland DaanHoogland force-pushed the cleanup-test-deploy-vm-iso-resources branch from 3a234d8 to acf78ab Compare May 8, 2026 13:52
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@apache apache deleted a comment from blueorangutan May 8, 2026
@apache apache deleted a comment from blueorangutan May 8, 2026
@apache apache deleted a comment from blueorangutan May 21, 2026
@apache apache deleted a comment from blueorangutan May 21, 2026
@apache apache deleted a comment from blueorangutan May 21, 2026
@apache apache deleted a comment from blueorangutan May 21, 2026
Comment thread test/integration/smoke/test_deploy_vm_iso.py Outdated
Comment thread test/integration/smoke/test_deploy_vm_iso.py Outdated
Co-authored-by: dahn <daan.hoogland@gmail.com>
@DaanHoogland DaanHoogland requested a review from Copilot May 26, 2026 09:50
@apache apache deleted a comment from blueorangutan May 26, 2026
@apache apache deleted a comment from blueorangutan May 26, 2026
@apache apache deleted a comment from blueorangutan May 26, 2026
@apache apache deleted a comment from blueorangutan May 26, 2026

This comment was marked as low quality.

@sonarqubecloud

Copy link
Copy Markdown

@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@blueorangutan test keepEnv

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

Comment on lines +164 to +169

response = self.virtual_machine.getState(
self.apiclient,
VirtualMachine.RUNNING)
self.assertEqual(response[0], PASS, response[1])

Comment on lines +116 to +124
list_iso_response = Iso.list(
self.apiclient,
id=self.iso.id
)
while not isinstance(list_iso_response, list):
list_iso_response = Iso.list(
self.apiclient,
id=self.iso.id
)
Comment on lines 101 to 106
# Validate the following:
# 1. deploy VM using ISO
# 2. listVM command should return the deployed VM. State of this VM
# should be "Running".
self.hypervisor = self.testClient.getHypervisorInfo()
if self.hypervisor.lower() in ['lxc']:
self.skipTest(
"vm deploy from ISO feature is not supported on %s" %
self.hypervisor.lower())
# 1. Create an ISO
# 2. Deploy a VM from the ISO
# 3. VM should be in 'Running' state

self.iso = Iso.create(
isinstance(list_vm_response, list),
True,
"Check list response returns a valid list"
)
@DaanHoogland

Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@apache apache deleted a comment from blueorangutan Jun 11, 2026
@apache apache deleted a comment from blueorangutan Jun 11, 2026
@apache apache deleted a comment from blueorangutan Jun 11, 2026
@blueorangutan

Copy link
Copy Markdown

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@apache apache deleted a comment from blueorangutan Jun 11, 2026
@sonarqubecloud

Copy link
Copy Markdown

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18228

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:integration-test Python Warning... Python code Ahead!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants