-
Notifications
You must be signed in to change notification settings - Fork 165
[1LP][RFR] Fixing rest_mapping related TC for OSP #10013
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.
Looks Good @mnadeem92
Please address one failure in 5.10 |
@sshveta The failure on 5.10 is at the fixture level( _start_event_workers_for_osp ) that need to debug and address in a separate PR, As the changes in this PR is not directly related with this issue. IMO lets do not hold this PR as it is passing with 5.11 and we need it for upcoming 5.11.5 build and upstream testing now. |
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.
Nice PR...
Thanks for mixing test @mnadeem92
Please add inline/proper comments. with first look; I confused.
|
||
clusters_obj = appliance.rest_api.collections.clusters.all | ||
if provider.one_of(OpenStackProvider): | ||
clusters_obj += appliance.rest_api.collections.cloud_tenants.all |
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.
optional: I would use extend
@@ -42,8 +42,13 @@ def get_clusters(appliance, provider, source_provider): | |||
target_cluster = provider.data.get("clusters")[0] | |||
except IndexError: | |||
pytest.skip("Cluster not found in given provider data") | |||
|
|||
clusters_obj = appliance.rest_api.collections.clusters.all | |||
if provider.one_of(OpenStackProvider): |
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 Add one comment
explaining why we combining clusters and cloud tenants for OpenStack provider.
@@ -65,15 +70,23 @@ def get_clusters(appliance, provider, source_provider): | |||
@pytest.fixture(scope="function") | |||
def get_datastores(appliance, provider, source_provider): | |||
datastores = {} | |||
datastores_obj = appliance.rest_api.collections.data_stores.all | |||
if provider.one_of(OpenStackProvider): |
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.
^^ same
Signed-off-by: mnadeem92 <[email protected]>
b59f763
to
a7c4a5e
Compare
I detected some fixture changes in commit a7c4a5e The local fixture
The local fixture
The local fixture
Please, consider creating a PRT run to make sure your fixture changes do not break existing usage 😃 |
Signed-off-by: mnadeem92 [email protected]
{{ pytest: cfme/tests/v2v/test_rest_migrations.py -k "test_rest_mapping_create" --use-provider={osp13-ims,rhv-ims} --use-provider vsphere67-ims --provider-limit 2 -v }}
5.11 - Passed
5.10 - Failed due to TimedOutError at fixture function( _start_event_workers_for_osp) which need to address in a separate PR.