[1LP][RFR] Add test coverage for BZ-1734629 and BZ-1734630

[Gauravtalreja1]

Purpose or Intent

Automated test for BZ's

• https://bugzilla.redhat.com/show_bug.cgi?id=1734629
• https://bugzilla.redhat.com/show_bug.cgi?id=1734630

PRT Run

{{ pytest: cfme/tests/ansible/test_embedded_ansible_automate.py::test_automate_ansible_playbook_set_stats --long-running }}

[mshriver]

Test looks good - just need to shuffle the finalizer to the top of the test function. Thanks!

[digitronik]

LGTM 👍 few questions.

[digitronik]

do we really need f-string here?

[Gauravtalreja1]

@digitronik Yes, it is needed here, as the variable import_datastore.name is from a fixture call import_datastore which set this variable.

[mshriver]

I think what @digitronik is getting at here is that the name attribute will resolve to a string at this line. Its redundant, but I wouldn't call it a blocker.

[digitronik]

same ^^

[Gauravtalreja1]

@digitronik Same here, as this instance variable is used to iterate two instances [CatalogItemInitialization_29, CatalogItemInitialization_30], passed through pytest parameteres.

[digitronik]

Optional:
I think we are iterating over two instances [CatalogItemInitialization_29, CatalogItemInitialization_30] so just if-else will work.

[Gauravtalreja1]

Yes, it will definitely work, but it is intended to use elif to specify a condition to iterate over 2nd instance, as it is optional, can we use elif here?

[digitronik]

Q: is it required? means manually setting catalog attribute to None.

[Gauravtalreja1]

Yes @digitronik , It is required too, as catatlog_item will be added to some catalog, and if one is deleting a catalog, then, in that case, a catalog_item has some catalog assigned, so it can't be deleted directly, and in the same way, dialog can't be deleted, because it's assigned to some catalog_item

[mshriver]

@Gauravtalreja1 I question that logic, because setting this instance attribute on the framework doesn't change anything on MIQ itself. Its only changing the instance within the pytest session/python context. An update method would have to be called to actually modify MIQ's catalog/catalog item association.

Were you actually finding an exception during test teardown without this line? I don't see anything in the catalog_item.delete method that is checking for catalog associations.

The global fixture for catalog_item in cfme.fixtures.service_fixtures isn't making any modifications to any linked catalogs before it calls delete_if_exists.

[Gauravtalreja1]

@mshriver Yes, without this line, it will fail in teardown while deleting a dialog with some AssertionError, I guess there is something, which is checking for catalog/catalog_items associations while deleting a dialog with dialog.delete_if_exists.

E AssertionError: assert_no_error: ['error:Dialog "HbIA5yO3Yr": Error during delete: Dialog cannot be deleted because it is connected to other components: ["ServiceTemplate:1 - cat_item_mjOTMB", "ServiceTemplate:1 - cat_item_mjOTMB"]']

Can you try running this test or maybe this finalizer definition? or suggest anything to workaround?

[mshriver]

Review actions on teardown, originally pointed out by @digitronik.

One request to use f-strings, otherwise the test content and structure look good, should be a quick merge after small updates.

[jawatts]

PRT failing, please check the failures out