mdedetrich commented on code in PR #12725:
URL: https://github.com/apache/kafka/pull/12725#discussion_r1003516803
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResourceTest.java:
##########
@@ -899,250 +750,199 @@ public void
testFenceZombiesWithInternalRequestSignature() throws Throwable {
expectedSignature,
signatureCapture.getValue()
);
-
- PowerMock.verifyAll();
}
@Test
public void testFenceZombiesConnectorNotFound() throws Throwable {
- final Capture<Callback<Void>> cb = Capture.newInstance();
- herder.fenceZombieSourceTasks(EasyMock.eq(CONNECTOR_NAME),
EasyMock.capture(cb), EasyMock.anyObject(InternalRequestSignature.class));
-
- expectAndCallbackException(cb, new NotFoundException("not found"));
+ final ArgumentCaptor<Callback<Void>> cb =
ArgumentCaptor.forClass(Callback.class);
- PowerMock.replayAll();
+ expectAndCallbackException(cb, new NotFoundException("not found"))
+ .when(herder).fenceZombieSourceTasks(eq(CONNECTOR_NAME),
cb.capture(), any());
assertThrows(NotFoundException.class,
() -> connectorsResource.fenceZombies(CONNECTOR_NAME,
NULL_HEADERS, FORWARD, serializeAsBytes(null)));
-
- PowerMock.verifyAll();
}
@Test
public void testRestartConnectorNotFound() {
- final Capture<Callback<Void>> cb = Capture.newInstance();
- herder.restartConnector(EasyMock.eq(CONNECTOR_NAME),
EasyMock.capture(cb));
- expectAndCallbackException(cb, new NotFoundException("not found"));
-
- PowerMock.replayAll();
+ final ArgumentCaptor<Callback<Void>> cb =
ArgumentCaptor.forClass(Callback.class);
+ expectAndCallbackException(cb, new NotFoundException("not found"))
+ .when(herder).restartConnector(eq(CONNECTOR_NAME), cb.capture());
assertThrows(NotFoundException.class, () ->
connectorsResource.restartConnector(CONNECTOR_NAME,
NULL_HEADERS, false, false, FORWARD)
);
-
- PowerMock.verifyAll();
}
@Test
public void testRestartConnectorLeaderRedirect() throws Throwable {
- final Capture<Callback<Void>> cb = Capture.newInstance();
- herder.restartConnector(EasyMock.eq(CONNECTOR_NAME),
EasyMock.capture(cb));
- expectAndCallbackNotLeaderException(cb);
+ final ArgumentCaptor<Callback<Void>> cb =
ArgumentCaptor.forClass(Callback.class);
+ expectAndCallbackNotLeaderException(cb).when(herder)
+ .restartConnector(eq(CONNECTOR_NAME), cb.capture());
- EasyMock.expect(RestClient.httpRequest(EasyMock.eq(LEADER_URL +
"connectors/" + CONNECTOR_NAME + "/restart?forward=true"),
- EasyMock.eq("POST"), EasyMock.isNull(), EasyMock.isNull(),
EasyMock.anyObject(), EasyMock.anyObject(WorkerConfig.class)))
- .andReturn(new RestClient.HttpResponse<>(202, new HashMap<>(),
null));
-
- PowerMock.replayAll();
+ restClientStatic.when(() ->
+ RestClient.httpRequest(eq(LEADER_URL + "connectors/" +
CONNECTOR_NAME + "/restart?forward=true"),
+ eq("POST"), isNull(), isNull(), any(), any(WorkerConfig.class))
+ ).thenReturn(new RestClient.HttpResponse<>(202, new HashMap<>(),
null));
Response response =
connectorsResource.restartConnector(CONNECTOR_NAME, NULL_HEADERS, false, false,
null);
assertEquals(Response.Status.NO_CONTENT.getStatusCode(),
response.getStatus());
- PowerMock.verifyAll();
-
- PowerMock.verifyAll();
}
@Test
public void testRestartConnectorOwnerRedirect() throws Throwable {
- final Capture<Callback<Void>> cb = Capture.newInstance();
- herder.restartConnector(EasyMock.eq(CONNECTOR_NAME),
EasyMock.capture(cb));
+ final ArgumentCaptor<Callback<Void>> cb =
ArgumentCaptor.forClass(Callback.class);
String ownerUrl = "http://owner:8083";;
- expectAndCallbackException(cb, new NotAssignedException("not owner
test", ownerUrl));
-
-
EasyMock.expect(RestClient.httpRequest(EasyMock.eq("http://owner:8083/connectors/";
+ CONNECTOR_NAME + "/restart?forward=false"),
- EasyMock.eq("POST"), EasyMock.isNull(), EasyMock.isNull(),
EasyMock.anyObject(), EasyMock.anyObject(WorkerConfig.class)))
- .andReturn(new RestClient.HttpResponse<>(202, new HashMap<>(),
null));
+ expectAndCallbackException(cb, new NotAssignedException("not owner
test", ownerUrl))
+ .when(herder).restartConnector(eq(CONNECTOR_NAME), cb.capture());
- PowerMock.replayAll();
+ restClientStatic.when(() ->
+ RestClient.httpRequest(eq("http://owner:8083/connectors/"; +
CONNECTOR_NAME + "/restart?forward=false"),
+ eq("POST"), isNull(), isNull(), any(), any(WorkerConfig.class))
+ ).thenReturn(new RestClient.HttpResponse<>(202, new HashMap<>(),
null));
Response response =
connectorsResource.restartConnector(CONNECTOR_NAME, NULL_HEADERS, false, false,
true);
assertEquals(Response.Status.NO_CONTENT.getStatusCode(),
response.getStatus());
- PowerMock.verifyAll();
-
- PowerMock.verifyAll();
}
@Test
public void testRestartTaskNotFound() {
ConnectorTaskId taskId = new ConnectorTaskId(CONNECTOR_NAME, 0);
- final Capture<Callback<Void>> cb = Capture.newInstance();
- herder.restartTask(EasyMock.eq(taskId), EasyMock.capture(cb));
- expectAndCallbackException(cb, new NotFoundException("not found"));
-
- PowerMock.replayAll();
+ final ArgumentCaptor<Callback<Void>> cb =
ArgumentCaptor.forClass(Callback.class);
+ expectAndCallbackException(cb, new NotFoundException("not found"))
+ .when(herder).restartTask(eq(taskId), cb.capture());
assertThrows(NotFoundException.class, () ->
connectorsResource.restartTask(CONNECTOR_NAME, 0, NULL_HEADERS, FORWARD));
-
- PowerMock.verifyAll();
}
@Test
public void testRestartTaskLeaderRedirect() throws Throwable {
ConnectorTaskId taskId = new ConnectorTaskId(CONNECTOR_NAME, 0);
- final Capture<Callback<Void>> cb = Capture.newInstance();
- herder.restartTask(EasyMock.eq(taskId), EasyMock.capture(cb));
- expectAndCallbackNotLeaderException(cb);
+ final ArgumentCaptor<Callback<Void>> cb =
ArgumentCaptor.forClass(Callback.class);
+ expectAndCallbackNotLeaderException(cb).when(herder)
+ .restartTask(eq(taskId), cb.capture());
- EasyMock.expect(RestClient.httpRequest(EasyMock.eq(LEADER_URL +
"connectors/" + CONNECTOR_NAME + "/tasks/0/restart?forward=true"),
- EasyMock.eq("POST"), EasyMock.isNull(), EasyMock.isNull(),
EasyMock.anyObject(), EasyMock.anyObject(WorkerConfig.class)))
- .andReturn(new RestClient.HttpResponse<>(202, new HashMap<>(),
null));
-
- PowerMock.replayAll();
+ restClientStatic.when(() ->
+ RestClient.httpRequest(eq(LEADER_URL + "connectors/" +
CONNECTOR_NAME + "/tasks/0/restart?forward=true"),
+ eq("POST"), isNull(), isNull(), any(), any(WorkerConfig.class))
+ ).thenReturn(new RestClient.HttpResponse<>(202, new HashMap<>(),
null));
connectorsResource.restartTask(CONNECTOR_NAME, 0, NULL_HEADERS, null);
-
- PowerMock.verifyAll();
}
@Test
public void testRestartTaskOwnerRedirect() throws Throwable {
ConnectorTaskId taskId = new ConnectorTaskId(CONNECTOR_NAME, 0);
- final Capture<Callback<Void>> cb = Capture.newInstance();
- herder.restartTask(EasyMock.eq(taskId), EasyMock.capture(cb));
+ final ArgumentCaptor<Callback<Void>> cb =
ArgumentCaptor.forClass(Callback.class);
String ownerUrl = "http://owner:8083";;
- expectAndCallbackException(cb, new NotAssignedException("not owner
test", ownerUrl));
-
-
EasyMock.expect(RestClient.httpRequest(EasyMock.eq("http://owner:8083/connectors/";
+ CONNECTOR_NAME + "/tasks/0/restart?forward=false"),
- EasyMock.eq("POST"), EasyMock.isNull(), EasyMock.isNull(),
EasyMock.anyObject(), EasyMock.anyObject(WorkerConfig.class)))
- .andReturn(new RestClient.HttpResponse<>(202, new HashMap<>(),
null));
+ expectAndCallbackException(cb, new NotAssignedException("not owner
test", ownerUrl))
+ .when(herder).restartTask(eq(taskId), cb.capture());
- PowerMock.replayAll();
+ restClientStatic.when(() ->
+ RestClient.httpRequest(eq("http://owner:8083/connectors/"; +
CONNECTOR_NAME + "/tasks/0/restart?forward=false"),
+ eq("POST"), isNull(), isNull(), any(), any(WorkerConfig.class))
+ ).thenReturn(new RestClient.HttpResponse<>(202, new HashMap<>(),
null));
connectorsResource.restartTask(CONNECTOR_NAME, 0, NULL_HEADERS, true);
-
- PowerMock.verifyAll();
}
@Test
public void testConnectorActiveTopicsWithTopicTrackingDisabled() {
- PowerMock.reset(workerConfig);
-
EasyMock.expect(workerConfig.getBoolean(TOPIC_TRACKING_ENABLE_CONFIG)).andReturn(false);
-
EasyMock.expect(workerConfig.getBoolean(TOPIC_TRACKING_ALLOW_RESET_CONFIG)).andReturn(false);
- PowerMock.replay(workerConfig);
+
when(workerConfig.getBoolean(TOPIC_TRACKING_ENABLE_CONFIG)).thenReturn(false);
+
when(workerConfig.getBoolean(TOPIC_TRACKING_ALLOW_RESET_CONFIG)).thenReturn(false);
connectorsResource = new ConnectorsResource(herder, workerConfig);
- PowerMock.replayAll();
Exception e = assertThrows(ConnectRestException.class,
() -> connectorsResource.getConnectorActiveTopics(CONNECTOR_NAME));
assertEquals("Topic tracking is disabled.", e.getMessage());
- PowerMock.verifyAll();
}
@Test
public void testResetConnectorActiveTopicsWithTopicTrackingDisabled() {
- PowerMock.reset(workerConfig);
-
EasyMock.expect(workerConfig.getBoolean(TOPIC_TRACKING_ENABLE_CONFIG)).andReturn(false);
-
EasyMock.expect(workerConfig.getBoolean(TOPIC_TRACKING_ALLOW_RESET_CONFIG)).andReturn(true);
- HttpHeaders headers = EasyMock.mock(HttpHeaders.class);
- PowerMock.replay(workerConfig);
+
when(workerConfig.getBoolean(TOPIC_TRACKING_ENABLE_CONFIG)).thenReturn(false);
+
when(workerConfig.getBoolean(TOPIC_TRACKING_ALLOW_RESET_CONFIG)).thenReturn(true);
+ HttpHeaders headers = mock(HttpHeaders.class);
connectorsResource = new ConnectorsResource(herder, workerConfig);
- PowerMock.replayAll();
Exception e = assertThrows(ConnectRestException.class,
() ->
connectorsResource.resetConnectorActiveTopics(CONNECTOR_NAME, headers));
assertEquals("Topic tracking is disabled.", e.getMessage());
- PowerMock.verifyAll();
}
@Test
public void testResetConnectorActiveTopicsWithTopicTrackingEnabled() {
- PowerMock.reset(workerConfig);
-
EasyMock.expect(workerConfig.getBoolean(TOPIC_TRACKING_ENABLE_CONFIG)).andReturn(true);
-
EasyMock.expect(workerConfig.getBoolean(TOPIC_TRACKING_ALLOW_RESET_CONFIG)).andReturn(false);
- HttpHeaders headers = EasyMock.mock(HttpHeaders.class);
- PowerMock.replay(workerConfig);
+
when(workerConfig.getBoolean(TOPIC_TRACKING_ENABLE_CONFIG)).thenReturn(true);
+
when(workerConfig.getBoolean(TOPIC_TRACKING_ALLOW_RESET_CONFIG)).thenReturn(false);
+ HttpHeaders headers = mock(HttpHeaders.class);
connectorsResource = new ConnectorsResource(herder, workerConfig);
- PowerMock.replayAll();
Exception e = assertThrows(ConnectRestException.class,
() ->
connectorsResource.resetConnectorActiveTopics(CONNECTOR_NAME, headers));
assertEquals("Topic tracking reset is disabled.", e.getMessage());
- PowerMock.verifyAll();
}
@Test
public void testConnectorActiveTopics() {
- PowerMock.reset(workerConfig);
-
EasyMock.expect(workerConfig.getBoolean(TOPIC_TRACKING_ENABLE_CONFIG)).andReturn(true);
-
EasyMock.expect(workerConfig.getBoolean(TOPIC_TRACKING_ALLOW_RESET_CONFIG)).andReturn(true);
- EasyMock.expect(herder.connectorActiveTopics(CONNECTOR_NAME))
- .andReturn(new ActiveTopicsInfo(CONNECTOR_NAME,
CONNECTOR_ACTIVE_TOPICS));
- PowerMock.replay(workerConfig);
+
when(workerConfig.getBoolean(TOPIC_TRACKING_ENABLE_CONFIG)).thenReturn(true);
+
when(workerConfig.getBoolean(TOPIC_TRACKING_ALLOW_RESET_CONFIG)).thenReturn(true);
+ when(herder.connectorActiveTopics(CONNECTOR_NAME))
+ .thenReturn(new ActiveTopicsInfo(CONNECTOR_NAME,
CONNECTOR_ACTIVE_TOPICS));
connectorsResource = new ConnectorsResource(herder, workerConfig);
- PowerMock.replayAll();
Response response =
connectorsResource.getConnectorActiveTopics(CONNECTOR_NAME);
assertEquals(Response.Status.OK.getStatusCode(), response.getStatus());
Map<String, Map<String, Object>> body = (Map<String, Map<String,
Object>>) response.getEntity();
assertEquals(CONNECTOR_NAME, ((ActiveTopicsInfo)
body.get(CONNECTOR_NAME)).connector());
assertEquals(new HashSet<>(CONNECTOR_ACTIVE_TOPICS),
((ActiveTopicsInfo) body.get(CONNECTOR_NAME)).topics());
- PowerMock.verifyAll();
}
@Test
public void testResetConnectorActiveTopics() {
- PowerMock.reset(workerConfig);
-
EasyMock.expect(workerConfig.getBoolean(TOPIC_TRACKING_ENABLE_CONFIG)).andReturn(true);
-
EasyMock.expect(workerConfig.getBoolean(TOPIC_TRACKING_ALLOW_RESET_CONFIG)).andReturn(true);
- HttpHeaders headers = EasyMock.mock(HttpHeaders.class);
+
when(workerConfig.getBoolean(TOPIC_TRACKING_ENABLE_CONFIG)).thenReturn(true);
+
when(workerConfig.getBoolean(TOPIC_TRACKING_ALLOW_RESET_CONFIG)).thenReturn(true);
+ HttpHeaders headers = mock(HttpHeaders.class);
herder.resetConnectorActiveTopics(CONNECTOR_NAME);
- EasyMock.expectLastCall();
- PowerMock.replay(workerConfig);
+ verify(herder).resetConnectorActiveTopics(CONNECTOR_NAME);
Review Comment:
This change ends up failing the test with
```
herder.resetConnectorActiveTopics("test");
Wanted 1 time:
-> at
org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest.testResetConnectorActiveTopics(ConnectorsResourceTest.java:912)
But was 2 times:
-> at
org.apache.kafka.connect.runtime.rest.resources.ConnectorsResourceTest.testResetConnectorActiveTopics(ConnectorsResourceTest.java:908)
-> at
org.apache.kafka.connect.runtime.rest.resources.ConnectorsResource.resetConnectorActiveTopics(ConnectorsResource.java:234)
```
Should I change `verify(herder).resetConnectorActiveTopics(CONNECTOR_NAME);`
to use `atLeastOnce()`?, i.e.
```java
verify(herder, atLeastOnce).resetConnectorActiveTopics(CONNECTOR_NAME);
```
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
