updated kcm flaky test #8508
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request attempts to fix a flaky test by increasing a timeout. However, the review identifies a fundamental logic flaw in the test itself. The ticket lifetime is shorter than the sleep duration, causing the ticket to expire before the test can check for renewal. The proposed change exacerbates this issue. A recommendation is made to adjust the ticket lifetime parameters to ensure the test correctly validates the renewal functionality.
|
Updated the renewable times to match Gemini's suggestions. |
danlavu
added
Waiting for review
Trivial
spoore1
left a comment
spoore1
left a comment
There was a problem hiding this comment.
LGTM. Fixing this test should resolve one of the failures we've seen recently.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request attempts to fix a flaky test by increasing a time.sleep() duration. While this might reduce the flakiness, a more robust solution is to replace the fixed sleep with a polling mechanism that actively waits for the asynchronous operation to complete. I've provided a suggestion to implement this, which will make the test more reliable.
| time.sleep(5) | ||
| renew_start, _ = krb.list_tgt_times(kdc.realm) |
There was a problem hiding this comment.
Using a fixed time.sleep() to wait for an asynchronous operation like ticket renewal can lead to flaky tests. The operation might take longer than the sleep duration on slower systems or under high load, causing the test to fail intermittently.
A more robust approach is to actively poll for the expected condition (the ticket being renewed) with a reasonable timeout. This makes the test more reliable as it waits for the renewal to actually happen.
renew_start = init_start
for _ in range(10):
try:
current_start, _ = krb.list_tgt_times(kdc.realm)
if init_start < current_start:
renew_start = current_start
break
except ProcessError:
pass
time.sleep(0.5)
It's a symptomatic fix. Ok, let's merge it, but chances are it will re-appear. |
Reviewed-by: Scott Poore <[email protected]>
4 participants
No description provided.