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

[1LP][RFR] Fix product features for role used in v2v migration test #9902

Merged
merged 5 commits into from
Feb 14, 2020

Conversation

nachandr
Copy link
Contributor

@nachandr nachandr commented Feb 4, 2020

Purpose or Intent

PRT Run

{{pytest: cfme/tests/v2v/test_v2v_migrations_ui.py -k test_rbac --use-template-cache --provider-limit 2 --use-provider rhv43 --use-provider vsphere65-nested -qsvv }}

@nachandr nachandr changed the title [WIP] Fix product features for role used in v2v migration test for 511 [RFR] Fix product features for role used in v2v migration test for 511 Feb 4, 2020
@dajoRH dajoRH added the lint-ok label Feb 4, 2020
@nachandr nachandr changed the title [RFR] Fix product features for role used in v2v migration test for 511 [RFR] Fix product features for role used in v2v migration test Feb 4, 2020
Copy link
Contributor

@sshveta sshveta left a comment

Choose a reason for hiding this comment

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

@nachandr : we can parametrize this test for True/False flag .
Code is same for both part except for this flag .
What do you think ?

"""
Test migration with role-based access control
Test to verify that the Migration tab is available/unavailable in the UI with
role-based access control

Polarion:
assignee: sshveta
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change the assignee to yourself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Also, parametrized the test on 'migration_feature_availability_for_role'.

@john-dupuy john-dupuy changed the title [RFR] Fix product features for role used in v2v migration test [WIPTEST] Fix product features for role used in v2v migration test Feb 6, 2020
@nachandr nachandr changed the title [WIPTEST] Fix product features for role used in v2v migration test [RFR] Fix product features for role used in v2v migration test Feb 7, 2020
@dajoRH dajoRH removed the WIP-testing label Feb 7, 2020
@dajoRH
Copy link
Contributor

dajoRH commented Feb 7, 2020

I detected some fixture changes in commit d1822c8

The local fixture setup_user_for_v2v_migration is used in the following files:

  • cfme/tests/v2v/test_v2v_migrations_ui.py
    • test_rbac_migration_tab_availability

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

@sshveta sshveta changed the title [RFR] Fix product features for role used in v2v migration test [1LP][RFR] Fix product features for role used in v2v migration test Feb 7, 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.

Nice PR @nachandr 👍
LGTM; one suggestion and an optional comment.

@@ -322,6 +322,33 @@ def _cleanup():
pass


@pytest.fixture
def setup_user_for_v2v_migration(appliance, new_credential):
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a good fixture 👍
@sshveta and @nachandr what you think to put this as a global fixture
https://github.com/ManageIQ/integration_tests/blob/master/cfme/fixtures/v2v_fixtures.py

Copy link
Contributor

Choose a reason for hiding this comment

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

@nachandr : Please check if there is already a fixture available like this . If not we can make it a global fixture .
Sure it can be used in all rbac tests



@pytest.mark.parametrize('migration_feature_availability_for_role', ['disabled', 'enabled'],
ids=['disabled', 'enabled'],
Copy link
Contributor

Choose a reason for hiding this comment

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

optional: These are default ids as we are iterating over the same list ['disabled', 'enabled']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@digitronik digitronik self-assigned this Feb 11, 2020
@jawatts jawatts self-assigned this Feb 13, 2020
@jawatts jawatts merged commit ca243ba into ManageIQ:master Feb 14, 2020
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.

6 participants