Skip to content
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(group): fix frequent testSearchGroups failure #50319

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

joshtrichards
Copy link
Member

Summary

Fixes one of the causes of us frequently needing to rerun unit tests. Specifically it fixes this test:

There was 1 failure:

1) Test\Group\DatabaseTest::testSearchGroups
Failed asserting that 3 is identical to 2.

/home/runner/actions-runner/_work/server/server/tests/lib/Group/Backend.php:133

Cause:

The testSearchGroups() test shares its group namespace with other tests. These tests often call getGroupName() without a parameter. This causes this function to generate a random 13 character group name.

These randomized group names frequently (enough) contain the string sequence bar somewhere embedded in them. And getGroups() searches all configured groups using ILIKE.

As a result, the getGroups() call can return results from other tests that just happen to match on bar.

This then leads to search here to return unexpected groups. Obviously then the assert here fails. A "good enough" solution here is to adjust the test to use a longer unique string. In my testing this small adjustment was sufficient. It can still fail in theory but should be fairly unlikely now.

TODO

  • ...

Checklist

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added bug 3. to review Waiting for reviews tests Related to tests CI labels Jan 22, 2025
@joshtrichards joshtrichards added this to the Nextcloud 31 milestone Jan 22, 2025
@joshtrichards joshtrichards requested review from susnux, come-nc, a team, artonge and nfebe and removed request for a team January 22, 2025 16:14
@joshtrichards
Copy link
Member Author

/backport to stable30

@joshtrichards
Copy link
Member Author

/backport to stable29

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

Thats a funny thing 😅

@joshtrichards joshtrichards merged commit 451a843 into master Jan 22, 2025
189 checks passed
@joshtrichards joshtrichards deleted the jtr/fix-testSearchGroups branch January 22, 2025 17:44
@Altahrim Altahrim mentioned this pull request Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug CI tests Related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants