Skip to content

tests: Add netgroup offline and nested hierarchy#8272

Merged
thalman merged 1 commit into
SSSD:masterfrom
madhuriupadhye:netgroup2
Dec 15, 2025
Merged

tests: Add netgroup offline and nested hierarchy#8272
thalman merged 1 commit into
SSSD:masterfrom
madhuriupadhye:netgroup2

Conversation

@madhuriupadhye

Copy link
Copy Markdown
Contributor

Add netgroup tests for offline mode, hierarchy removal, and nested modifications

Add new system tests for netgroup functionality:

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

Copy link
Copy Markdown

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 introduces three valuable system tests for SSSD netgroup functionality, covering offline caching, hierarchical removal, and nested modifications. The tests are well-designed and target important use-cases. My feedback focuses on improving the test suite's reporting accuracy. I've identified an instance where pytest.skip should be used instead of raising a ValueError to correctly handle tests on unsupported provider configurations, ensuring they are reported as skipped rather than as errors.

Comment thread src/tests/system/tests/test_netgroups.py Outdated

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Add new system tests for netgroup functionality:

- test_netgroup__offline: Verify netgroups remain
	accessible from cache when backend goes offline
- test_netgroups__step_by_step_removal: Test removing
	netgroups from hierarchy and deleting them with proper cache invalidation
- test_netgroups__nested_modification: Test that adding/removing
	members in nested netgroups (3-level deep) propagates correctly through the hierarchy
	Related: https://fedorahosted.org/sssd/ticket/2841

Signed-off-by: Madhuri Upadhye <[email protected]>
Reviewed-by: Jakub Vávra <[email protected]>
Reviewed-by: Tomáš Halman <[email protected]>
@sssd-bot

Copy link
Copy Markdown
Contributor

The pull request was accepted by @thalman 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) (success)
🟢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants