Skip to content
This repository was archived by the owner on Apr 7, 2022. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cfme/common/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ def refresh_provider_relationships(self, from_list_view=False,
col[0].action.refresh()
self.wait_for_relationship_refresh(wait, delay, refresh_delta)
except IndexError:
raise Exception("Provider collection empty")
raise LookupError("Provider collection empty")

@refresh_provider_relationships.variant('ui')
def refresh_provider_relationships_ui(self, from_list_view=False, wait=0, delay=1,
Expand Down
8 changes: 6 additions & 2 deletions cfme/common/vm.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
from cfme.utils.rest import assert_response
from cfme.utils.update import Updateable
from cfme.utils.virtual_machines import deploy_template
from cfme.utils.wait import TimedOutError
from cfme.utils.wait import wait_for


Expand Down Expand Up @@ -989,10 +990,13 @@ def cleanup_on_provider(self, handle_cleanup_exception=True):
Helper method to avoid NotFoundError's during test case tear down.
"""
if self.exists_on_provider:
wait_for(self.mgmt.cleanup, handle_exception=handle_cleanup_exception,
try:
wait_for(self.mgmt.cleanup, handle_exception=handle_cleanup_exception,
timeout=300)
except TimedOutError:
logger.exception(f'cleanup_on_provider: entity {self.name} timed out.')
else:
logger.debug('cleanup_on_provider: entity "%s" does not exist', self.name)
logger.debug(f'cleanup_on_provider: entity {self.name} does not exist')

def equal_drift_results(self, drift_section, section, *indexes):
"""Compares drift analysis results of a row specified by it's title text.
Expand Down
6 changes: 5 additions & 1 deletion cfme/fixtures/vm.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ def _create_vm(request, template_type, provider, vm_name):
@request.addfinalizer
def _cleanup():
vm_obj.cleanup_on_provider()
provider.refresh_provider_relationships()
try:
provider.refresh_provider_relationships()
except Exception as e:
Copy link
Contributor

@jarovo jarovo Aug 17, 2020

Choose a reason for hiding this comment

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

Hey! Good job matching the args when we only have the too-broad Exception risen.

Optional:
I would suggest to include this and change accordingly.:

diff --git a/cfme/common/provider.py b/cfme/common/provider.py
index 11933a7c2..a3d0cd792 100644
--- a/cfme/common/provider.py
+++ b/cfme/common/provider.py
@@ -801,9 +801,10 @@ class BaseProvider(Taggable, Updateable, Navigatable, BaseEntity, CustomButtonEv
         col = self.appliance.rest_api.collections.providers.find_by(name=name)
         try:
             col[0].action.refresh()
-            self.wait_for_relationship_refresh(wait, delay, refresh_delta)
         except IndexError:
-            raise Exception("Provider collection empty")
+            raise LookupError("Provider collection empty")
+        else:
+            self.wait_for_relationship_refresh(wait, delay, refresh_delta)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. And then also catch the more specific LookupError instead of just an Exception in _cleanup().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why we'd want the else clause, however.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing exception but not adding else.

if e.args[0] != "Provider collection empty":
raise

vm_obj.mgmt.ensure_state(VmState.RUNNING)

Expand Down
27 changes: 4 additions & 23 deletions cfme/tests/infrastructure/test_vm_migrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from cfme.infrastructure.provider.rhevm import RHEVMProvider
from cfme.infrastructure.provider.virtualcenter import VMwareProvider
from cfme.utils.appliance.implementations.ui import navigate_to
from cfme.utils.generators import random_vm_name
from cfme.utils.log_validator import LogValidator


Expand All @@ -17,25 +16,7 @@
]


@pytest.fixture()
def new_vm(setup_provider, provider):
"""Fixture to provision appliance to the provider being tested if necessary"""
vm_name = random_vm_name(context='migrate')
try:
template_name = provider.data.templates.small_template.name
except AttributeError:
pytest.skip('Could not find templates.small_template.name in provider yaml: {}'
.format(provider.data))

vm = provider.appliance.collections.infra_vms.instantiate(vm_name, provider, template_name)

if not provider.mgmt.does_vm_exist(vm_name):
vm.create_on_provider(find_in_cfme=True, allow_skip="default")
yield vm
vm.cleanup_on_provider()


def test_vm_migrate(appliance, new_vm, provider):
def test_vm_migrate(appliance, create_vm, provider):
"""Tests migration of a vm

Metadata:
Expand All @@ -47,15 +28,15 @@ def test_vm_migrate(appliance, new_vm, provider):
initialEstimate: 1/4h
"""
# auto_test_services should exist to test migrate VM
view = navigate_to(new_vm, 'Details')
view = navigate_to(create_vm, 'Details')
vm_host = view.entities.summary('Relationships').get_text_of('Host')
hosts = [vds.name for vds in provider.hosts.all() if vds.name not in vm_host]
if hosts:
migrate_to = hosts[0]
else:
pytest.skip("There is only one host in the provider")
new_vm.migrate_vm("[email protected]", "first", "last", host=migrate_to)
request_description = new_vm.name
create_vm.migrate_vm("[email protected]", "first", "last", host=migrate_to)
request_description = create_vm.name
cells = {'Description': request_description, 'Request Type': 'Migrate'}
migrate_request = appliance.collections.requests.instantiate(request_description, cells=cells,
partial_check=True)
Expand Down
21 changes: 4 additions & 17 deletions cfme/tests/infrastructure/test_vm_ownership.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from cfme.markers.env_markers.provider import ONE_PER_TYPE
from cfme.rest.gen_data import vm as _vm
from cfme.utils.appliance.implementations.ui import navigate_to
from cfme.utils.generators import random_vm_name
from cfme.utils.rest import assert_response
from cfme.utils.wait import wait_for

Expand Down Expand Up @@ -113,21 +112,9 @@ def test_set_vm_owner(self, appliance, vm, from_detail):
assert rest_vm.evm_owner.userid == "admin"


@pytest.fixture(scope='function')
def small_vm(provider, small_template):
vm = provider.appliance.collections.infra_vms.instantiate(random_vm_name(context='rename'),
provider,
small_template.name)
vm.create_on_provider(find_in_cfme=True, allow_skip="default")
vm.refresh_relationships()
yield vm
if vm.exists:
vm.cleanup_on_provider()


@test_requirements.power
@pytest.mark.provider([VMwareProvider], scope="function", selector=ONE_PER_TYPE)
def test_rename_vm(small_vm):
def test_rename_vm(create_vm):
"""Test for rename the VM.

Polarion:
Expand All @@ -144,9 +131,9 @@ def test_rename_vm(small_vm):
5. Click on submit
6. Check whether VM is renamed or not
"""
view = navigate_to(small_vm, 'Details')
vm_name = small_vm.name
changed_vm = small_vm.rename(new_vm_name=fauxfactory.gen_alphanumeric(15, start="renamed_"))
view = navigate_to(create_vm, 'Details')
vm_name = create_vm.name
changed_vm = create_vm.rename(new_vm_name=fauxfactory.gen_alphanumeric(15, start="renamed_"))
view.flash.wait_displayed(timeout=20)
view.flash.assert_success_message('Rename of Virtual Machine "{vm_name}" has been initiated'
.format(vm_name=vm_name))
Expand Down
Loading