-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: ad groups update of the user model #103
test: ad groups update of the user model #103
Conversation
485499a
to
54417b7
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #103 +/- ##
==========================================
+ Coverage 56.12% 65.75% +9.63%
==========================================
Files 31 29 -2
Lines 1062 1098 +36
==========================================
+ Hits 596 722 +126
+ Misses 466 376 -90 ☔ View full report in Codecov by Sentry. |
helusers/tests/test_models.py
Outdated
# Nothing changes | ||
( | ||
ALL_AD_GROUPS_MAPPING, | ||
ALL_AD_GROUP_NAMES, | ||
ALL_GROUP_NAMES, | ||
ALL_AD_GROUP_NAMES, | ||
ALL_GROUP_NAMES, | ||
), |
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.
Just FYI, you can do something like this, which would make it more obvious which part of the test fails.
# Nothing changes | |
( | |
ALL_AD_GROUPS_MAPPING, | |
ALL_AD_GROUP_NAMES, | |
ALL_GROUP_NAMES, | |
ALL_AD_GROUP_NAMES, | |
ALL_GROUP_NAMES, | |
), | |
pytest.param( | |
ALL_AD_GROUPS_MAPPING, | |
ALL_AD_GROUP_NAMES, | |
ALL_GROUP_NAMES, | |
ALL_AD_GROUP_NAMES, | |
ALL_GROUP_NAMES, | |
id="nothing_changes", | |
), |
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.
I added the pytest-params with ids'.
helusers/tests/test_models.py::TestUserAdGroups::test_update_ad_groups[nothing_changes] PASSED [ 46%]
helusers/tests/test_models.py::TestUserAdGroups::test_update_ad_groups[not_mapped_not_added] PASSED [ 47%]
helusers/tests/test_models.py::TestUserAdGroups::test_update_ad_groups[new_added] PASSED [ 47%]
helusers/tests/test_models.py::TestUserAdGroups::test_update_ad_groups[old_removed] PASSED [ 47%]
helusers/tests/test_models.py::TestUserAdGroups::test_update_ad_groups[mapped_twice_given_once] PASSED [ 48%]
helusers/tests/test_models.py::TestUserAdGroups::test_update_ad_groups[mapped_twice_given_twice] PASSED [ 48%]
helusers/tests/test_models.py::TestUserAdGroups::test_update_ad_groups[all_removed] PASSED
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.
Added one minor comment, but LGTM! 👍
Test that the `update_ad_groups` function of the User-model works as expected: 1. If the same group is mapped to multiple ad-groups and the ad-groups input argument contains only some, the group is still persisted to user instance since it was mapped to at least one of given ad-groups. 2. If the ad-groups input argument does not have any link to an existing group of a user, the group is removed from the user instance. 3. If the ad-groups input argument contains links to new groups that the user is not yet linked to, the group will be added to the user instance.
54417b7
to
f3201ad
Compare
@charn just a note that I don't have privileges to merge. The pr is from my fork because I didn't have privileges to add it to the real project. |
Test that the
update_ad_groups
function of the User-model works as expected:If the same group is mapped to multiple ad-groups and the ad-groups input argument contains only some, the group is still persisted to user instance since it was mapped to at least one of given ad-groups.
If the ad-groups input argument does not have any link to an existing group of a user, the group is removed from the user instance.
If the ad-groups input argument contains links to new groups that the user is not yet linked to, the group will be added to the user instance.