C0urante commented on code in PR #12791:
URL: https://github.com/apache/kafka/pull/12791#discussion_r1006666618
##########
connect/runtime/src/test/java/org/apache/kafka/connect/util/RetryUtilTest.java:
##########
@@ -50,16 +52,15 @@ public void setUp() throws Exception {
@Test
public void testSuccess() throws Exception {
Mockito.when(mockCallable.call()).thenReturn("success");
- assertEquals("success", RetryUtil.retryUntilTimeout(mockCallable,
testMsg, Duration.ofMillis(100), 1));
+ assertEquals("success", RetryUtil.retryUntilTimeout(mockCallable,
testMsg, Duration.ofMillis(100), 1, mockTime));
Mockito.verify(mockCallable, Mockito.times(1)).call();
}
- // timeout the test after 1000ms if unable to complete within a reasonable
time frame
- @Test(timeout = 1000)
+ @Test
public void testExhaustingRetries() throws Exception {
Mockito.when(mockCallable.call()).thenThrow(new TimeoutException());
ConnectException e = assertThrows(ConnectException.class,
Review Comment:
Nit: while we're in the neighborhood, can we remove this unnecessary
variable? Can just change the line to:
```java
assertThrows(ConnectException.class,
```
##########
connect/runtime/src/test/java/org/apache/kafka/connect/util/RetryUtilTest.java:
##########
@@ -116,16 +117,20 @@ public void testNoBackoffTimeAndSucceed() throws
Exception {
.thenThrow(new TimeoutException())
.thenReturn("success");
- assertEquals("success", RetryUtil.retryUntilTimeout(mockCallable,
testMsg, TIMEOUT, 0));
+ assertEquals("success", RetryUtil.retryUntilTimeout(mockCallable,
testMsg, Duration.ofMillis(100), 0, mockTime));
Mockito.verify(mockCallable, Mockito.times(4)).call();
}
@Test
public void testNoBackoffTimeAndFail() throws Exception {
- Mockito.when(mockCallable.call()).thenThrow(new
TimeoutException("timeout exception"));
+ Mockito.when(mockCallable.call()).thenAnswer(invocation -> {
+ // Without any backoff time, the speed of the operation itself
limits the number of retries and retry rate.
+ mockTime.sleep(30);
+ throw new TimeoutException("timeout exception");
+ });
Review Comment:
We can use the auto-tick feature of the `MockTime` class instead of adding
in fake calls to `sleep` from our mock object.
```suggestion
// Without backoff time, we never sleep; auto-tick helps move things
along
mockTime = new MockTime(10);
Mockito.when(mockCallable.call()).thenThrow(new
TimeoutException("timeout exception"));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
