-
Notifications
You must be signed in to change notification settings - Fork 165
[1LP][RFR] Embedded Ansible service to check "Cloud Credentials" used in the service. #9414
Conversation
fb35bc6
to
c0a389e
Compare
} | ||
|
||
service = MyService(appliance, ansible_catalog_item.name) | ||
if service_request.exists(): |
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.
Comment for reviewers: This tear down is already written in funcscope
fixture, in this PR #9194
|
||
@request.addfinalizer | ||
def _revert(): | ||
with update(ansible_catalog_item): |
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.
Why is this required ? The ansible_catalog_item fixture is deleting the catalog item after yield .
https://github.com/ManageIQ/integration_tests/blob/master/cfme/fixtures/ansible_fixtures.py#L81
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.
ansible_catalog_item
fixture will be used in different tests. So, resetting back to default values to use it as default fixture for next tests.
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 don't think this is a good practice, as if this fails it will potentially impact all following test cases, and be difficult to debug. If you are modifying the catalog item, it should be made function scoped, at least for this test case.
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.
@mshriver This will impact on Over all tests execution in terms of time. Also, in most of the tests ansible_catalog_tem
is being modified to use another playbook. Does that makes sense.?
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.
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.
Also, will push changes to #9490
c0a389e
to
8a39f98
Compare
8a39f98
to
7b1fa88
Compare
"credential", CREDENTIALS, ids=[cred[0] for cred in CREDENTIALS] | ||
) | ||
@pytest.mark.provider( | ||
[RHEVMProvider, EC2Provider, VMwareProvider, AzureProvider], selector=ONE_PER_TYPE |
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.
Can we do pytest.mark.provider([InfraProvider, cloudProvider], selector=ONE_PER_TYPE) ?
If all providers are eligible
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.
@sshveta I am uncollecting other providers because of Playbooks. Once I will add playbooks for other Providers like OSP, I will Add Marker to collect all Providers.
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.
Please add in-line comment/TODO for why specific providers are collected.
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.
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 Comment/TODO for other providers.
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.
Module scoped catalog item instance needs to be treated differently if we're modifying it within test functions.
e4d0c4a
to
a13c08e
Compare
a13c08e
to
68f7999
Compare
I detected some fixture changes in commit 68f7999 The local fixture
Please, consider creating a PRT run to make sure your fixture changes do not break existing usage 😃 |
… in the service. (ManageIQ#9414) * Removed manual tests. * Added automation for BZ 1444092. * Added one more manual tests. * Updated new RHV playbook and added TODO.
Purpose or Intent
PRT Run
{{pytest: cfme/tests/ansible/test_embedded_ansible_services.py -k "test_ansible_service_cloud_credentials" --long-running --use-provider=complete -svvvv }}