[1LP][RFR] New Test: Test overwrite domain using rake command #9756

ganeshhubale · 2019-12-11T14:11:52Z

Signed-off-by: Ganesh Hubale [email protected]

Purpose or Intent

Test overwrite domain using rake

PRT Run

{{ pytest: cfme/tests/automate/test_file_import.py -k 'test_overwrite_import_domain' --use-template-cache -qsvv }}

- Test overwrite domain using rake Signed-off-by: Ganesh Hubale <[email protected]>

valaparthvi

Some changes and questions :)

valaparthvi · 2019-12-17T10:14:49Z

cfme/tests/automate/test_file_import.py

+
+@pytest.fixture(scope='module')
+def local_domain(appliance):
+    """This fixture used to create automate domain - Datastore/Domain"""


This fixture is used...

valaparthvi · 2019-12-17T10:15:09Z

cfme/tests/automate/test_file_import.py

+    domain = appliance.collections.domains.create(
+        name="bz_1753860",
+        description=fauxfactory.gen_alpha(),
+        enabled=True)


Can you improve the formatting here?

valaparthvi · 2019-12-17T10:17:02Z

cfme/tests/automate/test_file_import.py

+
+@pytest.mark.tier(2)
+@pytest.mark.meta(automates=[1753860])
+@pytest.mark.parametrize("file_name", ["bz_1753860.zip"])


In case you decide to use a different file name in the future, the test name would also change. Can you use ids here, so that the test name remains consistent?

valaparthvi · 2019-12-17T10:19:58Z

cfme/tests/automate/test_file_import.py

+        caseposneg: positive
+        casecomponent: Automate
+        testSteps:
+            1. Create custom domain, namespace, class, instance, method. Do not delete this domain.


This looks like it is a part of the setup.
Also, I'm curious, you're only creating domain here, I don't see namespace, class, instance and method creation, where is it happening?

These things needs to create manually by exporting datastore which is not possible in automation.

Okay, so the datastore you're importing creates all these, correct?

valaparthvi · 2019-12-17T10:40:04Z

cfme/tests/automate/test_file_import.py

+    for domain in view.domains.read():
+        if domain['Name'] == local_domain.name:
+            assert domain['Enabled'] == "true"


Suggested change

for domain in view.domains.read():

if domain['Name'] == local_domain.name:

assert domain['Enabled'] == "true"

assert view.domains.row(name__contains=local_domain.name)["Enabled"].text == "true"

valaparthvi · 2019-12-17T10:40:56Z

cfme/tests/automate/test_file_import.py

+    for domain in view.domains.read():
+        if domain['Name'] == f"{local_domain.name} (Disabled)":
+            assert domain['Enabled'] == "false"


Suggested change

for domain in view.domains.read():

if domain['Name'] == f"{local_domain.name} (Disabled)":

assert domain['Enabled'] == "false"

assert view.domains.row(name__contains=local_domain.name)["Enabled"].text == "false"

valaparthvi · 2019-12-17T10:58:57Z

cfme/tests/automate/test_file_import.py

+    cmd = [(
+        f"evm:automate:import PREVIEW=false DOMAIN=bz_1753860 IMPORT_AS=bz_1753860 "
+        f"ZIP_FILE={file_path} SYSTEM=false ENABLED={enable} OVERWRITE=true"
+    ) for enable, file_path in [{file_path, 'false'}, {file_path, 'true'}]]


I think something is wrong with these commands, perhaps it should be something like this -

Suggested change

cmd = [(

f"evm:automate:import PREVIEW=false DOMAIN=bz_1753860 IMPORT_AS=bz_1753860 "

f"ZIP_FILE={file_path} SYSTEM=false ENABLED={enable} OVERWRITE=true"

) for enable, file_path in [{file_path, 'false'}, {file_path, 'true'}]]

cmd = [(

f"evm:automate:import PREVIEW=false DOMAIN=bz_1753860 IMPORT_AS=bz_1753860 "

f"ZIP_FILE={file_path} SYSTEM=false ENABLED={enable} OVERWRITE=true"

) for enable in ['false', 'true']]

valaparthvi · 2019-12-17T11:29:40Z

cfme/tests/automate/test_file_import.py

+        description=fauxfactory.gen_alpha(),
+        enabled=True)
+    yield domain
+    domain.delete_if_exists()


I think it would be better if you use REST to delete the domain.

if domain.rest_api_entity.exists: domain.rest_api_entity.action.delete()

