-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Description
Is your feature request related to a problem? Please describe
Using a fixed sleep in tests is a great way to introduce a flaky test. It's almost always better to poll for the condition you're waiting for rather than waiting a fixed amount of time. Here are a handful of flaky tests from the past that were caused by fixed sleeps:
- [AUTOCUT] Gradle Check Flaky Test Report for TelemetryMetricsEnabledSanityIT #19422
- [AUTOCUT] Gradle Check Flaky Test Report for IndicesRequestCacheIT #14288
- [AUTOCUT] Gradle Check Flaky Test Report for AutoForceMergeManagerTests #18820
This list is certainly not exhaustive.
Describe the solution you'd like
Use forbidden APIs to forbid the usage of Thread#sleep in test code. There are certainly cases where sleeping will still be needed so this can be overridden with the @SuppressForbidden annotation, but making the default behavior to fail the build will make it clear to contributors and reviewers that any usage of sleep needs scrutiny.
Additional context
Because usage of Thread.sleep is currently quite pervasive, my rollout plan for this is to chip away one at a time by making it an optional property that can be turned on one test suite at a time. For example, the following can be added to server/build.gradle to enable this for internal cluster tests:
tasks.named('forbiddenApisInternalClusterTest').configure { forbidSleep() }
This will avoid a huge PR to annotate or remove hundreds of usages across all the projects. Once we reach a critical mass we can switch the default in this repo.