-
Notifications
You must be signed in to change notification settings - Fork 165
[1LP][RFR] Automate multiple vm retirement, fix ec2 instance-backed retirement #9540
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work and awesome job automating all these test cases! I have some questions about the do_set_retirement_date
function you've defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small comments. Looking great though!
cfme/common/vm.py
Outdated
# explicit is/not None used here because of empty strings and dicts | ||
if when is not None and offset is not None: | ||
raise ValueError("set_retirement_date takes 'when' or 'offset', but not both") | ||
if when is not None and not isinstance(when, (datetime, date)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if when is not None and not isinstance(when, (datetime, date)): | |
if when and not isinstance(when, (datetime, date)): |
cfme/common/vm.py
Outdated
@@ -67,6 +67,90 @@ def all_types(template=False): | |||
return all_types | |||
|
|||
|
|||
def set_retirement_date(obj, when=None, offset=None, warn=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
REQUIRED: please turn these functions back into vm/instance methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved these methods so that they could be called both for a single VM/Instance plenty and for a list of one or more VMs/Instances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tpapaioa noted on your intended use, but this moves us toward a functional implementation instead of an OO one.
These methods, ideally, would operate on the collection or entity for a VM, so that a filtered collection or a single vm entity could have the same retire/set_retirement_date
method called on the instance, instead of the instance being passed.
The common functionality (param checking, form interaction) could be defined as module methods, but would need to be wrapped into instance methods for the collection/entity.
This would require better filtering support on the collections - right now I'm not aware of a way to filter the vms collections based on partially matched names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going to accept/allow this implementation of these, I'd like to see:
- docblock
- Better name for the first arg, since it could be an entity or list of entities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you would like to do that for a list of VMs. you can add collection method and pass it vm list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added retire and set_retirement_date (instance) methods (with docblocks) to the VMCollection class, which InstanceCollection and InfraVMCollection inherit.
retire(retire_vm_pair) | ||
|
||
# Verify flash message | ||
view = retire_vm_pair[0].appliance.browser.create_view(RequestsView) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can simplify this chain - and call create_view straight from retire_vm_pair or from appliance.browser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
retire_vm_pair is a list, but I've defined 'collection' as a reference to the vm collection retire_vm_pair[0].parent, which is used for calling create_view as well as the collection-level retire and set_retirement_date methods (see comment below).
I detected some fixture changes in commit 1c8087e The global fixture
The global fixture
The local fixture
The local fixture
Please, consider creating a PRT run to make sure your fixture changes do not break existing usage 😃 |
""" | ||
vms = create_vms(small_template.name, provider, 2) | ||
yield vms | ||
for num in range(2): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to index these:
for vm in vms:
vm.cleanup_on_provider()
[1LP][RFR] Automate multiple vm retirement, fix ec2 instance-backed retirement
1.) This PR removes test_retirement_manual.py, and replaces the following manual tests:
test_retire_infra_vms_folder
test_retire_infra_vms_date_folder
test_retire_infra_vms_notification_folder
test_retire_cloud_vms_folder
test_retire_cloud_vms_date_folder
test_retire_cloud_vms_notification_folder
with new automated tests in test_retirement.py:
test_retirement_now_multiple
test_set_retirement_date_multiple
test_set_retirement_offset_multiple
These new tests are similar to the ones for testing retirement of a single VM/Instance:
test_retirement_now
test_set_retirement_date
test_set_retirement_offset
but use a new fixture retire_vm_pair, which provisions two VMs or instances. And instead of selecting:
Lifecycle > Set Retirement Date or Lifecycle > Retire this [VM | Instance]
from the the VM | Instance Details page, the new tests will select the two VMs | instances from the All Instances / All VMs page and select:
Lifecycle > Set Retirement Dates or Lifecycle > Retire selected items.
2.) This also fixes test_retirement_now_ec2_instance_backed, which previously failed with the error:
3.) The required version of widgetastic.patternfly is now 1.2.0, to take advantage of the regex capability introduced in RedHatQE/widgetastic.patternfly#112
For test_set_retirement_offset and test_set_retirement_offset_multiple, the exact minute of the retirement date that ends up getting saved is hard to predict, because it will depend on exactly when the 'Set/Remove Retirement Date' page is loaded. These tests will create a regex of the form:
"^Retirement date set to (date_1|date_2|...|date_N)$" (single VM)
or
"^Retirement dates set to (date_1|date_2|...|date_N)$" (multiple VMs)
for a range of consecutive datetimes spanning the runtime of the test.
{{pytest: cfme/tests/cloud_infra_common/test_retirement.py -v --long-running --use-provider vsphere67-nested --use-provider ec2west -k 'test_set_retirement or test_retirement_now'}}