Skip to content

Tests: Fix ipa multihost test_authentication_indicators#8525

Merged
jakub-vavra-cz merged 1 commit intoSSSD:masterfrom
aborah-sudo:indicator
Mar 17, 2026
Merged

Tests: Fix ipa multihost test_authentication_indicators#8525
jakub-vavra-cz merged 1 commit intoSSSD:masterfrom
aborah-sudo:indicator

Conversation

@aborah-sudo
Copy link
Contributor

Provide sleep time to test

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

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 fixes a test by adding a time.sleep() to wait for a sudo rule to propagate. While this can fix the immediate issue, using fixed sleeps in tests is an anti-pattern that can lead to either flaky or slow tests. I've suggested replacing the sleep with a more robust polling mechanism that retries the operation until it succeeds or a timeout is reached. This will make the test more reliable and efficient.

Copy link
Contributor

@jakub-vavra-cz jakub-vavra-cz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Provide sleep time to test

Reviewed-by: Jakub Vávra <jvavra@redhat.com>
@sssd-bot
Copy link
Contributor

The pull request was accepted by @jakub-vavra-cz 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)
🟢 Build / freebsd (success)
🟢 Build / make-distcheck (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) (failure)
➖ 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.

3 participants