Skip to content

Tests: Rectify the docstring n testcode#8255

Merged
alexey-tikhonov merged 1 commit intoSSSD:masterfrom
shridhargadekar:ofl
Dec 5, 2025
Merged

Tests: Rectify the docstring n testcode#8255
alexey-tikhonov merged 1 commit intoSSSD:masterfrom
shridhargadekar:ofl

Conversation

@shridhargadekar
Copy link
Contributor

@shridhargadekar shridhargadekar commented Dec 2, 2025

Modifying test case doc-string to reflect correct group number.
Added verification of number of groups the user is part of initially.

@shridhargadekar shridhargadekar added Waiting for review Tests Trivial A single reviewer is sufficient to review the Pull Request no-backport This should go to target branch only. labels Dec 2, 2025
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request corrects the docstrings in test_ipa_trusts__aduser_membership_update_cache to match the number of groups used in the test code (10 instead of 4000). It also adds a valuable assertion to verify the initial number of group memberships. My review includes a suggestion to refactor the hardcoded number of groups into a constant to improve the test's maintainability and prevent future inconsistencies.

@jakub-vavra-cz jakub-vavra-cz self-assigned this Dec 2, 2025
Copy link
Contributor

@jakub-vavra-cz jakub-vavra-cz left a comment

Choose a reason for hiding this comment

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

Looks like the test is failing

@shridhargadekar shridhargadekar force-pushed the ofl branch 2 times, most recently from 216fbfc to 83b0cd4 Compare December 2, 2025 12:49
@shridhargadekar
Copy link
Contributor Author

Now the test is passing. One assertion failed as the user will have additional two group-membership of domain users and user-private-group. I've rectified the assertion accordingly.
Please check

Modifying the doc-string to reflect correct group number.
Added verification of number of groups the user is part of initially.

Reviewed-by: Jakub Vávra <jvavra@redhat.com>
@sssd-bot
Copy link
Contributor

sssd-bot commented Dec 3, 2025

The pull request was accepted by @jakub-vavra-cz with the following PR CI status:


🟢 CodeFactor (success)
🟢 CodeQL (success)
🟢 osh-diff-scan:fedora-rawhide-x86_64:upstream (success)
🟢 rpm-build:centos-stream-10-x86_64:upstream (success)
🟢 rpm-build:fedora-41-x86_64:upstream (success)
🟢 rpm-build:fedora-42-x86_64:upstream (success)
🟢 rpm-build:fedora-43-x86_64:upstream (success)
🟢 rpm-build:fedora-rawhide-x86_64:upstream (success)
🟢 Analyze (target) / cppcheck (success)
🔴 ci / intgcheck (centos-10) (failure)
🟢 ci / intgcheck (fedora-41) (success)
🟢 ci / intgcheck (fedora-42) (success)
🟢 ci / intgcheck (fedora-43) (success)
🟢 ci / intgcheck (fedora-44) (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-41) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🟢 ci / system (fedora-44) (success)
➖ Coverity scan / coverity (skipped)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)


There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

@alexey-tikhonov alexey-tikhonov merged commit a0574f7 into SSSD:master Dec 5, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Accepted no-backport This should go to target branch only. Tests Trivial A single reviewer is sufficient to review the Pull Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants