[Umbrella Issue] Many tests use Thread.sleep instead of synchronization primitives

[brendandburns]

Many of our tests simply have a Thread.sleep(duration) when testing multi-threaded code. This is inherently flaky.

Instead we should use synchronization classes (generally Semaphore) to synchronized the tests which is both faster in the general case and not flaky in the extreme case.

See for instance #1218 and #1219

[brendandburns]

• util/src/test/java/io/kubernetes/client/informer/cache/ReflectorRunnableTest.java
• extended/src/test/java/io/kubernetes/client/extended/workqueue/DefaultDelayingQueueTest.java
• util/src/test/java/io/kubernetes/client/informer/cache/ProcessorListenerTest.java
• util/src/test/java/io/kubernetes/client/informer/cache/SharedProcessorTest.java
• util/src/test/java/io/kubernetes/client/CopyTest.java
• util/src/test/java/io/kubernetes/client/informer/cache/ControllerTest.java
• extended/src/test/java/io/kubernetes/client/extended/workqueue/DefaultWorkQueueTest.java
• extended/src/test/java/io/kubernetes/client/extended/leaderelection/LeaderElectionTest.java
• util/src/test/java/io/kubernetes/client/informer/impl/DefaultSharedIndexInformerTest.java
• extended/src/test/java/io/kubernetes/client/extended/controller/LeaderElectingControllerTest.java
• util/src/test/java/io/kubernetes/client/PortForwardTest.java
• util/src/test/java/io/kubernetes/client/AttachTest.java
• util/src/test/java/io/kubernetes/client/ExecTest.java
• extended/src/test/java/io/kubernetes/client/extended/workqueue/ratelimiter/BucketRateLimiterTest.java
• spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesInformerCreatorTest.java
• spring/src/test/java/io/kubernetes/client/spring/extended/controller/KubernetesReconcilerCreatorTest.java
• extended/src/test/java/io/kubernetes/client/extended/event/EventCorrelatorTest.java

[yue9944882]

@brendandburns thanks for sorting out these potentially flaking tests.. will fix them later!

/assign

[sarveshkaushal]

I would like to help :-)

[brendandburns]

@sarveshkaushal we'd welcome the help. In most cases the Thread.sleep(foo) is to wait for some other thread to complete it's work. Instead, we should create a Semaphore, then acquire() one or more permits for the executions we're waiting for, and then release() them in the Thread, then attempt to acquire() in the test thread to implement the wait.

See an example PR #1219

The reason that Thread.sleep() is flaky is because in a CI/CD system like GitHub actions the machines can be massively overloaded, which leads to much longer delays than you might otherwise see when running tests, so the sleep(..) ends up not being long enough. You can always make the sleep longer, but that makes the tests run longer (and it's still flaky in extreme cases) so using synchronization primitives is (almost) always the right answer.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[brendandburns]

/lifecycle frozen

[Priya103]

If still available I would like to contribute