Repository navigation
test(gax): use FakeApiClock and RecordingScheduler in ScheduledRetryingExecutorTest - #14706
Conversation
…read.sleep Replace CurrentMillisClock and real Thread.sleep delays in DirectRetryingExecutorTest with FakeApiClock and a sleep() override that advances virtual time deterministically. This prevents testMaxRetriesExceeded from flaking with premature totalTimeout expiration on slow CI runners. Fixes googleapis#14625
…elpers Use FakeApiClock and RecordingScheduler in ScheduledRetryingExecutorTest for deterministic, sleep-free retry tests without modifying existing asynchronous in-flight test methods. Refactor RecordingScheduler to extend ScheduledThreadPoolExecutor directly, eliminating Mockito dependencies and strict-stubbing friction. Add Javadoc documentation for FakeApiClock and RecordingScheduler. Fixes googleapis#13535
![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 refactors the retrying executor tests to use deterministic simulated time instead of real wall-clock sleeps, replacing the Mockito-based RecordingScheduler with a concrete ScheduledThreadPoolExecutor that advances a FakeApiClock. DirectRetryingExecutorTest and ScheduledRetryingExecutorTest are updated to use this fake clock. The feedback suggests replacing the non-thread-safe ArrayList used for sleepDurations in RecordingScheduler with a CopyOnWriteArrayList to prevent potential concurrent modification issues during multi-threaded test execution.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors RecordingScheduler from a Mockito-mocked ScheduledExecutorService to a concrete class extending ScheduledThreadPoolExecutor. This allows scheduled tasks to execute asynchronously with zero delay while advancing a FakeApiClock, eliminating real-time backoff sleeps in tests. Additionally, ScheduledRetryingExecutorTest is updated to utilize this new scheduler. The feedback recommends also overriding the Callable overload of schedule in RecordingScheduler to prevent potential test hangs or real-time delays if the underlying executor is updated to schedule Callable tasks in the future.
lqiu96
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
Fixes #13535