gharris1727 commented on code in PR #15389:
URL: https://github.com/apache/kafka/pull/15389#discussion_r1538282799
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##########
@@ -115,64 +115,61 @@ public class StandaloneHerderTest {
private static final String TOPICS_LIST_STR = "topic1,topic2";
private static final String WORKER_ID = "localhost:8083";
private static final String KAFKA_CLUSTER_ID = "I4ZmrWqfT2e-upky_4fdPA";
+ private static final Long WAIT_TIME = 1000L;
Review Comment:
nit: 1 second is probably good enough, but it costs nothing to increase this
now.
Also the constant should probably include the unit, so `WAIT_TIME_MS`.
```suggestion
private static final Long WAIT_TIME = 15000L;
```
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##########
@@ -227,59 +222,48 @@ public void testCreateConnectorAlreadyExists() throws
Throwable {
@Test
public void testCreateSinkConnector() throws Exception {
- connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map<String, String> config = connectorConfig(SourceSink.SINK);
- Connector connectorMock = mock(SinkConnector.class);
- expectConfigValidation(connectorMock, true, config);
+ expectConfigValidation(SourceSink.SINK, config);
herder.putConnectorConfig(CONNECTOR_NAME, config, false,
createCallback);
- Herder.Created<ConnectorInfo> connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
+ Herder.Created<ConnectorInfo> connectorInfo =
createCallback.get(WAIT_TIME, TimeUnit.MILLISECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
}
@Test
public void testCreateConnectorWithStoppedInitialState() throws Exception {
- connector = mock(BogusSinkConnector.class);
Map<String, String> config = connectorConfig(SourceSink.SINK);
Connector connectorMock = mock(SinkConnector.class);
Review Comment:
nit: unused
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##########
@@ -1168,25 +1090,27 @@ private static Map<String, String>
taskConfig(SourceSink sourceSink) {
}
private void expectConfigValidation(
- Connector connectorMock,
- boolean shouldCreateConnector,
+ SourceSink sourceSink,
Map<String, String>... configs
) {
// config validation
+ Connector connectorMock = sourceSink == SourceSink.SOURCE ?
mock(SourceConnector.class) : mock(SinkConnector.class);
when(worker.configTransformer()).thenReturn(transformer);
final ArgumentCaptor<Map<String, String>> configCapture =
ArgumentCaptor.forClass(Map.class);
when(transformer.transform(configCapture.capture())).thenAnswer(invocation ->
configCapture.getValue());
when(worker.getPlugins()).thenReturn(plugins);
when(plugins.connectorLoader(anyString())).thenReturn(pluginLoader);
when(plugins.withClassLoader(pluginLoader)).thenReturn(loaderSwap);
- if (shouldCreateConnector) {
- when(worker.getPlugins()).thenReturn(plugins);
- when(plugins.newConnector(anyString())).thenReturn(connectorMock);
- }
+
+ // Assume the connector should always be created
+ when(worker.getPlugins()).thenReturn(plugins);
+ when(plugins.newConnector(anyString())).thenReturn(connectorMock);
when(connectorMock.config()).thenReturn(new ConfigDef());
- for (Map<String, String> config : configs)
+ // Set up validation for each config
+ for (Map<String, String> config : configs) {
when(connectorMock.validate(config)).thenReturn(new
Config(Collections.emptyList()));
+ }
}
// We need to use a real class here due to some issue with mocking
java.lang.Class
Review Comment:
nit: My IDE is also warning me that these should be `static`, which is fine
for how these are used. Could you also fix that?
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##########
@@ -726,22 +668,21 @@ public void testAccessors() throws Exception {
doNothing().when(tasksConfigCb).onCompletion(any(NotFoundException.class),
isNull());
doNothing().when(connectorConfigCb).onCompletion(any(NotFoundException.class),
isNull());
- // Create connector
- connector = mock(BogusSourceConnector.class);
+
expectAdd(SourceSink.SOURCE);
- expectConfigValidation(connector, true, connConfig);
+ expectConfigValidation(SourceSink.SOURCE, connConfig);
// Validate accessors with 1 connector
doNothing().when(listConnectorsCb).onCompletion(null,
singleton(CONNECTOR_NAME));
ConnectorInfo connInfo = new ConnectorInfo(CONNECTOR_NAME, connConfig,
Arrays.asList(new ConnectorTaskId(CONNECTOR_NAME, 0)),
Review Comment:
nit: Could you also solve the Arrays.asList warnings? My IDE wants these to
be Collections.singletonList(). Here and throughout.
--
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]
