feat(groups): add a list_ldap_group_links to go along with the pre ex… - #2371
Conversation
rayisbadat
commented
Nov 9, 2022
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2371 +/- ##
=======================================
Coverage 95.97% 95.97%
=======================================
Files 80 80
Lines 5313 5318 +5
=======================================
+ Hits 5099 5104 +5
Misses 214 214
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
a4456be to
858dd8b
Compare
I added a unit test and tox passed. But this my first time doing such a thing. So if its not right , I might need a little help on whats needed. |
nejch
left a comment
There was a problem hiding this comment.
@lmilbaum Could you please add a unit test?
I added a unit test and tox passed. But this my first time doing such a thing. So if its not right , I might need a little help on whats needed.
@rayisbadat it's a great start! You've added a test fixture, which you can now use to also create a test case, for example def test_list_ldap_group_links(group, resp_list_ldap_group_links):.
It will look a bit like this existing test, but without the isinstance assert probably:
python-gitlab/tests/unit/objects/test_groups.py
Lines 248 to 251 in 373b46c
Just a small tip, we usually put all the fixtures (things decorated with @pytest.fixture) at the top of the module, and test cases (def test_*) after them.
…isting add_ldap_group_link and delete_ldap_group_link
7a81117 to
6194085
Compare
|
@JohnVillalovos I think you wanted to have another look at this? |
JohnVillalovos
left a comment
There was a problem hiding this comment.
@nejch Looks good to me, though would like it to be squashed when it is merged in.
|
Thanks everyone. I did a squash and merge @JohnVillalovos |