Skip to content

tests: poll for KCM TGT renewal instead of fixed sleep#8650

Merged
alexey-tikhonov merged 1 commit into
SSSD:masterfrom
madhuriupadhye:kcm_max
Apr 28, 2026
Merged

tests: poll for KCM TGT renewal instead of fixed sleep#8650
alexey-tikhonov merged 1 commit into
SSSD:masterfrom
madhuriupadhye:kcm_max

Conversation

@madhuriupadhye

@madhuriupadhye madhuriupadhye commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

test_kcm__tgt_renewal_updates_ticket_as_configured no longer uses a fixed sleep(5) and flaky retries.
The test now polls klist every 0.5s and asserts renewal by checking that TGT start or end time advances, with a detailed failure message that prints both initial and last timestamps.

To match KCM renewal behavior (renewal only starts after roughly half the ticket lifetime), the test uses a short renewable ticket (-r 5s -l 5s) and a bounded polling window (9s) so it stays fast while still waiting long enough for renewal to be attempted.
Also removed the temporary CI comment and the flaky marker from this test.

@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 updates the pytest.mark.flaky decorator in src/tests/system/tests/test_kcm.py by changing the parameter name from reruns to max_runs. No review comments were provided for this change, and I have no feedback to provide.

Comment thread src/tests/system/tests/test_kcm.py Outdated
Comment thread src/tests/system/tests/test_kcm.py Outdated
@madhuriupadhye
madhuriupadhye force-pushed the kcm_max branch 8 times, most recently from 597d6a7 to f1a98af Compare April 28, 2026 10:13
@madhuriupadhye madhuriupadhye changed the title tests: fix KCM flaky mark — use max_runs tests: poll for KCM TGT renewal instead of fixed sleep Apr 28, 2026
@spoore1
spoore1 self-requested a review April 28, 2026 12:52
@spoore1 spoore1 self-assigned this Apr 28, 2026

@spoore1 spoore1 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

test_kcm__tgt_renewal_updates_ticket_as_configured
no longer uses a fixed sleep(5) and flaky retries.
The test now polls klist every 0.5s and asserts renewal
by checking that TGT start or end time advances,
with a detailed failure message that prints both
initial and last timestamps.

To match KCM renewal behavior (renewal only starts
after roughly half the ticket lifetime),
the test uses a short renewable ticket (-r 5s -l 5s) and
a bounded polling window (9s) so it stays fast while still
waiting long enough for renewal to be attempted.
Also removed the temporary CI comment and the flaky marker from this test.

Assited by: Cursor(Claude Opus 4.6)

Signed-off-by: Madhuri Upadhye <[email protected]>
Reviewed-by: Alexey Tikhonov <[email protected]>
Reviewed-by: Scott Poore <[email protected]>
@sssd-bot

Copy link
Copy Markdown
Contributor

The pull request was accepted by @spoore1 with the following PR CI status:


🟢 CodeQL (success)
🟢 osh-diff-scan:fedora-rawhide-x86_64:upstream (success)
🟢 rpm-build:centos-stream-10-x86_64:upstream (success)
🟢 rpm-build:fedora-42-x86_64:upstream (success)
🟢 rpm-build:fedora-43-x86_64:upstream (success)
🟢 rpm-build:fedora-44-x86_64:upstream (success)
🟢 rpm-build:fedora-rawhide-x86_64:upstream (success)
🟢 Analyze (target) / cppcheck (success)
🟢 ci / intgcheck (centos-10) (success)
🟢 ci / intgcheck (fedora-42) (success)
🟢 ci / intgcheck (fedora-43) (success)
🟢 ci / intgcheck (fedora-44) (success)
🟢 ci / intgcheck (fedora-45) (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🟢 ci / system (fedora-44) (success)
🟢 ci / system (fedora-45) (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