[1LP][RFR] - Check STDOUT of provisioned ansible service #10289

dgaikwad · 2020-08-10T13:52:11Z

Added test to check provisioned service's standard output of particular user
Fixed standard_output locator of service
Written view for "Set Ownership" for ansible catalog item.
test_appliance_console_logfile_config_reboot moved test to manual only because need large effort to automate it.

{{pytest: cfme/tests/configure/test_access_control.py::test_ansible_playbook_stdout cfme/tests/ansible/test_embedded_ansible_services.py::test_ansible_group_id_in_payload cfme/tests/ansible/test_embedded_ansible_services.py::test_service_ansible_playbook_pass_extra_vars --long-running -v}}

valaparthvi

This test is good but there are few things that could be made better.

valaparthvi · 2020-08-13T07:25:22Z

cfme/services/catalogs/catalog_items/__init__.py

+
+    @property
+    def is_displayed(self):
+        return self.form.select_an_owner.is_displayed and self.form.select_group.is_displayed


Optional - You might want to add other checks such as self.submit.is_displayed and self.cancel.is_enabled and self.reset.is_enabled.

Done, Can't add is_display for reset button before changing the state.

valaparthvi · 2020-08-13T07:26:07Z

cfme/services/myservice/ui.py

@@ -144,7 +144,7 @@ class provisioning(View):  # noqa
                      'normalize-space(.)="Plays"]]')
        details = SummaryTable(title='Details')
        credentials = SummaryTable(title='Credentials')
-        standart_output = Text('.//div[@id="provisioning"]//pre')
+        standart_output = Text('.//div[@id="provisioning"]/ansible-raw-stdout')


This doesn't break anything else, does it?

At this point we're only supporting 5.11 and upstream with the master branch, so I'm not concerned with the potential for this to break the locator in older versions.

valaparthvi · 2020-08-13T07:27:34Z

cfme/services/catalogs/catalog_items/__init__.py

@@ -826,3 +827,26 @@ class EditTags(CFMENavigateStep):

    def step(self, *args, **kwargs):
        self.prerequisite_view.policy.item_select('Edit Tags')
+
+
+class SetOwnershipView(ServicesCatalogView):


It would be nice if you could move this view with other views in the top part of this file.

valaparthvi · 2020-08-13T07:28:36Z

cfme/tests/configure/test_access_control.py

+@pytest.fixture()
+def custom_user(appliance):
+    group = appliance.collections.groups.instantiate("EvmGroup-user_self_service")
+    user = appliance.collections.users.create(


Why don't you use REST API to create the user? It would make this thing faster.

valaparthvi · 2020-08-13T07:31:46Z

cfme/tests/configure/test_access_control.py

+    view = navigate_to(ansible_catalog_item, "SetOwnership")
+    view.form.select_an_owner.fill("Administrator")
+    view.form.select_group.fill(group.description)
+    view.submit.click()


Thoughts on adding a method called set_ownership to BaseCatalogItem?

valaparthvi · 2020-08-13T07:37:56Z

cfme/tests/configure/test_access_control.py

+        fauxfactory.gen_alphanumeric(),
+        description="my ansible catalog",
+        items=[ansible_catalog_item.name],
+    )


Everything until here seems like a part of the setup, thoughts on moving this to a fixture, maybe renaming custom_user and putting everything there?

valaparthvi · 2020-08-13T07:38:26Z

cfme/tests/configure/test_access_control.py

+
+    with user:
+        with appliance.context.use(ViaSSUI):
+            appliance.server.login(user)


Since you're using with user you probably don't need to manually login.

I tried but it was not working without this login()

valaparthvi · 2020-08-13T07:40:29Z

cfme/tests/configure/test_access_control.py

+            view = ssui_nav(service_catalog, "Details")
+            view.add_to_shopping_cart.click()
+            view = ssui_nav(service_catalog, "ShoppingCart")
+            view.order.click()


I think you can directly do service_catalog.order() to order it and since you're using the SSUI context, it will order it via SSUI.

valaparthvi · 2020-08-13T07:41:37Z

cfme/tests/configure/test_access_control.py

+        if ansible_catalog.exists:
+            ansible_catalog.delete()


Suggested change

if ansible_catalog.exists:

ansible_catalog.delete()

ansible_catalog.delete_if_exists()

cfme/tests/configure/test_access_control.py

valaparthvi · 2020-08-17T07:48:22Z

cfme/tests/configure/test_access_control.py

-@pytest.mark.meta(coverage=[1836125])
-def test_ansible_playbook_stdout():
-    """ Test standard output of ansible playbook of particular user
+@pytest.mark.meta(automate=[1836125])


typo: automates

valaparthvi · 2020-08-17T08:05:08Z

cfme/tests/configure/test_access_control.py

+            ansible_service.delete()
+        ansible_catalog.delete_if_exists()
+
+    with user:


Have you tried putting everything under with user block?

with user: with appliance.context.use(ViaSSUI): appliance.server.login(user) ... ... error_message = ( "MIQ(Api::TasksController.api_error) Api::NotFoundError: Couldn't find MiqTask with" ) with LogValidator( "/var/www/miq/vmdb/log/api.log", failure_patterns=[error_message], ).waiting(timeout=120): ... ...

No because context is different.

valaparthvi · 2020-08-17T08:09:11Z

cfme/tests/configure/test_access_control.py

+            view = ssui_nav(service_catalog, "Details")
+            view.add_to_shopping_cart.click()
+            service_catalog.order()


You don't need to navigate to details page and click on shopping cart, you can just do service_catalog.order().

Suggested change

view = ssui_nav(service_catalog, "Details")

view.add_to_shopping_cart.click()

service_catalog.order()

service_catalog.order()

We have to explicitly add service in the catalog service_catalog.order() don't add

valaparthvi · 2020-08-17T08:11:34Z

cfme/tests/configure/test_access_control.py

@@ -242,6 +247,26 @@ def tenant_custom_role(appliance, request, provider):
    tenant_role.delete_if_exists()


+@pytest.fixture()
+def user_catalog(appliance, request, ansible_catalog_item):
+    group = appliance.rest_api.collections.groups.get(description="EvmGroup-user_self_service")


Optional - You can just assign group description to a variable instead of instantiating the group since you're only using group description.

dajoRH · 2020-08-17T12:00:17Z

I detected some fixture changes in commit eba58b3

The local fixture user_catalog is used in the following files:

cfme/tests/configure/test_access_control.py

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

mshriver · 2020-08-17T17:20:19Z

Updated pytest options in description to include cfme/tests/ansible/test_embedded_ansible_services.py::test_service_ansible_playbook_pass_extra_vars, which is using the widget with the modified locator. This should help validating that the locator modification will not impact existing tests.

@dgaikwad I saw some failures in PRT after including that test, blocking verification of the widget behavior.

Check out #10299, where I moved all ansible service requests to use the function scoped fixture.

mshriver · 2020-08-19T11:56:55Z

cfme/tests/configure/test_access_control.py

+    service_catalog = ServiceCatalogs(appliance, ansible_catalog, ansible_catalog_item.name)
+    ansible_service = MyService(appliance, ansible_catalog_item.name)
+
+    @request.addfinalizer


If you rebase this onto 10299, this finalizer will no longer be necessary.

mshriver · 2020-08-19T11:57:18Z

cfme/tests/configure/test_access_control.py

@@ -3024,4 +3049,33 @@ def test_ansible_playbook_stdout():
            9.
            10. able to see standard output
    """
-    pass
+    user, ansible_catalog = user_catalog
+    service_catalog = ServiceCatalogs(appliance, ansible_catalog, ansible_catalog_item.name)


There are fixtures to provide for and cleanup both of these instances.

mshriver

#10299 should follow this, I'll end up changing some things that you have here, but the test content and the new view is good!

dgaikwad force-pushed the test_ansible_playbook_stdout branch 3 times, most recently from 41bc916 to d810b05 Compare August 12, 2020 15:17

dgaikwad changed the title ~~[WIP]Check STDOUT of provisioned service~~ [WIPTEST]Check STDOUT of provisioned service Aug 12, 2020

dgaikwad force-pushed the test_ansible_playbook_stdout branch from d810b05 to 7f93ef5 Compare August 12, 2020 15:32

dgaikwad changed the title ~~[WIPTEST]Check STDOUT of provisioned service~~ [RFR] - Check STDOUT of provisioned ansible service Aug 12, 2020

valaparthvi suggested changes Aug 13, 2020

View reviewed changes

valaparthvi changed the title ~~[RFR] - Check STDOUT of provisioned ansible service~~ [WIPTEST] - Check STDOUT of provisioned ansible service Aug 13, 2020

valaparthvi added enhancement test-automation To be applied on PR's which are automating existing manual cases labels Aug 13, 2020

dgaikwad force-pushed the test_ansible_playbook_stdout branch 2 times, most recently from bd49003 to 18ec2db Compare August 13, 2020 17:00

dgaikwad changed the title ~~[WIPTEST] - Check STDOUT of provisioned ansible service~~ [RFR] - Check STDOUT of provisioned ansible service Aug 17, 2020

valaparthvi suggested changes Aug 17, 2020

View reviewed changes

valaparthvi changed the title ~~[RFR] - Check STDOUT of provisioned ansible service~~ [WIPTEST - Check STDOUT of provisioned ansible service Aug 17, 2020

valaparthvi changed the title ~~[WIPTEST - Check STDOUT of provisioned ansible service~~ [WIPTEST] - Check STDOUT of provisioned ansible service Aug 17, 2020

dgaikwad force-pushed the test_ansible_playbook_stdout branch from 18ec2db to d5f695f Compare August 17, 2020 10:33

dgaikwad changed the title ~~[WIPTEST] - Check STDOUT of provisioned ansible service~~ RFR] - Check STDOUT of provisioned ansible service Aug 17, 2020

dgaikwad changed the title ~~RFR] - Check STDOUT of provisioned ansible service~~ [RFR] - Check STDOUT of provisioned ansible service Aug 17, 2020

dgaikwad changed the title ~~[RFR] - Check STDOUT of provisioned ansible service~~ [WIPTEST] - Check STDOUT of provisioned ansible service Aug 17, 2020

Check STDOUT of provisioned service

eba58b3

dgaikwad force-pushed the test_ansible_playbook_stdout branch from d5f695f to eba58b3 Compare August 17, 2020 11:53

dgaikwad requested a review from valaparthvi August 17, 2020 12:18

dgaikwad changed the title ~~[WIPTEST] - Check STDOUT of provisioned ansible service~~ [RFR] - Check STDOUT of provisioned ansible service Aug 17, 2020

valaparthvi approved these changes Aug 17, 2020

View reviewed changes

valaparthvi changed the title ~~[RFR] - Check STDOUT of provisioned ansible service~~ [1LP][RFR] - Check STDOUT of provisioned ansible service Aug 17, 2020

mshriver self-assigned this Aug 17, 2020

mshriver mentioned this pull request Aug 17, 2020

[1LP][RFR] Refactor ansible service fixtures and embedded services tests #10299

Merged

mshriver reviewed Aug 19, 2020

View reviewed changes

mshriver approved these changes Aug 19, 2020

View reviewed changes

mshriver merged commit 2a89956 into ManageIQ:master Aug 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1LP][RFR] - Check STDOUT of provisioned ansible service #10289

[1LP][RFR] - Check STDOUT of provisioned ansible service #10289

dgaikwad commented Aug 10, 2020 •

edited by mshriver

Loading

valaparthvi left a comment

valaparthvi Aug 13, 2020

dgaikwad Aug 13, 2020

dgaikwad Aug 13, 2020

valaparthvi Aug 13, 2020

mshriver Aug 17, 2020

valaparthvi Aug 13, 2020

dgaikwad Aug 13, 2020

dgaikwad Aug 13, 2020

valaparthvi Aug 13, 2020

dgaikwad Aug 13, 2020

valaparthvi Aug 13, 2020

dgaikwad Aug 13, 2020

valaparthvi Aug 13, 2020

dgaikwad Aug 13, 2020

valaparthvi Aug 13, 2020

dgaikwad Aug 13, 2020

valaparthvi Aug 13, 2020

dgaikwad Aug 13, 2020

valaparthvi Aug 13, 2020

valaparthvi Aug 17, 2020

valaparthvi Aug 17, 2020

dgaikwad Aug 17, 2020

valaparthvi Aug 17, 2020

dgaikwad Aug 17, 2020

valaparthvi Aug 17, 2020

dajoRH commented Aug 17, 2020

mshriver commented Aug 17, 2020 •

edited

Loading

mshriver Aug 19, 2020

mshriver Aug 19, 2020

mshriver left a comment

	if ansible_catalog.exists:
	ansible_catalog.delete()
	ansible_catalog.delete_if_exists()

[1LP][RFR] - Check STDOUT of provisioned ansible service #10289

[1LP][RFR] - Check STDOUT of provisioned ansible service #10289

Conversation

dgaikwad commented Aug 10, 2020 • edited by mshriver Loading

valaparthvi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dajoRH commented Aug 17, 2020

mshriver commented Aug 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshriver left a comment

Choose a reason for hiding this comment

dgaikwad commented Aug 10, 2020 •

edited by mshriver

Loading

mshriver commented Aug 17, 2020 •

edited

Loading