Repository navigation
Add AD certificate authentication test - #8453
krishnavema wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a new system test for AD certificate authentication using a virtual smartcard. The changes include updating a test dependency and adding the new test case with a helper function for certificate enrollment. The overall approach is sound, but I've identified one area for improvement to increase test reliability by replacing a fixed sleep with a more robust polling mechanism.
| client.sssd.restart() | ||
|
|
||
| client.svc.restart("virt_cacard.service") | ||
| time.sleep(5) |
There was a problem hiding this comment.
Using a fixed time.sleep(5) can lead to flaky tests. If the virt_cacard.service takes longer to start under load, the test might fail. Conversely, if the service starts quickly, the test is unnecessarily delayed. A more robust approach is to poll for the service's status until it becomes active.
| time.sleep(5) | |
| for _ in range(10): | |
| result = client.host.conn.run("systemctl is-active --quiet virt_cacard.service", raise_on_error=False) | |
| if result.rc == 0: | |
| break | |
| time.sleep(1) | |
| else: | |
| pytest.fail("virt_cacard.service did not become active in time.") |
b32d42d to
f9ad7bf
Compare
358d5e7 to
f9ad7bf
Compare
f9ad7bf to
2888874
Compare
2888874 to
40ef7aa
Compare
|
Hi, please rebase to get Pavel's "test: fix missing cmocka prerequisite includes in test_nfsidmap.c" included to make the CI builds pass. bye, |
No description provided.