-
Notifications
You must be signed in to change notification settings - Fork 165
[1LP][RFR] - Service Dialog test #9995
[1LP][RFR] - Service Dialog test #9995
Conversation
a53d11a
to
fb4aade
Compare
assert view.element_information.dynamic_chkbox.is_enabled | ||
view.options.click() | ||
assert view.options.load_values_on_init.is_enabled | ||
request.addfinalizer(dialog.delete_if_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.
Put request.addfinalizer(dialog.delete_if_exists)
above the assert. If assert gets failed then it will not work.
} | ||
} | ||
dialog, element = create_dialog(appliance, element_data) | ||
view = appliance.browser.create_view(EditElementView) |
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 make create_dialog
- fixture if it is setup.
box_desc="my box desc") | ||
element = box.elements.create(element_data=[element_data]) | ||
return sd, element | ||
|
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.
Instead of this function. please check if you can use this fixture in this file: https://github.com/ManageIQ/integration_tests/pull/9995/files#diff-faeac2d3d489ac01e0ebae6983c9c3cbR36
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.
Actually I do not want to pass same input to create_dialog
so kept it as a function instead of fixture
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 are already passing input
dialog, element = create_dialog(appliance, element_data)
. - Also, for now, I can see this function used for this test case only, Want to know use of label=None argument here? or we can directly provide
sd = service_dialog.create(label=fauxfactory.gen_alphanumeric(15, start="label_"), description="my dialog")
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 have updated the function signature now
fb4aade
to
100d0b5
Compare
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.
@dgaikwad Some more questions. Please have a look :)
box_desc="my box desc") | ||
element = box.elements.create(element_data=[element_data]) | ||
return sd, element | ||
|
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 are already passing input
dialog, element = create_dialog(appliance, element_data)
. - Also, for now, I can see this function used for this test case only, Want to know use of label=None argument here? or we can directly provide
sd = service_dialog.create(label=fauxfactory.gen_alphanumeric(15, start="label_"), description="my dialog")
100d0b5
to
aaafd6a
Compare
@ganeshhubale can we move this to 1LP? |
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.
Need to update this PR with tier marker :)
Thanks @john-dupuy I will move it to 1LP once done with that suggestion
aaafd6a
to
b720ed2
Compare
@ganeshhubale required changes are done, please review PR. |
b720ed2
to
454b361
Compare
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.
@dgaikwad I found one fixture in same file which is creating dialog. Please check it is possible to use same with some modification.
@@ -88,6 +87,18 @@ def custom_categories(appliance): | |||
category.delete_if_exists() | |||
|
|||
|
|||
def create_dialog(appliance, element_data): |
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.
is it possible to use fallowing fixture?
https://github.com/ManageIQ/integration_tests/pull/9995/files#diff-faeac2d3d489ac01e0ebae6983c9c3cbL38
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.
Hello @digitronik , I have tried with fixture which you mentioned here but this fixture is test_required_dialog_elements
test-case specific fixture so can not use as normal generic fixture. So now instead of using normal function I have created one generic fixture and automated testcase. from now new testcase also can use new implemented create_service_dialog
fixture.
view.element.edit_element(element_data['element_information']['ele_label']) | ||
assert view.element_information.dynamic_chkbox.is_enabled | ||
view.options.click() | ||
request.addfinalizer(dialog.delete_if_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.
Best practice: Always put finalizer after step of creation. here I think your dialog creating at L804.
66422ab
to
9104c44
Compare
I detected some fixture changes in commit 9104c441479ab3d4abf3cdc5d07d11ead7273e73 The local fixture
Please, consider creating a PRT run to make sure your fixture changes do not break existing usage 😃 |
d6f20cf
to
7377076
Compare
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. One question please take look.
@@ -718,13 +746,18 @@ def test_load_values_on_init_option_service_dialog_element(): | |||
casecomponent: Services | |||
initialEstimate: 1/16h | |||
testSteps: | |||
1. create a service dialog | |||
1. create a service dialog with option dynamic to True |
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.
It looks like part of test steps not setup. If yes, I would try to create dynamic service dialog as part of test only.
Second thought if it setup; we already have one fixture service_dialog
better try to modify that in state of creating new fixture.
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.
dialog creation is part of testcase so updated tc accordingly.
7377076
to
491bcf9
Compare
491bcf9
to
247dc51
Compare
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.
LGTM
Adding new test - Test to check
Load values on init
option in service dialog whenDynamix
box enablePurpose or Intent
PRT Run
{{pytest: cfme/tests/services/test_dialog_element_in_catalog.py::test_catalog_load_values_on_init --long-running -v}}