Skip to content
This repository has been archived by the owner on Apr 7, 2022. It is now read-only.

[RFR] Remove local VM fixtures and replace usage with global create_vm fixture. #10229

Merged
merged 11 commits into from
Aug 19, 2020

Conversation

prichard77
Copy link
Contributor

@prichard77 prichard77 commented Jun 30, 2020

Purpose or Intent

  • Updating tests to use a common create_vm fixture instead of local ones.

PRT Run

{{ pytest: --long-running cfme/tests/infrastructure/test_vm_migrate.py::test_vm_migrate cfme/tests/infrastructure/test_vm_ownership.py::test_rename_vm --long-running cfme/tests/infrastructure/test_vm_power_control.py cfme/tests/infrastructure/test_vm_reconfigure.py }}

@prichard77
Copy link
Contributor Author

test_vm_migrate did previously use a fixture that put "migrate" into the vm name. I don't believe we have support for altering the name in create_vm but I'll double check. Should we even worry about adding "migrate' to the name?

@prichard77 prichard77 changed the title [WIP] Remove local VM fixtures and replace usage with global create_vm fixture. [WIPTEST] Remove local VM fixtures and replace usage with global create_vm fixture. Jun 30, 2020
@prichard77
Copy link
Contributor Author

Some of these are failing and I am going thru and trying to determine if the tests were failing initially.
test_vm_migrate is failing for rhv4.3 but it fails with initial code. I don't own these tests, so I am not familiar with the history. Looking into now.

@prichard77
Copy link
Contributor Author

making RFR so I can see in active PRs on Ostriz

@prichard77 prichard77 changed the title [WIPTEST] Remove local VM fixtures and replace usage with global create_vm fixture. [RFR] Remove local VM fixtures and replace usage with global create_vm fixture. Aug 11, 2020
@prichard77
Copy link
Contributor Author

test_vm_ownership.py - test_rename_vm[virtualcenter] fails in cleanup. Is this related to the fact we did rename?

@prichard77
Copy link
Contributor Author

prichard77 commented Aug 11, 2020

Here is the rest of my notes from retesting the last PRT run. I am including the ones I put above so they will be in one place.

test_vm_migrate.py - test_vm_migrate[rhevm-4.3] retesting with 4.3 - Failed. Still fails when I add old code back in. 
                   - test_vm_migrate[rhevm-4.4] is passing in PRT.

test_vm_ownership.py - test_rename_vm[virtualcenter] fails in cleanup. Is this related to the fact we did rename?
                
test_vm_power_control.py - TestVmDetailsPowerControlPerProvider.test_power_off[virtualcenter-6.5] - passes locally
			   TestVmDetailsPowerControlPerProvider.test_power_on[rhevm-4.3] - passes locally
			   TestVmDetailsPowerControlPerProvider.test_power_on[rhevm-4.4] - failed locally first try, passed second
                           test_guest_os_shutdown[virtualcenter-6.5-full_template] timed out in PRT. Passed locally on retest.	   
		These all failed because of: Exception: Provider collection empty
			   These are orphaned VMs. Which means they have been disassociated with the provider. So wouldn't we expect the 
                              provider collection to be empty. We might need to add an "orphaned" option to create_view to support this?
		           test_orphaned_vm_status[rhevm-4.3] - passed on local retest? but failed teardown? 
                           test_orphaned_vm_status[rhevm-4.4] - passed on local retest? but failed teardown? 
                           test_orphaned_vm_status[virtualcenter-6.5] - passed on local retest? but failed teardown? 
                           test_power_options_on_archived_ophaned_vms_all_page[orphaned-rhevm-4.3]
			   test_power_options_on_archived_ophaned_vms_all_page[orphaned-rhevm-4.4]
                           test_power_options_on_archived_ophaned_vms_all_page[virtualcenter-6.5]```

@prichard77
Copy link
Contributor Author

i believe the orphan tests are failing teardown because they are orphans. We are getting failures for an empty Provider collection and I believe this would be expected for orphans. Still looking for confirmation.

@prichard77
Copy link
Contributor Author

I updated _cleaup in _create_vm to catch the "Provider collection empty" exception.

@prichard77
Copy link
Contributor Author

I do believe test_vm_ownership.py::test_rename_vm[virtualcenter] is failing in cleanup because we renamed the vm. I need to locate the actual cause and see if it a bug or something we need to update to support. Or maybe update the testcase to rename the vm back to the original.

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.

@prichard77
Copy link
Contributor Author

I updated cleanup_on_provider to catch TimeOutError and log it. This fixes the failure on cleanup for test_rename_vm.

@dajoRH
Copy link
Contributor

dajoRH commented Aug 18, 2020

I detected some fixture changes in commit 08ac103

Show fixtures

The local fixture vm_name is used in the following files:

  • cfme/tests/infrastructure/test_vm_power_control.py

The local fixture archived_vm is used in the following files:

  • cfme/tests/infrastructure/test_vm_power_control.py
    • test_no_power_controls_on_archived_vm
    • test_archived_vm_status

The local fixture orphaned_vm is used in the following files:

  • cfme/tests/infrastructure/test_vm_power_control.py
    • test_orphaned_vm_status

The local fixture archive_orphan_vm is used in the following files:

  • cfme/tests/infrastructure/test_vm_power_control.py
    • test_power_options_on_archived_orphaned_vms_all_page

The local fixture ensure_vm_stopped is used in the following files:

  • cfme/tests/infrastructure/test_vm_reconfigure.py
    • enable_hot_plugin
    • test_vm_reconfig_add_remove_hw_cold
    • test_reconfig_vm_negative_cancel

The local fixture ensure_vm_running is used in the following files:

  • cfme/tests/infrastructure/test_vm_reconfigure.py

The local fixture vm_state is used in the following files:

  • cfme/tests/infrastructure/test_vm_reconfigure.py
    • test_vm_reconfig_add_remove_disk
    • test_vm_reconfig_resize_disk

The local fixture enable_hot_plugin is used in the following files:

  • cfme/tests/infrastructure/test_vm_reconfigure.py

Please, consider creating a PRT run to make sure your fixture changes do not break existing usage 😃

@mshriver mshriver merged commit 6a1a2ae into ManageIQ:master Aug 19, 2020
dgaikwad pushed a commit to dgaikwad/integration_tests that referenced this pull request Nov 25, 2020
…m fixture. (ManageIQ#10229)

* Remove new_vm fixture and replace usage with create_vm in test_vm_migrate.py

* Update test_vm_ownership.py to use create_vm

* Update TestControlOnQuadicons in test_vm_power_control.py for create_vm usage

* Update vm_name fixture to use create_vm.name

* Update the rest of test_vm_power_control.py to use create_vm

* Update the rest of test_vm_reconfigure.py to use create_vm

* Add create_vm back where rebase removed it

* Update _create_vm to handle exception thrown by provider.refresh_provider_relationships() when using orphaned VMs

* Update except so exceptions other than Provider collection empty will get raised

* Update refresh_provider_relationships to use LookupError instead of Exception

* Catch TimeOutError in cleanup_on_provider and log it
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants