tests: Add incomplete triples and complex hierarchy netgroup tests#8262
tests: Add incomplete triples and complex hierarchy netgroup tests#8262ikerexxe merged 2 commits intoSSSD:masterfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds two valuable system tests for netgroups, covering incomplete triples and complex hierarchies. The tests are well-structured and improve coverage. I've identified a correctness issue in the assertions of test_netgroup__incomplete_triples and suggested a fix to make it more robust. Additionally, I've proposed refactoring the assertions in test_netgroups__complex_hierarchy for better conciseness and reliability. With these changes, the new tests will be excellent additions to the suite.
d9af156 to
002331b
Compare
| :customerscenario: False | ||
| """ | ||
| if not isinstance(provider, (LDAP, Samba, AD)): | ||
| raise ValueError("IPA does not support domain in netgroups") |
There was a problem hiding this comment.
Use pytest skip instead.
There was a problem hiding this comment.
updated in all test cases.
|
|
||
| # Level 1: Base netgroups with only triples (no nested members) | ||
| ng_base1 = provider.netgroup("ng-base1").add() | ||
| ng_base1.add_member(host="host1", user="user1", domain="ldap.test") |
There was a problem hiding this comment.
There is everywhere hardcoded ldap.test domain.
There was a problem hiding this comment.
Updated in all test cases.
002331b to
6e83b81
Compare
6e83b81 to
58967fe
Compare
ikerexxe
left a comment
There was a problem hiding this comment.
Thanks for your work on this. I noticed the commit structure, and I think we need to clean it up before merging to maintain a clear git history and simplify cherry-picking for maintenance branches.
Commit number 2 changes code introduced in commit number 1 and it also refactors already existing code. My suggestion would be to:
- Commit 1: pure refactoring of the original, existing tests. This commit must pass tests on its own.
- Commit 2: add the two new test cases using the new, refactored structure.
c5415b7 to
2048c0c
Compare
1c98a62 to
bbc14e9
Compare
6a7e6b0 to
2692665
Compare
Replace hardcoded "ldap.test" domain strings with dynamic `provider.domain`
and fix type checking errors in netgroup tests.
Changes:
- Remove `raise ValueError` and `pytest.skip` isinstance checks since
topology markers already restrict tests to LDAP, AD, and Samba providers
- Change function signatures from `GenericProvider` to `AD | LDAP | Samba`
to match the topology decorators
- Replace all hardcoded "ldap.test" with `provider.domain` for dynamic
domain resolution across different provider topologies
- Rework parametrized test to use boolean flag and format string with
`{domain}` placeholder for runtime substitution
- Fix type errors by passing netgroup names as strings instead of objects
to `add_member(ng=...)` calls
- Fix type conflict by using separate variable names (`passwd_result`,
`group_result`) for different entry types
Affected tests:
- test_netgroup__user_attribute_membernisnetgroup_uses_group_dn
- test_netgroup__lookup_nested_groups
- test_netgroup__lookup_nested_groups_with_host_and_domain_values_present
- test_netgroup__uid_gt_2147483647
Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Reviewed-by: Jakub Vávra <jvavra@redhat.com>
Add new test cases to verify netgroup functionality with incomplete triples and complex nested hierarchies. New tests: - test_netgroup__incomplete_triples: Verify netgroups with various incomplete triple combinations (empty, only host, only user, only domain, missing host, missing user, missing domain) - test_netgroups__complex_hierarchy: Verify netgroups with multiple levels of nesting (base -> mid -> top) work correctly and return the expected combination of direct triples and inherited members Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com> Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com> Reviewed-by: Jakub Vávra <jvavra@redhat.com>
|
Failure in |
Adds two new netgroup tests.
test_netgroup__incomplete_triples - Verifies netgroups with missing host/user/domain fields work correctly
test_netgroups__complex_hierarchy - Verifies multi-level nested netgroups resolve inherited members correctly