The reason being, if the test case fails/errors out/stops after disabling the domain, it will not be able to delete the domain via UI, since domain name in the tree would change to domain_name (Disabled), and its tree path would also change to `["Datastore", "domain_name (Disabled)"].

To delete a domain, it first checks the path in the bootstrap tree. Since .enabled property of local_domain does not get updated when you enable or disable it and since the original domain.enabled value is True, it tries to search for ["Datastore", local_domain.name] in the bootstrap tree path, but does not find it and hence is not able to delete the domain.

With REST this problem does not arise, so it doesn't matter whether a domain is enabled or disabled, it will delete it if it exists as long as it is not locked.

Thank you for detailed solution @valaparthvi
It is very helpful. But it worked with 5.11 and givin some error in 5.10 as below:

ipdb> domain.rest_api_entity.action.all ['refresh_from_source']

It does not have delete action in 5.10 via REST

Oh yes, so maybe in that case, instead of using REST, you could make sure that the domain is enabled before deleting it, or maybe modify the delete() method to delete a domain no matter it's status.

Another solution would be something like this -

Suggested change

domain.delete_if_exists()

@pytest.fixture(scope='module')

def local_domain(appliance):

"""This fixture used to create automate domain - Datastore/Domain"""

# Domain name should be static to match with name of domain imported using rake command

domain = appliance.collections.domains.create(

name="bz_1753860", description=fauxfactory.gen_alpha(), enabled=True

)

yield domain

domain.enabled = domain.rest_api_entity.enabled

domain.delete()

The reason being, treepath of a domain is decided on the basis of it's enabled status. We can change domain.enabled to whatever it's current value is, that way the domain will be deleted for sure.

valaparthvi

LGTM. Thank you for being patient and working on all the changes. :)

digitronik

LGTM 👍 thanks for PR

Please try to avoid repetition by creating data dict for state and cmd as per enable and disable status.

digitronik · 2019-12-18T10:25:35Z

cfme/tests/automate/test_file_import.py

+        expectedResults:
+            1. Description and enabled status of existing domain should update.
+    """
+    file = FTPClientWrapper(cfme_data.ftpserver.entities.datastores).get_file(file_name)


I would try to avoid overwrite of python builtin.

digitronik · 2019-12-18T10:44:42Z

cfme/tests/automate/test_file_import.py

+    assert appliance.ssh_client.run_command(f"curl -o {file_path} ftp://{file.link}").success
+
+    # Rake command to update domain
+    cmd = [(


optional: we need to use indexing (:, I would unpack here only enable_cmd, disable_cmd =

dajoRH · 2019-12-18T11:33:34Z

I detected some fixture changes in commit f702429

The local fixture local_domain is used in the following files:

cfme/tests/automate/test_file_import.py
- test_overwrite_import_domain

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

digitronik

Thanks for PR 👍

…IQ#9756) * New Test: - Test overwrite domain using rake Signed-off-by: Ganesh Hubale <[email protected]> * Updated with requested * Updated teardown of domain * Updated code to remove repetition

ganeshhubale added the test-automation To be applied on PR's which are automating existing manual cases label Dec 11, 2019

dajoRH added lint-ok WIP-testing labels Dec 11, 2019

ganeshhubale force-pushed the rake-over branch from 8678484 to 3c77c30 Compare December 17, 2019 09:09

New Test:

a1df91e

- Test overwrite domain using rake Signed-off-by: Ganesh Hubale <[email protected]>

ganeshhubale force-pushed the rake-over branch from 3c77c30 to a1df91e Compare December 17, 2019 09:11

ganeshhubale changed the title ~~[WIPTEST] New Test: Test overwrite domain using rake command~~ [RFR] New Test: Test overwrite domain using rake command Dec 17, 2019

valaparthvi suggested changes Dec 17, 2019

View reviewed changes

valaparthvi changed the title ~~[RFR] New Test: Test overwrite domain using rake command~~ [WIPTEST] New Test: Test overwrite domain using rake command Dec 17, 2019

Ganesh Hubale added 2 commits December 17, 2019 19:10

Updated with requested

3584e87

Updated teardown of domain

11d7bf9

ganeshhubale changed the title ~~[WIPTEST] New Test: Test overwrite domain using rake command~~ [RFR] New Test: Test overwrite domain using rake command Dec 18, 2019

valaparthvi approved these changes Dec 18, 2019

View reviewed changes

valaparthvi changed the title ~~[RFR] New Test: Test overwrite domain using rake command~~ [1LP][RFR] New Test: Test overwrite domain using rake command Dec 18, 2019

dajoRH removed the WIP-testing label Dec 18, 2019

digitronik suggested changes Dec 18, 2019

View reviewed changes

digitronik self-assigned this Dec 18, 2019

digitronik changed the title ~~[1LP][RFR] New Test: Test overwrite domain using rake command~~ [1LP][WIPTEST] New Test: Test overwrite domain using rake command Dec 18, 2019

Updated code to remove repetition

f702429

dajoRH added the WIP-testing label Dec 18, 2019

ganeshhubale changed the title ~~[1LP][WIPTEST] New Test: Test overwrite domain using rake command~~ [1LP][RFR] New Test: Test overwrite domain using rake command Dec 18, 2019

digitronik approved these changes Dec 18, 2019

View reviewed changes

digitronik merged commit 47f4f89 into ManageIQ:master Dec 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1LP][RFR] New Test: Test overwrite domain using rake command #9756

[1LP][RFR] New Test: Test overwrite domain using rake command #9756

ganeshhubale commented Dec 11, 2019

valaparthvi left a comment

valaparthvi Dec 17, 2019

valaparthvi Dec 17, 2019

valaparthvi Dec 17, 2019

valaparthvi Dec 17, 2019

ganeshhubale Dec 18, 2019

valaparthvi Dec 18, 2019

ganeshhubale Dec 18, 2019

valaparthvi Dec 17, 2019

valaparthvi Dec 17, 2019

valaparthvi Dec 17, 2019

valaparthvi Dec 17, 2019

ganeshhubale Dec 18, 2019

valaparthvi Dec 18, 2019

valaparthvi Dec 18, 2019

valaparthvi left a comment

digitronik left a comment

digitronik Dec 18, 2019

digitronik Dec 18, 2019

dajoRH commented Dec 18, 2019

digitronik left a comment

[1LP][RFR] New Test: Test overwrite domain using rake command #9756

[1LP][RFR] New Test: Test overwrite domain using rake command #9756

Conversation

ganeshhubale commented Dec 11, 2019

Purpose or Intent

PRT Run

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

valaparthvi left a comment

Choose a reason for hiding this comment

digitronik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dajoRH commented Dec 18, 2019

digitronik left a comment

Choose a reason for hiding this comment