[RFR] New test: test_reconfigure_vm_vmware_multiple #10236
Conversation
valaparthvi
added
fix-framework
test-cleanup
valaparthvi
force-pushed
the
hot-cold-reconfigure
branch
from
3d744c2
to
2538194
Compare
valaparthvi
force-pushed
the
hot-cold-reconfigure
branch
from
2538194
to
87aae13
Compare
valaparthvi
changed the title
| full_vm.wait_for_vm_state_change(full_vm.STATE_ON) | ||
| else: | ||
| raise Exception("Unknown power state - unable to continue!") | ||
| raise Exception("Unknown power state - unable to continue!") |
There was a problem hiding this comment.
Optional: Umn, I would rather used either AssertionError or ValueError here to be more specific.
| view = navigate_to(reconfig_request.parent, "All") | ||
| # Get the latest request | ||
| request_id = max( | ||
| [ |
| """ | ||
| vms = create_vms_modscope |
There was a problem hiding this comment.
Optional:
I think you are expecting only one vm to get created here, but I am not sure. I like to do this:
vms, = create_vms_modscope
This makes clear the only single vm is expected. Also this statement has an advantage that it fails when there are more or less than expected vms returned by the fixture and I think it can be statically checked as well in many situations, so we may see a problem with static type checker.
There was a problem hiding this comment.
I am expecting 2 vms here, I need the whole vms list to get entities and navigate to the reconfiguration page. We already have tests to check reconfiguration for 1 VM, we're testing for multiple VMs here.
But to make sure I receive 2 vms, I added one more item({"num_vms": 2}) to the create_vms_modscope parametrization to ensure it creates 2 vms, incase someone changes default value in the fixture. So as long as there's no other change in the fixture, it will work just fine for me.
| try: | ||
| reconfig_request.remove_request() | ||
| except (RequestException, NoSuchElementException): | ||
| # sometimes the delete button is not present on the page. |
There was a problem hiding this comment.
Thanks for the comment! It is helpful for me as a reader of the code.
| new_memory = current_memory + 1 | ||
| else: | ||
| if current_memory < 1: | ||
| pytest.skip("Cannot test when the value is less than the minimum required.") |
There was a problem hiding this comment.
Optional nitpick:
The skip message seem to say something different than what the code seem to be doing. I think it should read
"Cannot decrease memory bellow 1".
I mean: I cannot find the code which gets the info about what is the minimum requirement. There is just an assumption the minimum is 1, but it could be something else, like 3, right?
| new_processor = current_processors + 1 | ||
| else: | ||
| if current_processors < 1: | ||
| pytest.skip("Cannot test when value is less than the minimum required.") |
There was a problem hiding this comment.
There is double space on this line.
valaparthvi
force-pushed
the
hot-cold-reconfigure
branch
from
c8563a3
to
54cc0b5
Compare
|
I detected some fixture changes in commit 54cc0b5 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 😃 |
|
Thank you for the review @JaryN, your suggestion were good! |
| @pytest.mark.tier(2) | ||
| @pytest.mark.provider([VMwareProvider]) | ||
| def test_reconfigure_vm_vmware_mem_multiple(): | ||
| @pytest.mark.provider([VMwareProvider], selector=ONE_PER_TYPE, scope="module") |
There was a problem hiding this comment.
This is effectively a ONE selector, as VMwareProvider is a single type.
Purpose or Intent
InfraVmReconfigureView.is_displayedto work when reconfiguring multiple VMs.-Deleting tests
PRT Run
{{ pytest: cfme/tests/infrastructure/test_vm_reconfigure.py::test_reconfigure_vm_vmware_multiple --use-provider=vsphere67-nested --long-running -svvv }}