Skip to content
This repository has been archived by the owner on Apr 7, 2022. It is now read-only.

[1LP][RFR] Fix AttributeError on test_git_import module import #9909

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

mshriver
Copy link
Member

@mshriver mshriver commented Feb 7, 2020

{{ pytest: cfme/tests/automate/test_git_import.py::test_automate_git_domain_removed_from_disk }}

@mshriver mshriver changed the title [RFR] Fix AttributeError on test_git_import module import [WIP] Fix AttributeError on test_git_import module import Feb 7, 2020
@mshriver mshriver changed the title [WIP] Fix AttributeError on test_git_import module import [RFR] Fix AttributeError on test_git_import module import Feb 7, 2020


@pytest.fixture
def imported_domain(appliance):
if GIT_REPO_URL is None:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also do this via a separate fixture instead of having to re-use the same block?

@dajoRH dajoRH added the lint-ok label Feb 7, 2020
@dajoRH
Copy link
Contributor

dajoRH commented Feb 7, 2020

I detected some fixture changes in commit 2854dd4

The local fixture imported_domain is used in the following files:

  • cfme/tests/automate/test_git_import.py
    • test_automate_git_domain_removed_from_disk
    • test_refresh_git_current_user
    • test_automate_git_verify_ssl
    • test_automate_git_import_deleted_tag

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

@john-dupuy
Copy link
Contributor

@mshriver Since this key, value pair is also in the template yamls, is it necessary to check this?

manageiq_automate: https://github.com/RedHatQE/ManageIQ-automate-git.git

@mshriver
Copy link
Member Author

@john-dupuy I'd say yes - because its done on module import.

Copy link
Member

@ganeshhubale ganeshhubale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mshriver Thanks for this PR :)
But yeah, I am agree to create separate fixture for that block of code to avoid duplication of code.
Can we add that fixture in this PR?



@pytest.fixture
def imported_domain(appliance):
if GIT_REPO_URL is None:
pytest.skip('No automate repo url available at '
'cfme_data.automate_links.datastore_repositories.manageiq_automate')
repo = appliance.collections.automate_import_exports.instantiate(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional:
Can we modify this message:
No automate datastore is available at URL - cfme_data.automate_links.datastore_repositories.manageiq_automate'

@ganeshhubale ganeshhubale changed the title [RFR] Fix AttributeError on test_git_import module import [1LP][RFR] Fix AttributeError on test_git_import module import Feb 11, 2020
@digitronik digitronik self-assigned this Feb 11, 2020
Copy link
Contributor

@digitronik digitronik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ganeshhubale can you take care of fixture. For now its LGTM 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants