-
Notifications
You must be signed in to change notification settings - Fork 165
[WIP] Adding test coverage #9085
base: master
Are you sure you want to change the base?
Conversation
4137261
to
a04cedf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test cases look good, just one comment about adding coverage=[<bug_id>]
metadata
1. Snapshot not created; 'Required' displayed | ||
2. Snapshot created successfully | ||
Bugzilla: | ||
1647916 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@apagac since you are putting in the BZ metadata, can you also add it to the top of the file?
@pytest.mark.meta(coverage=[<bug_id>])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and the same for other tests
|
||
@pytest.mark.manual | ||
@pytest.mark.provider([OpenStackProvider]) | ||
@test_requirements.snapshot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All tests in this module have the same requirement, so please put it for the whole module
pytestmark = [test_requirements.snapshot]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lina-nikiforova some have rbac
and auth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh but they are different files, please ignore above 🤦♂️
|
||
|
||
@pytest.mark.manual | ||
@test_requirements.rbac |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here for the requirements, I think it's better to define at the module level
@pytest.mark.tier(2) | ||
def test_create_higher_permission(): | ||
""" | ||
Test creating a group with higher permission that the user currently has. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it the correct description? I don't see group creation in the test steps...
|
||
|
||
@pytest.mark.manual | ||
@test_requirements.auth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for the requirements as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, please look into the comments
Would you mind rebasing this Pull Request against latest master, please? |
Adding test coverage for BZs with test_coverage_flag.
Also removed
test_osp_snapshot_buttons
which is no longer relevant. Dev choose different approach.