Skip to content

Commit 70b7438

Browse files
joviegasjoviegas
authored
Fix Flaky S3JavaMultipartTransferProgressListenerTest test case (#5918)
* Fix Flaky S3JavaMultipartTransferProgressListenerTest test case * Handled comments
1 parent 580bf8a commit 70b7438

File tree

1 file changed

+27
-19
lines changed

1 file changed

+27
-19
lines changed

‎services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/S3JavaMultipartTransferProgressListenerTest.java

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@
2525
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
2626
import static org.assertj.core.api.Assertions.assertThat;
2727
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
28+
import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
2829
import static org.mockito.Mockito.mock;
2930
import static org.mockito.Mockito.times;
3031

3132
import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
3233
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
3334
import java.io.IOException;
3435
import java.net.URI;
36+
import java.time.Duration;
3537
import java.util.concurrent.CancellationException;
3638
import java.util.concurrent.CompletionException;
3739
import org.junit.jupiter.api.BeforeAll;
@@ -95,7 +97,7 @@ private static void assertMockOnFailure(TransferListener transferListenerMock) {
9597

9698
@ParameterizedTest
9799
@ValueSource(booleans = {true, false})
98-
void listeners_reports_ErrorsWithValidPayload(boolean multipartEnabled) throws InterruptedException {
100+
void listeners_reports_ErrorsWithValidPayload(boolean multipartEnabled) {
99101
S3AsyncClient s3Async = s3AsyncClient(multipartEnabled);
100102

101103
TransferListener transferListenerMock = mock(TransferListener.class);
@@ -113,8 +115,8 @@ void listeners_reports_ErrorsWithValidPayload(boolean multipartEnabled) throws I
113115
.addTransferListener(transferListenerMock)
114116
.build());
115117

118+
assertTransferListenerCompletion(transferListener);
116119
assertThatExceptionOfType(CompletionException.class).isThrownBy(() -> fileUpload.completionFuture().join());
117-
Thread.sleep(500);
118120
assertThat(transferListener.getExceptionCaught()).isInstanceOf(NoSuchBucketException.class);
119121
assertThat(transferListener.isTransferComplete()).isFalse();
120122
assertThat(transferListener.isTransferInitiated()).isTrue();
@@ -124,7 +126,7 @@ void listeners_reports_ErrorsWithValidPayload(boolean multipartEnabled) throws I
124126

125127
@ParameterizedTest
126128
@ValueSource(booleans = {true, false})
127-
void listeners_reports_ErrorsWithValidInValidPayload(boolean multipartEnabled) throws InterruptedException {
129+
void listeners_reports_ErrorsWithValidInValidPayload(boolean multipartEnabled) {
128130
S3AsyncClient s3Async = s3AsyncClient(multipartEnabled);
129131

130132
TransferListener transferListenerMock = mock(TransferListener.class);
@@ -142,9 +144,8 @@ void listeners_reports_ErrorsWithValidInValidPayload(boolean multipartEnabled) t
142144
.addTransferListener(transferListenerMock)
143145
.build());
144146

147+
assertTransferListenerCompletion(transferListener);
145148
assertThatExceptionOfType(CompletionException.class).isThrownBy(() -> fileUpload.completionFuture().join());
146-
Thread.sleep(500);
147-
148149
assertThat(transferListener.getExceptionCaught()).isInstanceOf(S3Exception.class);
149150
assertThat(transferListener.isTransferComplete()).isFalse();
150151
assertThat(transferListener.isTransferInitiated()).isTrue();
@@ -155,7 +156,7 @@ void listeners_reports_ErrorsWithValidInValidPayload(boolean multipartEnabled) t
155156

156157
@ParameterizedTest
157158
@ValueSource(booleans = {true, false})
158-
void listeners_reports_ErrorsWhenCancelled(boolean multipartEnabled) throws InterruptedException {
159+
void listeners_reports_ErrorsWhenCancelled(boolean multipartEnabled) {
159160
S3AsyncClient s3Async = s3AsyncClient(multipartEnabled);
160161

161162
TransferListener transferListenerMock = mock(TransferListener.class);
@@ -171,9 +172,7 @@ void listeners_reports_ErrorsWhenCancelled(boolean multipartEnabled) throws Inte
171172
.addTransferListener(transferListener)
172173
.addTransferListener(transferListenerMock)
173174
.build()).completionFuture().cancel(true);
174-
175-
Thread.sleep(500);
176-
175+
assertTransferListenerCompletion(transferListener);
177176
assertThat(transferListener.getExceptionCaught()).isInstanceOf(CancellationException.class);
178177
assertThat(transferListener.isTransferComplete()).isFalse();
179178
assertThat(transferListener.isTransferInitiated()).isTrue();
@@ -182,7 +181,7 @@ void listeners_reports_ErrorsWhenCancelled(boolean multipartEnabled) throws Inte
182181

183182
@ParameterizedTest
184183
@ValueSource(booleans = {true, false})
185-
void listeners_reports_ProgressWhenSuccess(boolean multipartEnabled) throws InterruptedException {
184+
void listeners_reports_ProgressWhenSuccess(boolean multipartEnabled) {
186185
S3AsyncClient s3Async = s3AsyncClient(multipartEnabled);
187186

188187
TransferListener transferListenerMock = mock(TransferListener.class);
@@ -202,7 +201,7 @@ void listeners_reports_ProgressWhenSuccess(boolean multipartEnabled) throws Inte
202201
.addTransferListener(transferListenerMock)
203202
.build()).completionFuture().join();
204203

205-
Thread.sleep(500);
204+
assertTransferListenerCompletion(transferListener);
206205
assertThat(transferListener.getExceptionCaught()).isNull();
207206
assertThat(transferListener.isTransferComplete()).isTrue();
208207
assertThat(transferListener.isTransferInitiated()).isTrue();
@@ -215,7 +214,7 @@ void listeners_reports_ProgressWhenSuccess(boolean multipartEnabled) throws Inte
215214
}
216215

217216
@Test
218-
void copyWithJavaBasedClient_listeners_reports_ErrorsWithValidPayload() throws InterruptedException {
217+
void copyWithJavaBasedClient_listeners_reports_ErrorsWithValidPayload() {
219218
S3AsyncClient s3Async = s3AsyncClient(true);
220219

221220
TransferListener transferListenerMock = mock(TransferListener.class);
@@ -235,17 +234,16 @@ void copyWithJavaBasedClient_listeners_reports_ErrorsWithValidPayload() throws I
235234
.addTransferListener(transferListener)
236235
.addTransferListener(transferListenerMock)
237236
.build());
238-
237+
assertTransferListenerCompletion(transferListener);
239238
assertThatExceptionOfType(CompletionException.class).isThrownBy(() -> copy.completionFuture().join());
240-
Thread.sleep(500);
241239
assertThat(transferListener.getExceptionCaught()).isInstanceOf(NoSuchKeyException.class);
242240
assertThat(transferListener.isTransferComplete()).isFalse();
243241
assertThat(transferListener.isTransferInitiated()).isTrue();
244242
assertMockOnFailure(transferListenerMock);
245243
}
246244

247245
@Test
248-
void copyWithJavaBasedClient_listeners_reports_ErrorsWithValidInValidPayload() throws InterruptedException {
246+
void copyWithJavaBasedClient_listeners_reports_ErrorsWithValidInValidPayload() {
249247
S3AsyncClient s3Async = s3AsyncClient(true);
250248

251249
TransferListener transferListenerMock = mock(TransferListener.class);
@@ -266,8 +264,8 @@ void copyWithJavaBasedClient_listeners_reports_ErrorsWithValidInValidPayload() t
266264
.addTransferListener(transferListenerMock)
267265
.build());
268266

267+
assertTransferListenerCompletion(transferListener);
269268
assertThatExceptionOfType(CompletionException.class).isThrownBy(() -> copy.completionFuture().join());
270-
Thread.sleep(500);
271269
assertThat(transferListener.getExceptionCaught()).isInstanceOf(S3Exception.class);
272270
assertThat(transferListener.isTransferComplete()).isFalse();
273271
assertThat(transferListener.isTransferInitiated()).isTrue();
@@ -295,15 +293,15 @@ void copyWithJavaBasedClient_listeners_reports_ErrorsWhenCancelled() throws Inte
295293
.addTransferListener(transferListenerMock)
296294
.build()).completionFuture().cancel(true);
297295

298-
Thread.sleep(500);
296+
assertTransferListenerCompletion(transferListener);
299297
assertThat(transferListener.getExceptionCaught()).isInstanceOf(CancellationException.class);
300298
assertThat(transferListener.isTransferComplete()).isFalse();
301299
assertThat(transferListener.isTransferInitiated()).isTrue();
302300
assertMockOnFailure(transferListenerMock);
303301
}
304302

305303
@Test
306-
void copyWithJavaBasedClient_listeners_reports_ProgressWhenSuccess_copy() throws InterruptedException {
304+
void copyWithJavaBasedClient_listeners_reports_ProgressWhenSuccess_copy() {
307305
String destinationKey = "copiedObj";
308306
S3AsyncClient s3Async = s3AsyncClient(true);
309307

@@ -339,7 +337,7 @@ void copyWithJavaBasedClient_listeners_reports_ProgressWhenSuccess_copy() throws
339337
.addTransferListener(transferListenerMock)
340338
.build());
341339

342-
Thread.sleep(500);
340+
assertTransferListenerCompletion(transferListener);
343341
assertThat(transferListener.getExceptionCaught()).isNull();
344342
assertThat(transferListener.isTransferComplete()).isTrue();
345343
assertThat(transferListener.isTransferInitiated()).isTrue();
@@ -350,4 +348,14 @@ void copyWithJavaBasedClient_listeners_reports_ProgressWhenSuccess_copy() throws
350348
int numTimesBytesTransferred = 2;
351349
Mockito.verify(transferListenerMock, times(numTimesBytesTransferred)).bytesTransferred(ArgumentMatchers.any());
352350
}
351+
352+
private static void assertTransferListenerCompletion(CaptureTransferListener transferListener) {
353+
Duration waitDuration = Duration.ofSeconds(5);
354+
assertTimeoutPreemptively(
355+
waitDuration, () -> {
356+
while (!transferListener.getCompletionFuture().isDone()) {
357+
Thread.sleep(50);
358+
}
359+
}, "TransferListener future not completed even after waiting for " + waitDuration);
360+
}
353361
}

0 commit comments

Comments
 (0)