[autobackport: sssd-2-10] Tests: Add integration tests validating SSSD socket#8562
[autobackport: sssd-2-10] Tests: Add integration tests validating SSSD socket#8562sssd-bot wants to merge 1 commit intoSSSD:sssd-2-10from
Conversation
Add integration tests validating SSSD socket activation behavior for individual responders and mixed socket/traditional configurations. Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> Reviewed-by: Jakub Vávra <jvavra@redhat.com> (cherry picked from commit abee6e7)
There was a problem hiding this comment.
Code Review
This pull request introduces a new suite of system tests for SSSD socket activation, covering responder lifecycles, autofs support, mixed-mode configurations, and configuration conflicts. The review feedback correctly identifies that while the test docstrings claim to verify that services are initially inactive, the actual implementation lacks these assertions. Actionable suggestions were provided to add checks ensuring service units are not running before the first client request is made.
| socket_unit = f"sssd-{responder}.socket" | ||
| service_unit = f"sssd-{responder}.service" | ||
|
|
||
| assert client.sssd.svc.is_active(socket_unit), f"{responder} socket should be active" |
There was a problem hiding this comment.
The test's docstring states that it verifies that the service unit is inactive initially. However, this crucial assertion is missing. To properly test the socket activation lifecycle, please add a check to confirm the service is not running before the first client request is made.
| assert client.sssd.svc.is_active(socket_unit), f"{responder} socket should be active" | |
| assert client.sssd.svc.is_active(socket_unit), f"{responder} socket should be active" | |
| assert not client.sssd.svc.is_active(service_unit), f"{responder} service should be inactive initially" |
| socket_unit = f"sssd-{responder}.socket" | ||
| service_unit = f"sssd-{responder}.service" | ||
|
|
||
| assert client.sssd.svc.is_active(socket_unit), f"{responder} socket should be active" |
There was a problem hiding this comment.
Similar to the other lifecycle tests, this one is missing an assertion to verify that the autofs service unit is inactive before the first request. The test's docstring indicates this check should be performed. Please add it to ensure the test correctly validates the socket activation behavior.
| assert client.sssd.svc.is_active(socket_unit), f"{responder} socket should be active" | |
| assert client.sssd.svc.is_active(socket_unit), f"{responder} socket should be active" | |
| assert not client.sssd.svc.is_active(service_unit), f"{responder} service should be inactive initially" |
| socket_unit = f"sssd-{socket_responder}.socket" | ||
| socket_service = f"sssd-{socket_responder}.service" | ||
|
|
||
| assert client.sssd.svc.is_active(socket_unit), f"{socket_responder} socket should be active" |
There was a problem hiding this comment.
This test for mixed-mode configuration is missing a key assertion. The docstring mentions verifying that the socket-activated responder is initially inactive, but the code doesn't check this. Please add an assertion to verify the service is not active before the request is triggered.
| assert client.sssd.svc.is_active(socket_unit), f"{socket_responder} socket should be active" | |
| assert client.sssd.svc.is_active(socket_unit), f"{socket_responder} socket should be active" | |
| assert not client.sssd.svc.is_active(socket_service), f"{socket_responder} service should be inactive initially" |
This is an automatic backport of PR#8481 Tests: Add integration tests validating SSSD socket to branch sssd-2-10, created by @aborah-sudo.
Please make sure this backport is correct.
Note
The commits were cherry-picked without conflicts.
You can push changes to this pull request
Original commits
abee6e7 - Tests: Add integration tests validating SSSD socket
Backported commits
Original Pull Request Body
Add integration tests validating SSSD socket activation behavior for individual responders and mixed socket/traditional configurations.