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

[1LP][RFR] New Test: Testing ansible service with quota enablement #9803

Merged
merged 1 commit into from
Jan 16, 2020

Conversation

ganeshhubale
Copy link
Member

@ganeshhubale ganeshhubale commented Dec 24, 2019

Purpose or Intent

  • Testing ansible service with quota enablement

PRT Run

{{ pytest: cfme/tests/automate/test_ansible_tower_automate.py -k 'test_quota_for_ansible_service' --use-template-cache -qsvvv --use-provider=complete }}

@ganeshhubale ganeshhubale added the test-automation To be applied on PR's which are automating existing manual cases label Dec 24, 2019
@ganeshhubale ganeshhubale force-pushed the ansible-quota branch 3 times, most recently from 82bc35f to 1abcd21 Compare December 27, 2019 09:38
@ganeshhubale ganeshhubale changed the title [WIPTEST] New Test: Testing ansible service with quota enablement [RFR] New Test: Testing ansible service with quota enablement Dec 27, 2019
Copy link
Contributor

@valaparthvi valaparthvi left a comment

Choose a reason for hiding this comment

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

Few optional suggestions but LGTM otherwise..

def set_roottenant_quota(request, appliance):
roottenant = appliance.collections.tenants.get_root_tenant()
field, value = request.param
roottenant.set_quota(**{'{}_cb'.format(field): True, field: value})
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using f-string here?

yield
# will refresh page as navigation to configuration is blocked if alerts are on the page
appliance.server.browser.refresh()
roottenant.set_quota(**{'{}_cb'.format(field): False})
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using f-string here?

Comment on lines 84 to 85
bundle_name, description="catalog_bundle",
display_in=True, catalog=catalog,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you fix formatting here?

@valaparthvi valaparthvi changed the title [RFR] New Test: Testing ansible service with quota enablement [1LP][RFR] New Test: Testing ansible service with quota enablement Dec 31, 2019
    - Testing ansible service with quota enablement
@dajoRH
Copy link
Contributor

dajoRH commented Dec 31, 2019

I detected some fixture changes in commit d4bc769

The local fixture ansible_catalog_item is used in the following files:

  • cfme/tests/automate/test_ansible_tower_automate.py
    • test_quota_for_ansible_service

The local fixture set_roottenant_quota is used in the following files:

  • cfme/tests/automate/test_ansible_tower_automate.py

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

pytestmark = [
test_requirements.automate,
pytest.mark.provider([AnsibleTowerProvider], scope='module'),
pytest.mark.usefixtures('setup_provider')
Copy link
Member

Choose a reason for hiding this comment

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

This is a function scoped fixture, your provider marker is module scoped. Is this intentional? It will remove/add the provider between test cases in the module. Right now there's one provider parametrized by our current configs, but when there are more this will cause a performance hit.

@mshriver mshriver merged commit d3ed03f into ManageIQ:master Jan 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lint-ok test-automation To be applied on PR's which are automating existing manual cases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants