mdedetrich commented on code in PR #12725:
URL: https://github.com/apache/kafka/pull/12725#discussion_r1008075576
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorsResourceTest.java:
##########
@@ -262,127 +254,96 @@ public void testFullExpandConnectors() {
assertEquals(connectorInfo, expanded.get(CONNECTOR_NAME).get("info"));
assertEquals(connector2, expanded.get(CONNECTOR2_NAME).get("status"));
assertEquals(connector, expanded.get(CONNECTOR_NAME).get("status"));
- PowerMock.verifyAll();
}
@Test
public void testExpandConnectorsWithConnectorNotFound() {
-
EasyMock.expect(herder.connectors()).andReturn(Arrays.asList(CONNECTOR2_NAME,
CONNECTOR_NAME));
- ConnectorStateInfo connector = EasyMock.mock(ConnectorStateInfo.class);
- ConnectorStateInfo connector2 =
EasyMock.mock(ConnectorStateInfo.class);
-
EasyMock.expect(herder.connectorStatus(CONNECTOR2_NAME)).andReturn(connector2);
-
EasyMock.expect(herder.connectorStatus(CONNECTOR_NAME)).andThrow(EasyMock.mock(NotFoundException.class));
+ when(herder.connectors()).thenReturn(Arrays.asList(CONNECTOR2_NAME,
CONNECTOR_NAME));
+ ConnectorStateInfo connector = mock(ConnectorStateInfo.class);
+ ConnectorStateInfo connector2 = mock(ConnectorStateInfo.class);
+ when(herder.connectorStatus(CONNECTOR2_NAME)).thenReturn(connector2);
+
doThrow(mock(NotFoundException.class)).when(herder).connectorStatus(CONNECTOR_NAME);
- forward = EasyMock.mock(UriInfo.class);
+ forward = mock(UriInfo.class);
MultivaluedMap<String, String> queryParams = new
MultivaluedHashMap<>();
queryParams.putSingle("expand", "status");
-
EasyMock.expect(forward.getQueryParameters()).andReturn(queryParams).anyTimes();
- EasyMock.replay(forward);
-
- PowerMock.replayAll();
+ when(forward.getQueryParameters()).thenReturn(queryParams);
Map<String, Map<String, Object>> expanded = (Map<String, Map<String,
Object>>) connectorsResource.listConnectors(forward, NULL_HEADERS).getEntity();
// Ordering isn't guaranteed, compare sets
assertEquals(Collections.singleton(CONNECTOR2_NAME),
expanded.keySet());
assertEquals(connector2, expanded.get(CONNECTOR2_NAME).get("status"));
- PowerMock.verifyAll();
}
@Test
public void testCreateConnector() throws Throwable {
CreateConnectorRequest body = new
CreateConnectorRequest(CONNECTOR_NAME,
Collections.singletonMap(ConnectorConfig.NAME_CONFIG, CONNECTOR_NAME));
- final Capture<Callback<Herder.Created<ConnectorInfo>>> cb =
Capture.newInstance();
- herder.putConnectorConfig(EasyMock.eq(CONNECTOR_NAME),
EasyMock.eq(body.config()), EasyMock.eq(false), EasyMock.capture(cb));
+ final ArgumentCaptor<Callback<Herder.Created<ConnectorInfo>>> cb =
ArgumentCaptor.forClass(Callback.class);
expectAndCallbackResult(cb, new Herder.Created<>(true, new
ConnectorInfo(CONNECTOR_NAME, CONNECTOR_CONFIG,
- CONNECTOR_TASK_NAMES, ConnectorType.SOURCE)));
-
- PowerMock.replayAll();
+ CONNECTOR_TASK_NAMES, ConnectorType.SOURCE))
+ ).when(herder).putConnectorConfig(eq(CONNECTOR_NAME),
eq(body.config()), eq(false), cb.capture());
connectorsResource.createConnector(FORWARD, NULL_HEADERS, body);
-
- PowerMock.verifyAll();
}
@Test
public void testCreateConnectorNotLeader() throws Throwable {
CreateConnectorRequest body = new
CreateConnectorRequest(CONNECTOR_NAME,
Collections.singletonMap(ConnectorConfig.NAME_CONFIG, CONNECTOR_NAME));
- final Capture<Callback<Herder.Created<ConnectorInfo>>> cb =
Capture.newInstance();
- herder.putConnectorConfig(EasyMock.eq(CONNECTOR_NAME),
EasyMock.eq(body.config()), EasyMock.eq(false), EasyMock.capture(cb));
- expectAndCallbackNotLeaderException(cb);
- // Should forward request
- EasyMock.expect(RestClient.httpRequest(EasyMock.eq(LEADER_URL +
"connectors?forward=false"), EasyMock.eq("POST"), EasyMock.isNull(),
EasyMock.eq(body), EasyMock.anyObject(),
EasyMock.anyObject(WorkerConfig.class)))
- .andReturn(new RestClient.HttpResponse<>(201, new HashMap<>(),
new ConnectorInfo(CONNECTOR_NAME, CONNECTOR_CONFIG, CONNECTOR_TASK_NAMES,
- ConnectorType.SOURCE)));
+ final ArgumentCaptor<Callback<Herder.Created<ConnectorInfo>>> cb =
ArgumentCaptor.forClass(Callback.class);
+ expectAndCallbackNotLeaderException(cb).when(herder)
+ .putConnectorConfig(eq(CONNECTOR_NAME), eq(body.config()),
eq(false), cb.capture());
- PowerMock.replayAll();
+ // Should forward request
+ restClientStatic.when(() ->
+ RestClient.httpRequest(eq(LEADER_URL +
"connectors?forward=false"), eq("POST"), isNull(), eq(body), any(),
any(WorkerConfig.class))
+ ).thenReturn(new RestClient.HttpResponse<>(201, new HashMap<>(), new
ConnectorInfo(CONNECTOR_NAME, CONNECTOR_CONFIG, CONNECTOR_TASK_NAMES,
ConnectorType.SOURCE)));
connectorsResource.createConnector(FORWARD, NULL_HEADERS, body);
-
- PowerMock.verifyAll();
-
-
}
@Test
public void testCreateConnectorWithHeaderAuthorization() throws Throwable {
CreateConnectorRequest body = new
CreateConnectorRequest(CONNECTOR_NAME,
Collections.singletonMap(ConnectorConfig.NAME_CONFIG, CONNECTOR_NAME));
- final Capture<Callback<Herder.Created<ConnectorInfo>>> cb =
Capture.newInstance();
- HttpHeaders httpHeaders = EasyMock.mock(HttpHeaders.class);
-
EasyMock.expect(httpHeaders.getHeaderString("Authorization")).andReturn("Basic
YWxhZGRpbjpvcGVuc2VzYW1l").times(1);
- EasyMock.replay(httpHeaders);
- herder.putConnectorConfig(EasyMock.eq(CONNECTOR_NAME),
EasyMock.eq(body.config()), EasyMock.eq(false), EasyMock.capture(cb));
- expectAndCallbackNotLeaderException(cb);
+ final ArgumentCaptor<Callback<Herder.Created<ConnectorInfo>>> cb =
ArgumentCaptor.forClass(Callback.class);
+ HttpHeaders httpHeaders = mock(HttpHeaders.class);
+ expectAndCallbackNotLeaderException(cb)
+ .when(herder).putConnectorConfig(eq(CONNECTOR_NAME),
eq(body.config()), eq(false), cb.capture());
- EasyMock.expect(RestClient.httpRequest(EasyMock.eq(LEADER_URL +
"connectors?forward=false"),
- EasyMock.eq("POST"), EasyMock.eq(httpHeaders),
EasyMock.anyObject(), EasyMock.anyObject(),
EasyMock.anyObject(WorkerConfig.class)))
- .andReturn(new RestClient.HttpResponse<>(202, new HashMap<>(),
null));
-
- PowerMock.replayAll();
+ restClientStatic.when(() ->
+ RestClient.httpRequest(eq(LEADER_URL +
"connectors?forward=false"), eq("POST"), eq(httpHeaders), any(), any(),
any(WorkerConfig.class))
+ ).thenReturn(new RestClient.HttpResponse<>(202, new HashMap<>(),
null));
connectorsResource.createConnector(FORWARD, httpHeaders, body);
-
- PowerMock.verifyAll();
}
@Test
public void testCreateConnectorWithoutHeaderAuthorization() throws
Throwable {
Review Comment:
Okay so I had a look into this because I ended up getting confused with what
happened. Originally the test was using specific Authorization headers, i.e.
```java
when(httpHeaders.getHeaderString("Authorization")).thenReturn("Basic
YWxhZGRpbjpvcGVuc2VzYW1l");
```
and
```java
when(httpHeaders.getHeaderString("Authorization")).thenReturn(null);
```
respectively. I then enabled
`@RunWith(MockitoJUnitRunner.StrictStubs.class)` at which point Mockito
reported that these stubs weren't being used which ended up making the two
tests identical since I removed them. So the question then is, is it useful to
test for the specific values of the `"Authorization"` header or just that the
`HttpHeaders` object is correctly being passed and returned?
If you don't care about testing the the actual value of the
`"Authorization"` header then I can just remove the identical test and rename
the other one to `testCreateConnectorWithForwardedHeaders` as you stated
otherwise I would need to improve the tests (I think that this can be solved by
just adding the headers into the `HttpHeaders` rather than mocking the
`getHeaderString` method, doing so means it will get picked up by the `eq`
check).
--
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
