From e73740dbdb21d7c28908554fe3725504dc8ce84b Mon Sep 17 00:00:00 2001 From: Blake Li Date: Wed, 6 Nov 2024 11:53:23 -0500 Subject: [PATCH] fix: Fix flaky test ScheduledRetryingExecutorTest.testCancelOuterFutureAfterStart (#3335) The test was flaky because the [Thread.sleep(150)](https://github.com/googleapis/sdk-platform-java/blob/b031b18a9bb35b77ca21d3665217aa4c219ced57/gax-java/gax/src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java#L288) does not guarantee the main thread would wake up after exactly 150ms, it might take much longer if the resource is limited on the machine or we are running tests in parallel. Hence if the thread sleeps more than [1525ms](https://github.com/googleapis/sdk-platform-java/blob/0cddadb8ad3eddfffa356a479964d8a720937503/gax-java/gax/src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java#L262), the future would be completed already, and the cancellation of the future would result in false. Update the total request duration to 32500ms now, it is still not bullet proof but we would have much less chance to run into flaky tests now. fixes: https://github.com/googleapis/sdk-platform-java/issues/2669 --- .../gax/retrying/ScheduledRetryingExecutorTest.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/gax-java/gax/src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java b/gax-java/gax/src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java index 53c1707290..5a65895ef0 100644 --- a/gax-java/gax/src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java +++ b/gax-java/gax/src/test/java/com/google/api/gax/retrying/ScheduledRetryingExecutorTest.java @@ -195,13 +195,13 @@ void testCancelGetAttempt(boolean withCustomRetrySettings) throws Exception { setUp(withCustomRetrySettings); for (int executionsCount = 0; executionsCount < EXECUTIONS_COUNT; executionsCount++) { ScheduledExecutorService localExecutor = Executors.newSingleThreadScheduledExecutor(); - final int maxRetries = 100; + final int maxRetries = 20; FailingCallable callable = new FailingCallable(maxRetries - 1, "request", "SUCCESS", tracer); RetrySettings retrySettings = FAST_RETRY_SETTINGS .toBuilder() - .setTotalTimeoutDuration(java.time.Duration.ofMillis(1000L)) + .setTotalTimeoutDuration(java.time.Duration.ofMillis(5000L)) .setMaxAttempts(maxRetries) .build(); @@ -259,10 +259,11 @@ void testCancelOuterFutureAfterStart() throws Exception { .toBuilder() // These params were selected to ensure that future tries to run and fail (at least // once) but does not complete before it is cancelled. Assuming no computation time, - // it would take 25 + 100 + 400 + 1000 = 1525ms for the future to complete, which should + // it would take 2500 + 10000 + 10000 + 10000 = 32500ms for the future to complete, + // which should // be more than enough time to cancel the future. - .setInitialRetryDelayDuration(java.time.Duration.ofMillis(25L)) - .setMaxRetryDelayDuration(java.time.Duration.ofMillis(1000L)) + .setInitialRetryDelayDuration(java.time.Duration.ofMillis(2500L)) + .setMaxRetryDelayDuration(java.time.Duration.ofMillis(10000L)) .setRetryDelayMultiplier(4.0) .setTotalTimeoutDuration(java.time.Duration.ofMillis(60000L)) // Set this test to not use jitter as the randomized retry delay (RRD) may introduce