fix(members): ensure group and project users are case-insensitive

[J4Numbers]

Meddle with group and project membership checks to ensure that
usernames are case-insensitive. This change ensures that users
are not added again by mistake, causing an error where a given
user already exists within the repository.

[codecov]

Codecov Report

Merging #556 (002bad8) into main (a44e184) will increase coverage by 0.06%.
The diff coverage is 95.45%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #556      +/-   ##
==========================================
+ Coverage   78.62%   78.69%   +0.06%     
==========================================
  Files          70       70              
  Lines        2699     2703       +4     
==========================================
+ Hits         2122     2127       +5     
+ Misses        577      576       -1     

Files Changed	Coverage Δ
gitlabform/processors/project/members_processor.py	87.32% <88.88%> (+0.36%)	⬆️
gitlabform/gitlab/members.py	75.00% <100.00%> (+2.08%)	⬆️
...abform/processors/group/group_members_processor.py	98.87% <100.00%> (-0.03%)	⬇️

... and 1 file with indirect coverage changes

[J4Numbers]

Addresses #555 and continues the work from #475.

[gdubicki]

Hey @J4Numbers, thank you for your contribution! And sorry for the CI problem, I noticed that there are HTTP 401 thrown. That's strange because it works for me locally. :/ I will try to troubleshoot it over the next few days.

[gdubicki]

Ok, now it seems to be failing on something related to your code, @J4Numbers:

FAILED tests/acceptance/premium/test_branches_users_case_insensitive.py::TestBranchesUsersCaseInsensitive::test__users_case_insensitive - TypeError: cannot unpack non-iterable ProjectProtectedBranch object

Can you check it out please?

[J4Numbers]

Thanks for kicking it off! Full error is below:

FAILED tests/acceptance/standard/test_members_enforce.py::TestMembersEnforce::test__enforce_keep_bots - assert 5 == 2
 +  where 5 = len([<ProjectMember id:37 username:gitlabform_user_maroon_4>, <ProjectMember id:40 username:gitlabform_user_maroon_5>, <Pr...:45 username:project_25_bot_016e6e53920e5925458ecb37e324d4a0>, <ProjectMember id:46 username:gitlabform_user_maroon_7>])

This might be a bit of trial and error since I can't test the premium items locally...

[gdubicki]

Thanks for kicking it off! Full error is below:

FAILED tests/acceptance/standard/test_members_enforce.py::TestMembersEnforce::test__enforce_keep_bots - assert 5 == 2
 +  where 5 = len([<ProjectMember id:37 username:gitlabform_user_maroon_4>, <ProjectMember id:40 username:gitlabform_user_maroon_5>, <Pr...:45 username:project_25_bot_016e6e53920e5925458ecb37e324d4a0>, <ProjectMember id:46 username:gitlabform_user_maroon_7>])

This might be a bit of trial and error since I can't test the premium items locally...

This is fixed now.

But

[J4Numbers]

Thanks for picking up the missed import!

This is fixed now.

But

Guessing something got cut off here. But it does look like there's a race condition or some test overlap in this somewhere (with the odd failure between different python versions). Want me to raise an issue for it?

[gdubicki]

This is fixed now.
But

Guessing something got cut off here. But it does look like there's a race condition or some test overlap in this somewhere (with the odd failure between different python versions). Want me to raise an issue for it?

This may be caused by the fact that many tests depend on each other, see #561 (review) for more info.

[J4Numbers]

Fair enough. Let me know if you need me to do anything else!

[amimas]

Unsure why there's a gitlab premium related test case. I think it doesn't belong there.

[amimas]

@J4Numbers - Pushed the latest update to your branch. Noticed a test failure from one of your test case.

https://github.com/gitlabform/gitlabform/actions/runs/5767054764/job/15636113972?pr=556

Is this an intermittent test failure? If yes, would you be able to look into the test setup? It'd be good to have it fixed if possible as intermittent failures take away too much time in processing other PRs.

Hopefully we can merge this PR soon.

[amimas]

@J4Numbers - Did you get a chance to look at the test failure mentioned above?

[J4Numbers]

Ah, sorry. I started looking into it, but then real life got in the way slightly. From what I can tell, a lot of the code should be pretty segregated due to the random group creation that the tests are separated between (something similar to gitlabform_group_horse) which then has its own projects and users - all of which follow that format of gitlabform_{basename}_{random_xkcd_string} which makes unpicking why the tests are interdependent... fun.

I can keep looking into it if required, but the base logic for generating random groups/projects/users/etc. seems sound (though the reliance on randomness is eh), which makes me suspect that it might be something to do with the fixtures holding state when they shouldn't...

[J4Numbers]

So, as far as I can tell, separating the two project member tests into their own fixtures seems to have resolved my issue. And this run seems to have rolled a lucky six so far in terms of pipelines behaving...

I did notice one other intermittent failure, which was regarding a branch protection test. (See: https://github.com/gitlabform/gitlabform/actions/runs/6031288955/job/16364708681)

[amimas]

I started looking into it, but then real life got in the way slightly.

Hey @J4Numbers . Completely understandable. Hope all is well. Thanks for continuing with this PR. Really appreciate it 🙏 .

[amimas]

makes me suspect that it might be something to do with the fixtures holding state when they shouldn't...

So, as far as I can tell, separating the two project member tests into their own fixtures seems to have resolved my issue. And this run seems to have rolled a lucky six so far in terms of pipelines behaving...

First of all, thanks for making it more reliable. I'm not an expert at this but think you're on the right path. Looks like you've split your original tests into 2 separate test class (i.e. tests/acceptance/standard/test_project_members_case_insensitve.py and tests/acceptance/standard/test_project_group_members_case_insensitive.py`).

I believe what was happening before is that the tests were in the same class. Both tests were using the project fixture whose scope is at the class level. So, when the first test's membership is updated and verified, the 2nd test's evaluation will need to keep in mind the result from the earlier test, unless that one manually did the cleanup of members so that the project is at it's initial state. Why this happens intermittently, I'm not sure. Another similar intermittent issue was addressed by switching to project_for_function fixture in the test cases instead of using the project fixture. This has the same effect as splitting the tests into separate classes as you've done.

It would probably be nice if those tests were in the same class. Reduce some duplication and logically grouped; might be easier to maintain.

@gdubicki - Would you be able to do another review, please?

[amimas]

I did notice one other intermittent failure, which was regarding a branch protection test. (See: https://github.com/gitlabform/gitlabform/actions/runs/6031288955/job/16364708681)

Yeah, I noticed it a few times too. It's annoying 😄 .

[gdubicki]

LGTM, thank you for the contribution @J4Numbers!