Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante merged PR #12728: URL: https://github.com/apache/kafka/pull/12728 -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1844822620 I have just pushed a merge commit which adds the `testCreateConnectorWithStoppedInitialState()` test. The translation of the test was quite straight forward, only thing to note is that the original test used `PowerMock.verifyAll();` as an escape hatch, I replaced it with `verify(loaderSwap).close();`, not sure if you want to add any more verifications (or even remove `verify(loaderSwap).close();` as being misleading). -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1843872683 > @mdedetrich Thanks, the changes look good to go. But the build is failing because there are new tests on trunk that still use EasyMock/PowerMock. Can you rebase on the latest from trunk? Yup doing so now -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1843418398 @mdedetrich Thanks, the changes look good to go. But the build is failing because there are new tests on trunk that still use EasyMock/PowerMock. Can you rebase on the latest from trunk? -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1841940640 @C0urante @ijuma I have addressed the final comments, let me know if I have missed anything -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1416539486
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -908,75 +799,58 @@ public void testCorruptConfig() throws Throwable {
config.put(ConnectorConfig.NAME_CONFIG, CONNECTOR_NAME);
config.put(ConnectorConfig.CONNECTOR_CLASS_CONFIG,
BogusSinkConnector.class.getName());
config.put(SinkConnectorConfig.TOPICS_CONFIG, TOPICS_LIST_STR);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
String error = "This is an error in your config!";
List errors = new ArrayList<>(singletonList(error));
String key = "foo.invalid.key";
-EasyMock.expect(connectorMock.validate(config)).andReturn(
+when(connectorMock.validate(config)).thenReturn(
new Config(
Arrays.asList(new ConfigValue(key, null,
Collections.emptyList(), errors))
)
);
ConfigDef configDef = new ConfigDef();
configDef.define(key, ConfigDef.Type.STRING,
ConfigDef.Importance.HIGH, "");
-
EasyMock.expect(worker.configTransformer()).andReturn(transformer).times(2);
-final Capture> configCapture =
EasyMock.newCapture();
-
EasyMock.expect(transformer.transform(EasyMock.capture(configCapture))).andAnswer(configCapture::getValue);
-EasyMock.expect(worker.getPlugins()).andReturn(plugins).times(3);
-
EasyMock.expect(plugins.connectorLoader(EasyMock.anyString())).andReturn(pluginLoader);
-
EasyMock.expect(plugins.withClassLoader(pluginLoader)).andReturn(loaderSwap);
-EasyMock.expect(worker.getPlugins()).andStubReturn(plugins);
-
EasyMock.expect(plugins.newConnector(EasyMock.anyString())).andReturn(connectorMock);
-EasyMock.expect(connectorMock.config()).andStubReturn(configDef);
-loaderSwap.close();
Review Comment:
Committed and pushed
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1416535578
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -887,19 +783,14 @@ public void testPutConnectorConfig() throws Exception {
assertEquals("bar", capturedConfig.getValue().get("foo"));
herder.connectorConfig(CONNECTOR_NAME, connectorConfigCb);
Review Comment:
Fixed and committed.
> It'd probably be better to do away with mocked callbacks and switch to
using a `FutureCallback` that we invoke `get` on (like we do with many other
tests), but it's up to you if you'd like to implement that or not.
I think its better to solve this as another PR/issue. This PR has already
been ongoing for long enough and this just appears to be a nice improvement for
me.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
ijuma commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1840638490 @mdedetrich looks like this is really close - can you address the last set of comments so we can merge? -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1390091434
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -908,75 +799,58 @@ public void testCorruptConfig() throws Throwable {
config.put(ConnectorConfig.NAME_CONFIG, CONNECTOR_NAME);
config.put(ConnectorConfig.CONNECTOR_CLASS_CONFIG,
BogusSinkConnector.class.getName());
config.put(SinkConnectorConfig.TOPICS_CONFIG, TOPICS_LIST_STR);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
String error = "This is an error in your config!";
List errors = new ArrayList<>(singletonList(error));
String key = "foo.invalid.key";
-EasyMock.expect(connectorMock.validate(config)).andReturn(
+when(connectorMock.validate(config)).thenReturn(
new Config(
Arrays.asList(new ConfigValue(key, null,
Collections.emptyList(), errors))
)
);
ConfigDef configDef = new ConfigDef();
configDef.define(key, ConfigDef.Type.STRING,
ConfigDef.Importance.HIGH, "");
-
EasyMock.expect(worker.configTransformer()).andReturn(transformer).times(2);
-final Capture> configCapture =
EasyMock.newCapture();
-
EasyMock.expect(transformer.transform(EasyMock.capture(configCapture))).andAnswer(configCapture::getValue);
-EasyMock.expect(worker.getPlugins()).andReturn(plugins).times(3);
-
EasyMock.expect(plugins.connectorLoader(EasyMock.anyString())).andReturn(pluginLoader);
-
EasyMock.expect(plugins.withClassLoader(pluginLoader)).andReturn(loaderSwap);
-EasyMock.expect(worker.getPlugins()).andStubReturn(plugins);
-
EasyMock.expect(plugins.newConnector(EasyMock.anyString())).andReturn(connectorMock);
-EasyMock.expect(connectorMock.config()).andStubReturn(configDef);
-loaderSwap.close();
Review Comment:
Don't we still want to verify that the call to `loaderSwap::close` takes
place?
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1390087872
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -817,19 +726,14 @@ public void testAccessors() throws Exception {
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
-EasyMock.reset(transformer);
-EasyMock.expect(transformer.transform(eq(CONNECTOR_NAME),
EasyMock.anyObject()))
-.andThrow(new AssertionError("Config transformation should not
occur when requesting connector or task info"))
-.anyTimes();
-EasyMock.replay(transformer);
-
+reset(transformer);
herder.connectors(listConnectorsCb);
herder.connectorInfo(CONNECTOR_NAME, connectorInfoCb);
herder.connectorConfig(CONNECTOR_NAME, connectorConfigCb);
herder.taskConfigs(CONNECTOR_NAME, taskConfigsCb);
herder.tasksConfig(CONNECTOR_NAME, tasksConfigCb);
-
-PowerMock.verifyAll();
+// Config transformation should not occur when requesting connector or
task info
+verify(transformer, never()).transform(eq(CONNECTOR_NAME), any());
Review Comment:
This is much cleaner than the existing strategy (where we cause invocations
of `transformer::transform` to throw an exception). Nice job, thanks!
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -908,75 +799,58 @@ public void testCorruptConfig() throws Throwable {
config.put(ConnectorConfig.NAME_CONFIG, CONNECTOR_NAME);
config.put(ConnectorConfig.CONNECTOR_CLASS_CONFIG,
BogusSinkConnector.class.getName());
config.put(SinkConnectorConfig.TOPICS_CONFIG, TOPICS_LIST_STR);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
String error = "This is an error in your config!";
List errors = new ArrayList<>(singletonList(error));
String key = "foo.invalid.key";
-EasyMock.expect(connectorMock.validate(config)).andReturn(
+when(connectorMock.validate(config)).thenReturn(
new Config(
Arrays.asList(new ConfigValue(key, null,
Collections.emptyList(), errors))
)
);
ConfigDef configDef = new ConfigDef();
configDef.define(key, ConfigDef.Type.STRING,
ConfigDef.Importance.HIGH, "");
-
EasyMock.expect(worker.configTransformer()).andReturn(transformer).times(2);
-final Capture> configCapture =
EasyMock.newCapture();
-
EasyMock.expect(transformer.transform(EasyMock.capture(configCapture))).andAnswer(configCapture::getValue);
-EasyMock.expect(worker.getPlugins()).andReturn(plugins).times(3);
-
EasyMock.expect(plugins.connectorLoader(EasyMock.anyString())).andReturn(pluginLoader);
-
EasyMock.expect(plugins.withClassLoader(pluginLoader)).andReturn(loaderSwap);
-EasyMock.expect(worker.getPlugins()).andStubReturn(plugins);
-
EasyMock.expect(plugins.newConnector(EasyMock.anyString())).andReturn(connectorMock);
-EasyMock.expect(connectorMock.config()).andStubReturn(configDef);
-loaderSwap.close();
Review Comment:
Don't we still want to verify that this takes place?
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -887,19 +783,14 @@ public void testPutConnectorConfig() throws Exception {
assertEquals("bar", capturedConfig.getValue().get("foo"));
herder.connectorConfig(CONNECTOR_NAME, connectorConfigCb);
Review Comment:
Can we also make sure that there aren't any errors reported with the
`connectorConfigCb`?
The easy way to do this would be to just add this line to the end of the
test:
```java
verifyNoMoreInteractions(connectorConfigCb);
```
It'd probably be better to do away with mocked callbacks and switch to using
a `FutureCallback` that we invoke `get` on (like we do with many other tests),
but it's up to you if you'd like to implement that or not.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1389230779
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -722,89 +646,68 @@ public void testRestartConnectorAndTasksBoth() throws
Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
+
+verifyConnectorStatusRestart();
+verify(statusBackingStore).put(taskStatus.capture());
}
@Test
public void testCreateAndStop() throws Exception {
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
Map connectorConfig =
connectorConfig(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-// herder.stop() should stop any running connectors and tasks even if
destroyConnector was not invoked
expectStop();
-statusBackingStore.put(new TaskStatus(new
ConnectorTaskId(CONNECTOR_NAME, 0), AbstractStatus.State.DESTROYED, WORKER_ID,
0));
-EasyMock.expectLastCall();
-
-statusBackingStore.stop();
-EasyMock.expectLastCall();
-worker.stop();
-EasyMock.expectLastCall();
-
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
+// herder.stop() should stop any running connectors and tasks even if
destroyConnector was not invoked
herder.stop();
assertTrue(noneConnectorClientConfigOverridePolicy.isClosed());
-
-PowerMock.verifyAll();
+verify(worker).stop();
+verify(statusBackingStore).stop();
+verify(statusBackingStore).put(new TaskStatus(new
ConnectorTaskId(CONNECTOR_NAME, 0), AbstractStatus.State.DESTROYED, WORKER_ID,
0));
}
@Test
public void testAccessors() throws Exception {
Map connConfig = connectorConfig(SourceSink.SOURCE);
System.out.println(connConfig);
-Callback> listConnectorsCb =
PowerMock.createMock(Callback.class);
-Callback connectorInfoCb =
PowerMock.createMock(Callback.class);
-Callback> connectorConfigCb =
PowerMock.createMock(Callback.class);
-Callback> taskConfigsCb =
PowerMock.createMock(Callback.class);
-Callback>> tasksConfigCb =
PowerMock.createMock(Callback.class);
+Callback> listConnectorsCb = mock(Callback.class);
+Callback connectorInfoCb = mock(Callback.class);
+Callback> connectorConfigCb = mock(Callback.class);
+Callback> taskConfigsCb = mock(Callback.class);
+Callback>> tasksConfigCb =
mock(Callback.class);
// Check accessors with empty worker
-listConnectorsCb.onCompletion(null, Collections.EMPTY_SET);
-EasyMock.expectLastCall();
-connectorInfoCb.onCompletion(EasyMock.anyObject(),
EasyMock.isNull());
-EasyMock.expectLastCall();
-
connectorConfigCb.onCompletion(EasyMock.anyObject(),
EasyMock.isNull());
-EasyMock.expectLastCall();
Review Comment:
Committed and pushed
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1389227114
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -656,48 +586,43 @@ public void testRestartConnectorAndTasksOnlyTasks()
throws Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
+ArgumentCaptor taskStatus =
ArgumentCaptor.forClass(TaskStatus.class);
+verify(statusBackingStore).put(taskStatus.capture());
+assertEquals(AbstractStatus.State.RESTARTING,
taskStatus.getValue().state());
Review Comment:
Commited and pushed
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -722,89 +646,68 @@ public void testRestartConnectorAndTasksBoth() throws
Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
+
+verifyConnectorStatusRestart();
+verify(statusBackingStore).put(taskStatus.capture());
Review Comment:
Commited and pushed
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1388621193
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -722,89 +646,68 @@ public void testRestartConnectorAndTasksBoth() throws
Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
+
+verifyConnectorStatusRestart();
+verify(statusBackingStore).put(taskStatus.capture());
}
@Test
public void testCreateAndStop() throws Exception {
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
Map connectorConfig =
connectorConfig(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-// herder.stop() should stop any running connectors and tasks even if
destroyConnector was not invoked
expectStop();
-statusBackingStore.put(new TaskStatus(new
ConnectorTaskId(CONNECTOR_NAME, 0), AbstractStatus.State.DESTROYED, WORKER_ID,
0));
-EasyMock.expectLastCall();
-
-statusBackingStore.stop();
-EasyMock.expectLastCall();
-worker.stop();
-EasyMock.expectLastCall();
-
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
+// herder.stop() should stop any running connectors and tasks even if
destroyConnector was not invoked
herder.stop();
assertTrue(noneConnectorClientConfigOverridePolicy.isClosed());
-
-PowerMock.verifyAll();
+verify(worker).stop();
+verify(statusBackingStore).stop();
+verify(statusBackingStore).put(new TaskStatus(new
ConnectorTaskId(CONNECTOR_NAME, 0), AbstractStatus.State.DESTROYED, WORKER_ID,
0));
}
@Test
public void testAccessors() throws Exception {
Map connConfig = connectorConfig(SourceSink.SOURCE);
System.out.println(connConfig);
-Callback> listConnectorsCb =
PowerMock.createMock(Callback.class);
-Callback connectorInfoCb =
PowerMock.createMock(Callback.class);
-Callback> connectorConfigCb =
PowerMock.createMock(Callback.class);
-Callback> taskConfigsCb =
PowerMock.createMock(Callback.class);
-Callback>> tasksConfigCb =
PowerMock.createMock(Callback.class);
+Callback> listConnectorsCb = mock(Callback.class);
+Callback connectorInfoCb = mock(Callback.class);
+Callback> connectorConfigCb = mock(Callback.class);
+Callback> taskConfigsCb = mock(Callback.class);
+Callback>> tasksConfigCb =
mock(Callback.class);
// Check accessors with empty worker
-listConnectorsCb.onCompletion(null, Collections.EMPTY_SET);
-EasyMock.expectLastCall();
-connectorInfoCb.onCompletion(EasyMock.anyObject(),
EasyMock.isNull());
-EasyMock.expectLastCall();
-
connectorConfigCb.onCompletion(EasyMock.anyObject(),
EasyMock.isNull());
-EasyMock.expectLastCall();
Review Comment:
Aren't we missing a corresponding `doNothing().when(...)` line for this
expectation?
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1389050516
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -838,39 +733,32 @@ public void testPutConnectorConfig() throws Exception {
Map newConnConfig = new HashMap<>(connConfig);
newConnConfig.put("foo", "bar");
-Callback> connectorConfigCb =
PowerMock.createMock(Callback.class);
-// Callback> putConnectorConfigCb =
PowerMock.createMock(Callback.class);
+Callback> connectorConfigCb = mock(Callback.class);
// Create
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, connConfig);
// Should get first config
-connectorConfigCb.onCompletion(null, connConfig);
-EasyMock.expectLastCall();
+doNothing().when(connectorConfigCb).onCompletion(null, connConfig);
// Update config, which requires stopping and restarting
-worker.stopAndAwaitConnector(CONNECTOR_NAME);
-EasyMock.expectLastCall();
-Capture> capturedConfig = EasyMock.newCapture();
-Capture> onStart = EasyMock.newCapture();
-worker.startConnector(eq(CONNECTOR_NAME),
EasyMock.capture(capturedConfig), EasyMock.anyObject(),
-eq(herder), eq(TargetState.STARTED),
EasyMock.capture(onStart));
-EasyMock.expectLastCall().andAnswer(() -> {
+doNothing().when(worker).stopAndAwaitConnector(CONNECTOR_NAME);
+final ArgumentCaptor> capturedConfig =
ArgumentCaptor.forClass(Map.class);
+final ArgumentCaptor> onStart =
ArgumentCaptor.forClass(Callback.class);
+doAnswer(invocation -> {
onStart.getValue().onCompletion(null, TargetState.STARTED);
return true;
-});
+}).when(worker).startConnector(eq(CONNECTOR_NAME),
capturedConfig.capture(), any(),
+eq(herder), eq(TargetState.STARTED), onStart.capture());
// Generate same task config, which should result in no additional
action to restart tasks
-EasyMock.expect(worker.connectorTaskConfigs(CONNECTOR_NAME, new
SourceConnectorConfig(plugins, newConnConfig, true)))
-.andReturn(singletonList(taskConfig(SourceSink.SOURCE)));
+when(worker.connectorTaskConfigs(CONNECTOR_NAME, new
SourceConnectorConfig(plugins, newConnConfig, true)))
+.thenReturn(singletonList(taskConfig(SourceSink.SOURCE)));
expectConfigValidation(connectorMock, false, newConnConfig);
-connectorConfigCb.onCompletion(null, newConnConfig);
-EasyMock.expectLastCall();
-EasyMock.expect(worker.getPlugins()).andReturn(plugins).anyTimes();
-
-PowerMock.replayAll();
+doNothing().when(connectorConfigCb).onCompletion(null, newConnConfig);
+when(worker.getPlugins()).thenReturn(plugins);
Review Comment:
Done and pushed
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1389049719
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -838,39 +733,32 @@ public void testPutConnectorConfig() throws Exception {
Map newConnConfig = new HashMap<>(connConfig);
newConnConfig.put("foo", "bar");
-Callback> connectorConfigCb =
PowerMock.createMock(Callback.class);
-// Callback> putConnectorConfigCb =
PowerMock.createMock(Callback.class);
+Callback> connectorConfigCb = mock(Callback.class);
// Create
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, connConfig);
// Should get first config
-connectorConfigCb.onCompletion(null, connConfig);
-EasyMock.expectLastCall();
+doNothing().when(connectorConfigCb).onCompletion(null, connConfig);
// Update config, which requires stopping and restarting
-worker.stopAndAwaitConnector(CONNECTOR_NAME);
-EasyMock.expectLastCall();
-Capture> capturedConfig = EasyMock.newCapture();
-Capture> onStart = EasyMock.newCapture();
-worker.startConnector(eq(CONNECTOR_NAME),
EasyMock.capture(capturedConfig), EasyMock.anyObject(),
-eq(herder), eq(TargetState.STARTED),
EasyMock.capture(onStart));
-EasyMock.expectLastCall().andAnswer(() -> {
+doNothing().when(worker).stopAndAwaitConnector(CONNECTOR_NAME);
+final ArgumentCaptor> capturedConfig =
ArgumentCaptor.forClass(Map.class);
+final ArgumentCaptor> onStart =
ArgumentCaptor.forClass(Callback.class);
+doAnswer(invocation -> {
onStart.getValue().onCompletion(null, TargetState.STARTED);
return true;
-});
+}).when(worker).startConnector(eq(CONNECTOR_NAME),
capturedConfig.capture(), any(),
+eq(herder), eq(TargetState.STARTED), onStart.capture());
// Generate same task config, which should result in no additional
action to restart tasks
-EasyMock.expect(worker.connectorTaskConfigs(CONNECTOR_NAME, new
SourceConnectorConfig(plugins, newConnConfig, true)))
-.andReturn(singletonList(taskConfig(SourceSink.SOURCE)));
+when(worker.connectorTaskConfigs(CONNECTOR_NAME, new
SourceConnectorConfig(plugins, newConnConfig, true)))
+.thenReturn(singletonList(taskConfig(SourceSink.SOURCE)));
expectConfigValidation(connectorMock, false, newConnConfig);
-connectorConfigCb.onCompletion(null, newConnConfig);
-EasyMock.expectLastCall();
-EasyMock.expect(worker.getPlugins()).andReturn(plugins).anyTimes();
-
-PowerMock.replayAll();
+doNothing().when(connectorConfigCb).onCompletion(null, newConnConfig);
Review Comment:
Done and pushed.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1389047935
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -817,19 +731,11 @@ public void testAccessors() throws Exception {
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
-EasyMock.reset(transformer);
-EasyMock.expect(transformer.transform(eq(CONNECTOR_NAME),
EasyMock.anyObject()))
Review Comment:
Committed and pushed
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1388623720
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -656,48 +586,43 @@ public void testRestartConnectorAndTasksOnlyTasks()
throws Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
+ArgumentCaptor taskStatus =
ArgumentCaptor.forClass(TaskStatus.class);
+verify(statusBackingStore).put(taskStatus.capture());
+assertEquals(AbstractStatus.State.RESTARTING,
taskStatus.getValue().state());
Review Comment:
Can we also assert the task ID here?
```java
assertEquals(taskId, taskStatus.getValue().id());
```
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1388627971
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -838,39 +733,32 @@ public void testPutConnectorConfig() throws Exception {
Map newConnConfig = new HashMap<>(connConfig);
newConnConfig.put("foo", "bar");
-Callback> connectorConfigCb =
PowerMock.createMock(Callback.class);
-// Callback> putConnectorConfigCb =
PowerMock.createMock(Callback.class);
+Callback> connectorConfigCb = mock(Callback.class);
// Create
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, connConfig);
// Should get first config
-connectorConfigCb.onCompletion(null, connConfig);
-EasyMock.expectLastCall();
+doNothing().when(connectorConfigCb).onCompletion(null, connConfig);
// Update config, which requires stopping and restarting
-worker.stopAndAwaitConnector(CONNECTOR_NAME);
-EasyMock.expectLastCall();
-Capture> capturedConfig = EasyMock.newCapture();
-Capture> onStart = EasyMock.newCapture();
-worker.startConnector(eq(CONNECTOR_NAME),
EasyMock.capture(capturedConfig), EasyMock.anyObject(),
-eq(herder), eq(TargetState.STARTED),
EasyMock.capture(onStart));
-EasyMock.expectLastCall().andAnswer(() -> {
+doNothing().when(worker).stopAndAwaitConnector(CONNECTOR_NAME);
+final ArgumentCaptor> capturedConfig =
ArgumentCaptor.forClass(Map.class);
+final ArgumentCaptor> onStart =
ArgumentCaptor.forClass(Callback.class);
+doAnswer(invocation -> {
onStart.getValue().onCompletion(null, TargetState.STARTED);
return true;
-});
+}).when(worker).startConnector(eq(CONNECTOR_NAME),
capturedConfig.capture(), any(),
+eq(herder), eq(TargetState.STARTED), onStart.capture());
// Generate same task config, which should result in no additional
action to restart tasks
-EasyMock.expect(worker.connectorTaskConfigs(CONNECTOR_NAME, new
SourceConnectorConfig(plugins, newConnConfig, true)))
-.andReturn(singletonList(taskConfig(SourceSink.SOURCE)));
+when(worker.connectorTaskConfigs(CONNECTOR_NAME, new
SourceConnectorConfig(plugins, newConnConfig, true)))
+.thenReturn(singletonList(taskConfig(SourceSink.SOURCE)));
expectConfigValidation(connectorMock, false, newConnConfig);
-connectorConfigCb.onCompletion(null, newConnConfig);
-EasyMock.expectLastCall();
-EasyMock.expect(worker.getPlugins()).andReturn(plugins).anyTimes();
-
-PowerMock.replayAll();
+doNothing().when(connectorConfigCb).onCompletion(null, newConnConfig);
+when(worker.getPlugins()).thenReturn(plugins);
Review Comment:
We can remove this.
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -722,89 +646,68 @@ public void testRestartConnectorAndTasksBoth() throws
Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
+
+verifyConnectorStatusRestart();
+verify(statusBackingStore).put(taskStatus.capture());
Review Comment:
Can we also verify the state and task ID? E.g.:
```java
assertEquals(AbstractStatus.State.RESTARTING, taskStatus.getValue().state());
assertEquals(taskId, taskStatus.getValue().id());
```
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -656,48 +586,43 @@ public void testRestartConnectorAndTasksOnlyTasks()
throws Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
+ArgumentCaptor taskStatus =
ArgumentCaptor.forClass(TaskStatus.class);
+verify(statusBackingStore).put(taskStatus.capture());
+assertEquals(AbstractStatus.State.RESTARTING,
taskStatus.getValue().state());
Review Comment:
Can we also assert the task ID here?
``java
assertEquals(taskId, taskStatus.getValue().id());
```
##
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1388621758
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -817,19 +731,11 @@ public void testAccessors() throws Exception {
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
-EasyMock.reset(transformer);
-EasyMock.expect(transformer.transform(eq(CONNECTOR_NAME),
EasyMock.anyObject()))
Review Comment:
I agree with @mimaison, we should keep this part. Otherwise, there's no
reason to issue duplicate calls to `Herder::connectors`,
`Herder::connectorInfo`, etc.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1388621193
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -722,89 +646,68 @@ public void testRestartConnectorAndTasksBoth() throws
Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
+
+verifyConnectorStatusRestart();
+verify(statusBackingStore).put(taskStatus.capture());
}
@Test
public void testCreateAndStop() throws Exception {
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
Map connectorConfig =
connectorConfig(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-// herder.stop() should stop any running connectors and tasks even if
destroyConnector was not invoked
expectStop();
-statusBackingStore.put(new TaskStatus(new
ConnectorTaskId(CONNECTOR_NAME, 0), AbstractStatus.State.DESTROYED, WORKER_ID,
0));
-EasyMock.expectLastCall();
-
-statusBackingStore.stop();
-EasyMock.expectLastCall();
-worker.stop();
-EasyMock.expectLastCall();
-
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
+// herder.stop() should stop any running connectors and tasks even if
destroyConnector was not invoked
herder.stop();
assertTrue(noneConnectorClientConfigOverridePolicy.isClosed());
-
-PowerMock.verifyAll();
+verify(worker).stop();
+verify(statusBackingStore).stop();
+verify(statusBackingStore).put(new TaskStatus(new
ConnectorTaskId(CONNECTOR_NAME, 0), AbstractStatus.State.DESTROYED, WORKER_ID,
0));
}
@Test
public void testAccessors() throws Exception {
Map connConfig = connectorConfig(SourceSink.SOURCE);
System.out.println(connConfig);
-Callback> listConnectorsCb =
PowerMock.createMock(Callback.class);
-Callback connectorInfoCb =
PowerMock.createMock(Callback.class);
-Callback> connectorConfigCb =
PowerMock.createMock(Callback.class);
-Callback> taskConfigsCb =
PowerMock.createMock(Callback.class);
-Callback>> tasksConfigCb =
PowerMock.createMock(Callback.class);
+Callback> listConnectorsCb = mock(Callback.class);
+Callback connectorInfoCb = mock(Callback.class);
+Callback> connectorConfigCb = mock(Callback.class);
+Callback> taskConfigsCb = mock(Callback.class);
+Callback>> tasksConfigCb =
mock(Callback.class);
// Check accessors with empty worker
-listConnectorsCb.onCompletion(null, Collections.EMPTY_SET);
-EasyMock.expectLastCall();
-connectorInfoCb.onCompletion(EasyMock.anyObject(),
EasyMock.isNull());
-EasyMock.expectLastCall();
-
connectorConfigCb.onCompletion(EasyMock.anyObject(),
EasyMock.isNull());
-EasyMock.expectLastCall();
Review Comment:
Aren't we missing a corresponding `doNoting().when(...)` line for this
expectation?
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1801718171 @C0urante So I can confirm the following things * No methods are directly called on mocks (I checked both mocks defined by the `@Mock` annotation or local mocks created with `mock(...)`) * There is no usage of `atLeastOnce()` and there is only one case of `times` (i.e. `times(2)`) * I think I found a couple of cases where `// herder.stop()` comments were in incorrect places so I added a `Move herder.stop() comments to appropriate place` comment * I may have missed some cases regarding comments, I am slightly unsure about the `// herder.stop()` comments because I don't know if the comment refers to an actual call of `herder.stop()` or if `herder.stop()` is being called behind the scenes (but this to me doesn't make sense anyways) -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1795695928 Will do, I just removed the other `herder::onRestart` and I forgot about the rest of the instances of `atLeastOnce()` (they are now all gone). Ill try and go through the tests on by one but understanding the domain specific anti-patterns (such as `herder::onRestart`) isn't immediately obvious on my end. -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1795622577 @mdedetrich It looks like there's still a direct call to `herder::onRestart` in the `testRestartConnectorAndTasksOnlyTasks` test. Can you please revisit the PR, reviewing every test case to try to spot these anti-patterns, and then ping me once you're confident this is ready for another round? -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1795575391 @C0urante Gone through all of your comments/remarks and the tests pass, let me know if anything additional is needed -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1383701619
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -127,94 +134,86 @@ private enum SourceSink {
noneConnectorClientConfigOverridePolicy = new
SampleConnectorClientConfigOverridePolicy();
@Before
-public void setup() {
-worker = PowerMock.createMock(Worker.class);
-String[] methodNames = new String[]{"connectorType",
"buildRestartPlan", "recordRestarting"};
-herder = PowerMock.createPartialMock(StandaloneHerder.class,
methodNames,
-worker, WORKER_ID, KAFKA_CLUSTER_ID, statusBackingStore, new
MemoryConfigBackingStore(transformer), noneConnectorClientConfigOverridePolicy,
new MockTime());
+public void setup() throws ExecutionException, InterruptedException {
+worker = mock(Worker.class);
+herder = mock(StandaloneHerder.class, withSettings()
+.useConstructor(worker, WORKER_ID, KAFKA_CLUSTER_ID,
statusBackingStore, new MemoryConfigBackingStore(transformer),
noneConnectorClientConfigOverridePolicy, new MockTime())
+.defaultAnswer(CALLS_REAL_METHODS));
createCallback = new FutureCallback<>();
-plugins = PowerMock.createMock(Plugins.class);
-pluginLoader = PowerMock.createMock(PluginClassLoader.class);
-loaderSwap = PowerMock.createMock(LoaderSwap.class);
-PowerMock.mockStatic(WorkerConnector.class);
-Capture> configCapture = Capture.newInstance();
-EasyMock.expect(transformer.transform(eq(CONNECTOR_NAME),
EasyMock.capture(configCapture))).andAnswer(configCapture::getValue).anyTimes();
+plugins = mock(Plugins.class);
+pluginLoader = mock(PluginClassLoader.class);
+loaderSwap = mock(LoaderSwap.class);
+final ArgumentCaptor> configCapture =
ArgumentCaptor.forClass(Map.class);
+when(transformer.transform(eq(CONNECTOR_NAME),
configCapture.capture())).thenAnswer(invocation -> configCapture.getValue());
+}
+
+@After
+public void tearDown() {
+verifyNoMoreInteractions(worker, statusBackingStore);
}
@Test
public void testCreateSourceConnector() throws Exception {
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
Map config = connectorConfig(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, config);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, config, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
-
-PowerMock.verifyAll();
}
@Test
public void testCreateConnectorFailedValidation() {
// Basic validation should be performed and return an error, but
should still evaluate the connector's config
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
Map config = connectorConfig(SourceSink.SOURCE);
config.remove(ConnectorConfig.NAME_CONFIG);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
-
EasyMock.expect(worker.configTransformer()).andReturn(transformer).times(2);
-final Capture> configCapture =
EasyMock.newCapture();
-
EasyMock.expect(transformer.transform(EasyMock.capture(configCapture))).andAnswer(configCapture::getValue);
-EasyMock.expect(worker.getPlugins()).andReturn(plugins).times(4);
-
EasyMock.expect(plugins.newConnector(EasyMock.anyString())).andReturn(connectorMock);
-
EasyMock.expect(plugins.connectorLoader(EasyMock.anyString())).andReturn(pluginLoader);
-
EasyMock.expect(plugins.withClassLoader(pluginLoader)).andReturn(loaderSwap);
+Connector connectorMock = mock(SourceConnector.class);
+when(worker.configTransformer()).thenReturn(transformer);
+final ArgumentCaptor> configCapture =
ArgumentCaptor.forClass(Map.class);
+
when(transformer.transform(configCapture.capture())).thenAnswer(invocation ->
configCapture.getValue());
+when(worker.getPlugins()).thenReturn(plugins);
+when(plugins.newConnector(anyString())).thenReturn(connectorMock);
+when(plugins.connectorLoader(anyString())).thenReturn(pluginLoader);
+when(plugins.withClassLoader(pluginLoader)).thenReturn(loaderSwap);
-EasyMock.expect(connectorMock.config()).andStubReturn(new ConfigDef());
+when(connectorMock.config()).thenReturn(new ConfigDef());
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1383700349
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -223,93 +216,81 @@ public void testCreateConnectorAlreadyExists() throws
Throwable {
// Second should fail
FutureCallback> failedCreateCallback =
new FutureCallback<>();
herder.putConnectorConfig(CONNECTOR_NAME, config, false,
failedCreateCallback);
+verify(loaderSwap, times(2)).close();
Review Comment:
Done and pushed.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1383698647
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -1256,4 +1120,11 @@ private abstract class BogusSinkConnector extends
SinkConnector {
private abstract class BogusSinkTask extends SourceTask {
}
+private void verifyConnectorStatusRestart() {
+ArgumentCaptor connectorStatus =
ArgumentCaptor.forClass(ConnectorStatus.class);
+verify(statusBackingStore,
atLeastOnce()).put(connectorStatus.capture());
Review Comment:
Fixed and pushed.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1383698000
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,61 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
Review Comment:
Fixed and pushed.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1383692832
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,61 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
Review Comment:
That makes a lot more sense, the github diff misled me once again.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1383690782
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -223,93 +216,81 @@ public void testCreateConnectorAlreadyExists() throws
Throwable {
// Second should fail
FutureCallback> failedCreateCallback =
new FutureCallback<>();
herder.putConnectorConfig(CONNECTOR_NAME, config, false,
failedCreateCallback);
+verify(loaderSwap, times(2)).close();
Review Comment:
Can we move this to the end of the test, or at least after invoking
`failedCreateCallback::get`? Otherwise the test is racy since we're not
guaranteed that `loaderSwap::close` has actually been invoked twice at this
point.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1383657133
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,61 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
Review Comment:
I was suggesting we remove the direct calls to `Herder::onRestart`, not the
mocking for `shouldRestartConnector`, `buildRestartPlan`, etc.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1382163078
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,61 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
Review Comment:
@C0urante So either I am misunderstanding or doing something wrong, but when
I remove these 4 lines i.e.
```java
when(restartPlan.shouldRestartConnector()).thenReturn(true);
when(restartPlan.shouldRestartTasks()).thenReturn(false);
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
```
With the current latest state of the branch I get the following
```
[2023-11-03 21:17:12,968] INFO SinkConnectorConfig values:
config.action.reload = restart
connector.class =
org.apache.kafka.connect.runtime.standalone.StandaloneHerderTest$BogusSinkConnector
errors.deadletterqueue.context.headers.enable = false
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1382134025
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -281,45 +274,38 @@ public void testDestroyConnector() throws Exception {
} catch (ExecutionException e) {
assertTrue(e.getCause() instanceof NotFoundException);
}
Review Comment:
Resolving this as fixed, feel free to unresolve if there are still problems.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1382133745
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -127,94 +134,86 @@ private enum SourceSink {
noneConnectorClientConfigOverridePolicy = new
SampleConnectorClientConfigOverridePolicy();
@Before
-public void setup() {
-worker = PowerMock.createMock(Worker.class);
-String[] methodNames = new String[]{"connectorType",
"buildRestartPlan", "recordRestarting"};
-herder = PowerMock.createPartialMock(StandaloneHerder.class,
methodNames,
-worker, WORKER_ID, KAFKA_CLUSTER_ID, statusBackingStore, new
MemoryConfigBackingStore(transformer), noneConnectorClientConfigOverridePolicy,
new MockTime());
+public void setup() throws ExecutionException, InterruptedException {
+worker = mock(Worker.class);
+herder = mock(StandaloneHerder.class, withSettings()
+.useConstructor(worker, WORKER_ID, KAFKA_CLUSTER_ID,
statusBackingStore, new MemoryConfigBackingStore(transformer),
noneConnectorClientConfigOverridePolicy, new MockTime())
+.defaultAnswer(CALLS_REAL_METHODS));
createCallback = new FutureCallback<>();
-plugins = PowerMock.createMock(Plugins.class);
-pluginLoader = PowerMock.createMock(PluginClassLoader.class);
-loaderSwap = PowerMock.createMock(LoaderSwap.class);
-PowerMock.mockStatic(WorkerConnector.class);
-Capture> configCapture = Capture.newInstance();
-EasyMock.expect(transformer.transform(eq(CONNECTOR_NAME),
EasyMock.capture(configCapture))).andAnswer(configCapture::getValue).anyTimes();
+plugins = mock(Plugins.class);
+pluginLoader = mock(PluginClassLoader.class);
+loaderSwap = mock(LoaderSwap.class);
+final ArgumentCaptor> configCapture =
ArgumentCaptor.forClass(Map.class);
+when(transformer.transform(eq(CONNECTOR_NAME),
configCapture.capture())).thenAnswer(invocation -> configCapture.getValue());
+}
+
+@After
+public void tearDown() {
+verifyNoMoreInteractions(worker, statusBackingStore);
}
@Test
public void testCreateSourceConnector() throws Exception {
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
Map config = connectorConfig(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, config);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, config, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
-
-PowerMock.verifyAll();
}
@Test
public void testCreateConnectorFailedValidation() {
// Basic validation should be performed and return an error, but
should still evaluate the connector's config
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
Map config = connectorConfig(SourceSink.SOURCE);
config.remove(ConnectorConfig.NAME_CONFIG);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
-
EasyMock.expect(worker.configTransformer()).andReturn(transformer).times(2);
-final Capture> configCapture =
EasyMock.newCapture();
-
EasyMock.expect(transformer.transform(EasyMock.capture(configCapture))).andAnswer(configCapture::getValue);
-EasyMock.expect(worker.getPlugins()).andReturn(plugins).times(4);
-
EasyMock.expect(plugins.newConnector(EasyMock.anyString())).andReturn(connectorMock);
-
EasyMock.expect(plugins.connectorLoader(EasyMock.anyString())).andReturn(pluginLoader);
-
EasyMock.expect(plugins.withClassLoader(pluginLoader)).andReturn(loaderSwap);
+Connector connectorMock = mock(SourceConnector.class);
+when(worker.configTransformer()).thenReturn(transformer);
+final ArgumentCaptor> configCapture =
ArgumentCaptor.forClass(Map.class);
+
when(transformer.transform(configCapture.capture())).thenAnswer(invocation ->
configCapture.getValue());
+when(worker.getPlugins()).thenReturn(plugins);
+when(plugins.newConnector(anyString())).thenReturn(connectorMock);
+when(plugins.connectorLoader(anyString())).thenReturn(pluginLoader);
+when(plugins.withClassLoader(pluginLoader)).thenReturn(loaderSwap);
-EasyMock.expect(connectorMock.config()).andStubReturn(new ConfigDef());
+when(connectorMock.config()).thenReturn(new ConfigDef());
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1378185193
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -281,45 +274,38 @@ public void testDestroyConnector() throws Exception {
} catch (ExecutionException e) {
assertTrue(e.getCause() instanceof NotFoundException);
}
Review Comment:
Thanks for the pickup, I have rebased the PR with the fix (see
https://github.com/apache/kafka/pull/12728/commits/784063310fe4676a8c57b73a566985ece25d0bc7).
Resolve the conversation if its all good.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1377879325
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -281,45 +274,38 @@ public void testDestroyConnector() throws Exception {
} catch (ExecutionException e) {
assertTrue(e.getCause() instanceof NotFoundException);
}
Review Comment:
The second line from my suggestion is still missing (which is important
since the cause of the `ExecutionException` could be anything right now, and we
need to assert that it's a `NotFoundException` instead).
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1376569126
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -602,36 +546,37 @@ public void testRestartConnectorAndTasksOnlyConnector()
throws Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
+
+verifyConnectorStatusRestart();
}
@Test
public void testRestartConnectorAndTasksOnlyTasks() throws Exception {
ConnectorTaskId taskId = new ConnectorTaskId(CONNECTOR_NAME, 0);
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.restartTaskCount()).andReturn(1).anyTimes();
-EasyMock.expect(restartPlan.totalTaskCount()).andReturn(1).anyTimes();
-
EasyMock.expect(restartPlan.taskIdsToRestart()).andReturn(Collections.singletonList(taskId)).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(true);
+when(restartPlan.restartTaskCount()).thenReturn(1);
+when(restartPlan.totalTaskCount()).thenReturn(1);
+
when(restartPlan.taskIdsToRestart()).thenReturn(Collections.singletonList(taskId));
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(taskId);
-EasyMock.expectLastCall();
+verify(statusBackingStore).put(new TaskStatus(new
ConnectorTaskId(CONNECTOR_NAME, 0), AbstractStatus.State.RESTARTING, WORKER_ID,
0));
-connector = PowerMock.createMock(BogusSinkConnector.class);
+doNothing().when(herder).onRestart(taskId);
Review Comment:
Last comment was a misnomer, committed and pushed.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1376522158
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -127,94 +134,86 @@ private enum SourceSink {
noneConnectorClientConfigOverridePolicy = new
SampleConnectorClientConfigOverridePolicy();
@Before
-public void setup() {
-worker = PowerMock.createMock(Worker.class);
-String[] methodNames = new String[]{"connectorType",
"buildRestartPlan", "recordRestarting"};
-herder = PowerMock.createPartialMock(StandaloneHerder.class,
methodNames,
-worker, WORKER_ID, KAFKA_CLUSTER_ID, statusBackingStore, new
MemoryConfigBackingStore(transformer), noneConnectorClientConfigOverridePolicy,
new MockTime());
+public void setup() throws ExecutionException, InterruptedException {
+worker = mock(Worker.class);
+herder = mock(StandaloneHerder.class, withSettings()
+.useConstructor(worker, WORKER_ID, KAFKA_CLUSTER_ID,
statusBackingStore, new MemoryConfigBackingStore(transformer),
noneConnectorClientConfigOverridePolicy, new MockTime())
+.defaultAnswer(CALLS_REAL_METHODS));
createCallback = new FutureCallback<>();
-plugins = PowerMock.createMock(Plugins.class);
-pluginLoader = PowerMock.createMock(PluginClassLoader.class);
-loaderSwap = PowerMock.createMock(LoaderSwap.class);
-PowerMock.mockStatic(WorkerConnector.class);
-Capture> configCapture = Capture.newInstance();
-EasyMock.expect(transformer.transform(eq(CONNECTOR_NAME),
EasyMock.capture(configCapture))).andAnswer(configCapture::getValue).anyTimes();
+plugins = mock(Plugins.class);
+pluginLoader = mock(PluginClassLoader.class);
+loaderSwap = mock(LoaderSwap.class);
+final ArgumentCaptor> configCapture =
ArgumentCaptor.forClass(Map.class);
+when(transformer.transform(eq(CONNECTOR_NAME),
configCapture.capture())).thenAnswer(invocation -> configCapture.getValue());
+}
+
+@After
+public void tearDown() {
+verifyNoMoreInteractions(worker, statusBackingStore);
}
@Test
public void testCreateSourceConnector() throws Exception {
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
Map config = connectorConfig(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, config);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, config, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
-
-PowerMock.verifyAll();
}
@Test
public void testCreateConnectorFailedValidation() {
// Basic validation should be performed and return an error, but
should still evaluate the connector's config
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
Map config = connectorConfig(SourceSink.SOURCE);
config.remove(ConnectorConfig.NAME_CONFIG);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
-
EasyMock.expect(worker.configTransformer()).andReturn(transformer).times(2);
-final Capture> configCapture =
EasyMock.newCapture();
-
EasyMock.expect(transformer.transform(EasyMock.capture(configCapture))).andAnswer(configCapture::getValue);
-EasyMock.expect(worker.getPlugins()).andReturn(plugins).times(4);
-
EasyMock.expect(plugins.newConnector(EasyMock.anyString())).andReturn(connectorMock);
-
EasyMock.expect(plugins.connectorLoader(EasyMock.anyString())).andReturn(pluginLoader);
-
EasyMock.expect(plugins.withClassLoader(pluginLoader)).andReturn(loaderSwap);
+Connector connectorMock = mock(SourceConnector.class);
+when(worker.configTransformer()).thenReturn(transformer);
+final ArgumentCaptor> configCapture =
ArgumentCaptor.forClass(Map.class);
+
when(transformer.transform(configCapture.capture())).thenAnswer(invocation ->
configCapture.getValue());
+when(worker.getPlugins()).thenReturn(plugins);
+when(plugins.newConnector(anyString())).thenReturn(connectorMock);
+when(plugins.connectorLoader(anyString())).thenReturn(pluginLoader);
+when(plugins.withClassLoader(pluginLoader)).thenReturn(loaderSwap);
-EasyMock.expect(connectorMock.config()).andStubReturn(new ConfigDef());
+when(connectorMock.config()).thenReturn(new ConfigDef());
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1376517329
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -281,45 +274,38 @@ public void testDestroyConnector() throws Exception {
} catch (ExecutionException e) {
assertTrue(e.getCause() instanceof NotFoundException);
}
Review Comment:
Resolved and pushed.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1376516820
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -281,45 +274,38 @@ public void testDestroyConnector() throws Exception {
} catch (ExecutionException e) {
assertTrue(e.getCause() instanceof NotFoundException);
}
-
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorSameTaskConfigs() throws Exception {
expectAdd(SourceSink.SOURCE);
Map config = connectorConfig(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, config);
-worker.stopAndAwaitConnector(CONNECTOR_NAME);
-EasyMock.expectLastCall();
-
-Capture> onStart = EasyMock.newCapture();
-worker.startConnector(eq(CONNECTOR_NAME), eq(config),
EasyMock.anyObject(HerderConnectorContext.class),
-eq(herder), eq(TargetState.STARTED), EasyMock.capture(onStart));
-EasyMock.expectLastCall().andAnswer(() -> {
+final ArgumentCaptor> onStart =
ArgumentCaptor.forClass(Callback.class);
+doAnswer(invocation -> {
onStart.getValue().onCompletion(null, TargetState.STARTED);
return true;
-});
+}).when(worker).startConnector(eq(CONNECTOR_NAME), eq(config),
any(HerderConnectorContext.class),
+eq(herder), eq(TargetState.STARTED), onStart.capture());
-
EasyMock.expect(worker.connectorNames()).andReturn(Collections.singleton(CONNECTOR_NAME));
-EasyMock.expect(worker.getPlugins()).andReturn(plugins);
+
when(worker.connectorNames()).thenReturn(Collections.singleton(CONNECTOR_NAME));
+when(worker.getPlugins()).thenReturn(plugins);
// same task configs as earlier, so don't expect a new set of tasks to
be brought up
-EasyMock.expect(worker.connectorTaskConfigs(CONNECTOR_NAME, new
SourceConnectorConfig(plugins, config,
true))).andReturn(Collections.singletonList(taskConfig(SourceSink.SOURCE)));
-
-PowerMock.replayAll();
+when(worker.connectorTaskConfigs(CONNECTOR_NAME, new
SourceConnectorConfig(plugins, config, true)))
+
.thenReturn(Collections.singletonList(taskConfig(SourceSink.SOURCE)));
herder.putConnectorConfig(CONNECTOR_NAME, config, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
FutureCallback restartCallback = new FutureCallback<>();
herder.restartConnector(CONNECTOR_NAME, restartCallback);
+worker.stopAndAwaitConnector(CONNECTOR_NAME);
Review Comment:
Resolved and pushed.
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -281,45 +274,38 @@ public void testDestroyConnector() throws Exception {
} catch (ExecutionException e) {
assertTrue(e.getCause() instanceof NotFoundException);
}
-
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorSameTaskConfigs() throws Exception {
expectAdd(SourceSink.SOURCE);
Map config = connectorConfig(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, config);
-worker.stopAndAwaitConnector(CONNECTOR_NAME);
-EasyMock.expectLastCall();
-
-Capture> onStart = EasyMock.newCapture();
-worker.startConnector(eq(CONNECTOR_NAME), eq(config),
EasyMock.anyObject(HerderConnectorContext.class),
-eq(herder), eq(TargetState.STARTED), EasyMock.capture(onStart));
-EasyMock.expectLastCall().andAnswer(() -> {
+final ArgumentCaptor> onStart =
ArgumentCaptor.forClass(Callback.class);
+doAnswer(invocation -> {
onStart.getValue().onCompletion(null, TargetState.STARTED);
return true;
-});
+}).when(worker).startConnector(eq(CONNECTOR_NAME), eq(config),
any(HerderConnectorContext.class),
+eq(herder), eq(TargetState.STARTED), onStart.capture());
-
EasyMock.expect(worker.connectorNames()).andReturn(Collections.singleton(CONNECTOR_NAME));
-EasyMock.expect(worker.getPlugins()).andReturn(plugins);
+
when(worker.connectorNames()).thenReturn(Collections.singleton(CONNECTOR_NAME));
+when(worker.getPlugins()).thenReturn(plugins);
// same task configs as earlier, so don't expect a new set of tasks to
be brought up
-
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1376517843
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -127,94 +134,86 @@ private enum SourceSink {
noneConnectorClientConfigOverridePolicy = new
SampleConnectorClientConfigOverridePolicy();
@Before
-public void setup() {
-worker = PowerMock.createMock(Worker.class);
-String[] methodNames = new String[]{"connectorType",
"buildRestartPlan", "recordRestarting"};
-herder = PowerMock.createPartialMock(StandaloneHerder.class,
methodNames,
-worker, WORKER_ID, KAFKA_CLUSTER_ID, statusBackingStore, new
MemoryConfigBackingStore(transformer), noneConnectorClientConfigOverridePolicy,
new MockTime());
+public void setup() throws ExecutionException, InterruptedException {
+worker = mock(Worker.class);
+herder = mock(StandaloneHerder.class, withSettings()
+.useConstructor(worker, WORKER_ID, KAFKA_CLUSTER_ID,
statusBackingStore, new MemoryConfigBackingStore(transformer),
noneConnectorClientConfigOverridePolicy, new MockTime())
+.defaultAnswer(CALLS_REAL_METHODS));
createCallback = new FutureCallback<>();
-plugins = PowerMock.createMock(Plugins.class);
-pluginLoader = PowerMock.createMock(PluginClassLoader.class);
-loaderSwap = PowerMock.createMock(LoaderSwap.class);
-PowerMock.mockStatic(WorkerConnector.class);
-Capture> configCapture = Capture.newInstance();
-EasyMock.expect(transformer.transform(eq(CONNECTOR_NAME),
EasyMock.capture(configCapture))).andAnswer(configCapture::getValue).anyTimes();
+plugins = mock(Plugins.class);
+pluginLoader = mock(PluginClassLoader.class);
+loaderSwap = mock(LoaderSwap.class);
+final ArgumentCaptor> configCapture =
ArgumentCaptor.forClass(Map.class);
+when(transformer.transform(eq(CONNECTOR_NAME),
configCapture.capture())).thenAnswer(invocation -> configCapture.getValue());
+}
+
+@After
+public void tearDown() {
+verifyNoMoreInteractions(worker, statusBackingStore);
}
@Test
public void testCreateSourceConnector() throws Exception {
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
Map config = connectorConfig(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, config);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, config, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
-
-PowerMock.verifyAll();
}
@Test
public void testCreateConnectorFailedValidation() {
// Basic validation should be performed and return an error, but
should still evaluate the connector's config
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
Map config = connectorConfig(SourceSink.SOURCE);
config.remove(ConnectorConfig.NAME_CONFIG);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
-
EasyMock.expect(worker.configTransformer()).andReturn(transformer).times(2);
-final Capture> configCapture =
EasyMock.newCapture();
-
EasyMock.expect(transformer.transform(EasyMock.capture(configCapture))).andAnswer(configCapture::getValue);
-EasyMock.expect(worker.getPlugins()).andReturn(plugins).times(4);
-
EasyMock.expect(plugins.newConnector(EasyMock.anyString())).andReturn(connectorMock);
-
EasyMock.expect(plugins.connectorLoader(EasyMock.anyString())).andReturn(pluginLoader);
-
EasyMock.expect(plugins.withClassLoader(pluginLoader)).andReturn(loaderSwap);
+Connector connectorMock = mock(SourceConnector.class);
+when(worker.configTransformer()).thenReturn(transformer);
+final ArgumentCaptor> configCapture =
ArgumentCaptor.forClass(Map.class);
+
when(transformer.transform(configCapture.capture())).thenAnswer(invocation ->
configCapture.getValue());
+when(worker.getPlugins()).thenReturn(plugins);
+when(plugins.newConnector(anyString())).thenReturn(connectorMock);
+when(plugins.connectorLoader(anyString())).thenReturn(pluginLoader);
+when(plugins.withClassLoader(pluginLoader)).thenReturn(loaderSwap);
-EasyMock.expect(connectorMock.config()).andStubReturn(new ConfigDef());
+when(connectorMock.config()).thenReturn(new ConfigDef());
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1376493619
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -656,48 +600,40 @@ public void testRestartConnectorAndTasksOnlyTasks()
throws Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
Review Comment:
Done and pushed
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1376484462
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -602,36 +546,37 @@ public void testRestartConnectorAndTasksOnlyConnector()
throws Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
+
+verifyConnectorStatusRestart();
}
@Test
public void testRestartConnectorAndTasksOnlyTasks() throws Exception {
ConnectorTaskId taskId = new ConnectorTaskId(CONNECTOR_NAME, 0);
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.restartTaskCount()).andReturn(1).anyTimes();
-EasyMock.expect(restartPlan.totalTaskCount()).andReturn(1).anyTimes();
-
EasyMock.expect(restartPlan.taskIdsToRestart()).andReturn(Collections.singletonList(taskId)).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(true);
+when(restartPlan.restartTaskCount()).thenReturn(1);
+when(restartPlan.totalTaskCount()).thenReturn(1);
+
when(restartPlan.taskIdsToRestart()).thenReturn(Collections.singletonList(taskId));
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(taskId);
-EasyMock.expectLastCall();
+verify(statusBackingStore).put(new TaskStatus(new
ConnectorTaskId(CONNECTOR_NAME, 0), AbstractStatus.State.RESTARTING, WORKER_ID,
0));
-connector = PowerMock.createMock(BogusSinkConnector.class);
+doNothing().when(herder).onRestart(taskId);
Review Comment:
Just like above i.e.
https://github.com/apache/kafka/pull/12728#discussion_r1376483865 this causes
`tearDown` to fail.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1376483865
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,61 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
Review Comment:
This causes `tearDown` to fail, i.e.
https://github.com/apache/kafka/pull/12728/files#diff-9143727aad47387b8283893ef39610c7f135264077f795f70a6f1d67f7db6e21R151-R153,
shall I remove `tearDown`
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1376416990
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,61 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
Review Comment:
We can remove this.
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -602,36 +546,37 @@ public void testRestartConnectorAndTasksOnlyConnector()
throws Exception {
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
+
+verifyConnectorStatusRestart();
}
@Test
public void testRestartConnectorAndTasksOnlyTasks() throws Exception {
ConnectorTaskId taskId = new ConnectorTaskId(CONNECTOR_NAME, 0);
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false,
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1374442966
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -127,94 +134,86 @@ private enum SourceSink {
noneConnectorClientConfigOverridePolicy = new
SampleConnectorClientConfigOverridePolicy();
@Before
-public void setup() {
-worker = PowerMock.createMock(Worker.class);
-String[] methodNames = new String[]{"connectorType",
"buildRestartPlan", "recordRestarting"};
-herder = PowerMock.createPartialMock(StandaloneHerder.class,
methodNames,
-worker, WORKER_ID, KAFKA_CLUSTER_ID, statusBackingStore, new
MemoryConfigBackingStore(transformer), noneConnectorClientConfigOverridePolicy,
new MockTime());
+public void setup() throws ExecutionException, InterruptedException {
+worker = mock(Worker.class);
+herder = mock(StandaloneHerder.class, withSettings()
+.useConstructor(worker, WORKER_ID, KAFKA_CLUSTER_ID,
statusBackingStore, new MemoryConfigBackingStore(transformer),
noneConnectorClientConfigOverridePolicy, new MockTime())
+.defaultAnswer(CALLS_REAL_METHODS));
createCallback = new FutureCallback<>();
-plugins = PowerMock.createMock(Plugins.class);
-pluginLoader = PowerMock.createMock(PluginClassLoader.class);
-loaderSwap = PowerMock.createMock(LoaderSwap.class);
-PowerMock.mockStatic(WorkerConnector.class);
-Capture> configCapture = Capture.newInstance();
-EasyMock.expect(transformer.transform(eq(CONNECTOR_NAME),
EasyMock.capture(configCapture))).andAnswer(configCapture::getValue).anyTimes();
+plugins = mock(Plugins.class);
+pluginLoader = mock(PluginClassLoader.class);
+loaderSwap = mock(LoaderSwap.class);
+final ArgumentCaptor> configCapture =
ArgumentCaptor.forClass(Map.class);
+when(transformer.transform(eq(CONNECTOR_NAME),
configCapture.capture())).thenAnswer(invocation -> configCapture.getValue());
+}
+
+@After
+public void tearDown() {
+verifyNoMoreInteractions(worker, statusBackingStore);
}
@Test
public void testCreateSourceConnector() throws Exception {
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
Map config = connectorConfig(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, config);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, config, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
-
-PowerMock.verifyAll();
}
@Test
-public void testCreateConnectorFailedValidation() {
+public void testCreateConnectorFailedValidation() throws
ExecutionException, InterruptedException {
Review Comment:
Resolved and pushed.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1374443260
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -817,19 +731,11 @@ public void testAccessors() throws Exception {
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
-EasyMock.reset(transformer);
-EasyMock.expect(transformer.transform(eq(CONNECTOR_NAME),
EasyMock.anyObject()))
Review Comment:
@C0urante You have an opinion on this?
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1374442341
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -838,39 +744,33 @@ public void testPutConnectorConfig() throws Exception {
Map newConnConfig = new HashMap<>(connConfig);
newConnConfig.put("foo", "bar");
-Callback> connectorConfigCb =
PowerMock.createMock(Callback.class);
-// Callback> putConnectorConfigCb =
PowerMock.createMock(Callback.class);
+Callback> connectorConfigCb = mock(Callback.class);
+// Callback> putConnectorConfigCb =
mock(Callback.class);
Review Comment:
Done
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mimaison commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1374348110
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -127,94 +134,86 @@ private enum SourceSink {
noneConnectorClientConfigOverridePolicy = new
SampleConnectorClientConfigOverridePolicy();
@Before
-public void setup() {
-worker = PowerMock.createMock(Worker.class);
-String[] methodNames = new String[]{"connectorType",
"buildRestartPlan", "recordRestarting"};
-herder = PowerMock.createPartialMock(StandaloneHerder.class,
methodNames,
-worker, WORKER_ID, KAFKA_CLUSTER_ID, statusBackingStore, new
MemoryConfigBackingStore(transformer), noneConnectorClientConfigOverridePolicy,
new MockTime());
+public void setup() throws ExecutionException, InterruptedException {
+worker = mock(Worker.class);
+herder = mock(StandaloneHerder.class, withSettings()
+.useConstructor(worker, WORKER_ID, KAFKA_CLUSTER_ID,
statusBackingStore, new MemoryConfigBackingStore(transformer),
noneConnectorClientConfigOverridePolicy, new MockTime())
+.defaultAnswer(CALLS_REAL_METHODS));
createCallback = new FutureCallback<>();
-plugins = PowerMock.createMock(Plugins.class);
-pluginLoader = PowerMock.createMock(PluginClassLoader.class);
-loaderSwap = PowerMock.createMock(LoaderSwap.class);
-PowerMock.mockStatic(WorkerConnector.class);
-Capture> configCapture = Capture.newInstance();
-EasyMock.expect(transformer.transform(eq(CONNECTOR_NAME),
EasyMock.capture(configCapture))).andAnswer(configCapture::getValue).anyTimes();
+plugins = mock(Plugins.class);
+pluginLoader = mock(PluginClassLoader.class);
+loaderSwap = mock(LoaderSwap.class);
+final ArgumentCaptor> configCapture =
ArgumentCaptor.forClass(Map.class);
+when(transformer.transform(eq(CONNECTOR_NAME),
configCapture.capture())).thenAnswer(invocation -> configCapture.getValue());
+}
+
+@After
+public void tearDown() {
+verifyNoMoreInteractions(worker, statusBackingStore);
}
@Test
public void testCreateSourceConnector() throws Exception {
-connector = PowerMock.createMock(BogusSourceConnector.class);
+connector = mock(BogusSourceConnector.class);
expectAdd(SourceSink.SOURCE);
Map config = connectorConfig(SourceSink.SOURCE);
-Connector connectorMock = PowerMock.createMock(SourceConnector.class);
+Connector connectorMock = mock(SourceConnector.class);
expectConfigValidation(connectorMock, true, config);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, config, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
-
-PowerMock.verifyAll();
}
@Test
-public void testCreateConnectorFailedValidation() {
+public void testCreateConnectorFailedValidation() throws
ExecutionException, InterruptedException {
Review Comment:
It looks like we don't need `throws ExecutionException, InterruptedException`
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -838,39 +744,33 @@ public void testPutConnectorConfig() throws Exception {
Map newConnConfig = new HashMap<>(connConfig);
newConnConfig.put("foo", "bar");
-Callback> connectorConfigCb =
PowerMock.createMock(Callback.class);
-// Callback> putConnectorConfigCb =
PowerMock.createMock(Callback.class);
+Callback> connectorConfigCb = mock(Callback.class);
+// Callback> putConnectorConfigCb =
mock(Callback.class);
Review Comment:
Should we delete this comment?
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -817,19 +731,11 @@ public void testAccessors() throws Exception {
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SOURCE), connectorInfo.result());
-EasyMock.reset(transformer);
-EasyMock.expect(transformer.transform(eq(CONNECTOR_NAME),
EasyMock.anyObject()))
Review Comment:
Should we keep this assertion?
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1374168471
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -617,8 +616,7 @@ public void testRestartConnectorAndTasksBoth() throws
Exception {
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
-doNothing().when(herder).onRestart(CONNECTOR_NAME);
-doNothing().when(herder).onRestart(taskId);
+ArgumentCaptor taskStatus =
ArgumentCaptor.forClass(TaskStatus.class);
Review Comment:
I had to add this otherwise
https://github.com/apache/kafka/pull/12728/files#diff-9143727aad47387b8283893ef39610c7f135264077f795f70a6f1d67f7db6e21R151-R153
would fail
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1782422797 @C0urante Commit pushed resolving your latest comments -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1374165166
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,64 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
-EasyMock.expectLastCall();
+verify(statusBackingStore).put(new ConnectorStatus(CONNECTOR_NAME,
ConnectorStatus.State.RESTARTING, WORKER_ID, 0));
-connector = PowerMock.createMock(BogusSinkConnector.class);
+doNothing().when(herder).onRestart(CONNECTOR_NAME);
Review Comment:
So it ended up working in the end, see latest commit i.e. `Don't mock
doNothing().when(herder).onRestart(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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1374163842
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,64 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
-EasyMock.expectLastCall();
+verify(statusBackingStore).put(new ConnectorStatus(CONNECTOR_NAME,
ConnectorStatus.State.RESTARTING, WORKER_ID, 0));
Review Comment:
Okay without some slight adjustments I can indeed remove this verification,
all good
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1374138454
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,64 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
-EasyMock.expectLastCall();
+verify(statusBackingStore).put(new ConnectorStatus(CONNECTOR_NAME,
ConnectorStatus.State.RESTARTING, WORKER_ID, 0));
-connector = PowerMock.createMock(BogusSinkConnector.class);
+doNothing().when(herder).onRestart(CONNECTOR_NAME);
Review Comment:
So a slightly modified version of
```java
ArgumentCaptor connectorStatus =
ArgumentCaptor.forClass(ConnectorStatus.class);
verify(statusBackingStore, atLeastOnce()).put(connectorStatus.capture());
assertEquals(CONNECTOR_NAME, connectorStatus.getValue().id());
assertEquals(AbstractStatus.State.RESTARTING,
connectorStatus.getValue().state());
```
(notice the `atLeastOnce()`) does seem to work, lets see if I can add it in
the other places.
--
This is an automated message
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1374136601
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,64 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
-EasyMock.expectLastCall();
+verify(statusBackingStore).put(new ConnectorStatus(CONNECTOR_NAME,
ConnectorStatus.State.RESTARTING, WORKER_ID, 0));
Review Comment:
This verification is required because we have
https://github.com/apache/kafka/pull/12728/files#diff-9143727aad47387b8283893ef39610c7f135264077f795f70a6f1d67f7db6e21R151-R153
(this is something that you asked to add when the PR was first being required).
Without it the test fails, ill see if the `RESTARTING` state fixes this in
another way.
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1374136601
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,64 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
-EasyMock.expectLastCall();
+verify(statusBackingStore).put(new ConnectorStatus(CONNECTOR_NAME,
ConnectorStatus.State.RESTARTING, WORKER_ID, 0));
Review Comment:
This verification is required because we have
https://github.com/apache/kafka/pull/12728/files#diff-9143727aad47387b8283893ef39610c7f135264077f795f70a6f1d67f7db6e21R151-R153
(this is something that you asked to add when the PR was first being required).
Without it the test fails
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1373831428
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,64 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
-EasyMock.expectLastCall();
+verify(statusBackingStore).put(new ConnectorStatus(CONNECTOR_NAME,
ConnectorStatus.State.RESTARTING, WORKER_ID, 0));
Review Comment:
I think we can just get rid of this part? This isn't really relevant to this
test case and it doesn't provide super strong guarantees (especially if we
explicitly verify that we're emitting the `RESTARTING` state in the status
store like suggested a few lines below).
--
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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
C0urante commented on code in PR #12728:
URL: https://github.com/apache/kafka/pull/12728#discussion_r1373830704
##
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##
@@ -528,72 +483,64 @@ public void testRestartConnectorAndTasksNoStatus() throws
Exception {
ExecutionException ee = assertThrows(ExecutionException.class, () ->
restartCallback.get(1000L, TimeUnit.MILLISECONDS));
assertTrue(ee.getCause() instanceof NotFoundException);
assertTrue(ee.getMessage().contains("Status for connector"));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksNoRestarts() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
-
-connector = PowerMock.createMock(BogusSinkConnector.class);
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(false);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
+
+connector = mock(BogusSinkConnector.class);
expectAdd(SourceSink.SINK);
Map connectorConfig = connectorConfig(SourceSink.SINK);
-Connector connectorMock = PowerMock.createMock(SinkConnector.class);
+Connector connectorMock = mock(SinkConnector.class);
expectConfigValidation(connectorMock, true, connectorConfig);
-PowerMock.replayAll();
-
herder.putConnectorConfig(CONNECTOR_NAME, connectorConfig, false,
createCallback);
Herder.Created connectorInfo =
createCallback.get(1000L, TimeUnit.SECONDS);
assertEquals(createdInfo(SourceSink.SINK), connectorInfo.result());
FutureCallback restartCallback = new
FutureCallback<>();
herder.restartConnectorAndTasks(restartRequest, restartCallback);
assertEquals(connectorStateInfo, restartCallback.get(1000L,
TimeUnit.MILLISECONDS));
-PowerMock.verifyAll();
}
@Test
public void testRestartConnectorAndTasksOnlyConnector() throws Exception {
RestartRequest restartRequest = new RestartRequest(CONNECTOR_NAME,
false, true);
-RestartPlan restartPlan = PowerMock.createMock(RestartPlan.class);
-ConnectorStateInfo connectorStateInfo =
PowerMock.createMock(ConnectorStateInfo.class);
-
EasyMock.expect(restartPlan.shouldRestartConnector()).andReturn(true).anyTimes();
-
EasyMock.expect(restartPlan.shouldRestartTasks()).andReturn(false).anyTimes();
-
EasyMock.expect(restartPlan.restartConnectorStateInfo()).andReturn(connectorStateInfo).anyTimes();
-EasyMock.expect(herder.buildRestartPlan(restartRequest))
-.andReturn(Optional.of(restartPlan)).anyTimes();
+RestartPlan restartPlan = mock(RestartPlan.class);
+ConnectorStateInfo connectorStateInfo = mock(ConnectorStateInfo.class);
+when(restartPlan.shouldRestartConnector()).thenReturn(true);
+when(restartPlan.shouldRestartTasks()).thenReturn(false);
+
when(restartPlan.restartConnectorStateInfo()).thenReturn(connectorStateInfo);
+
doReturn(Optional.of(restartPlan)).when(herder).buildRestartPlan(restartRequest);
herder.onRestart(CONNECTOR_NAME);
-EasyMock.expectLastCall();
+verify(statusBackingStore).put(new ConnectorStatus(CONNECTOR_NAME,
ConnectorStatus.State.RESTARTING, WORKER_ID, 0));
-connector = PowerMock.createMock(BogusSinkConnector.class);
+doNothing().when(herder).onRestart(CONNECTOR_NAME);
Review Comment:
Instead of mocking this method on the class we're testing, can we let the
real method be invoked and add a verification at the end of the test? It could
be something like this:
```java
ArgumentCaptor connectorStatus =
ArgumentCaptor.forClass(ConnectorStatus.class);
verify(statusBackingStore).put(connectorStatus.capture());
assertEquals(CONNECTOR_NAME, connectorStatus.getValue().id());
assertEquals(AbstractStatus.State.RESTARTING,
connectorStatus.getValue().state());
```
We can
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
ijuma commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1779282256 @mimaison Do you have the cycles to review this? -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1779161332 @mimaison @ijuma I have just finished PR and I can confirm the tests are passing locally. Due to the fact we are no longer dealing with static classes, the setting up of the static mocks on the `connectorExecutor` is no longer needed. -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1775892079 @mimaison I started rebasing across `trunk` and under-estimated the amount of changes that have since landed so this may take a day or two longer -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1773039378 > @mdedetrich Sorry I just notice now that I was "can _we_ rebase", I meant "can _you_ rebase". If you're not able to complete this PR, let me know. I'll have it done by Monday -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mimaison commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1772979943 @mdedetrich Sorry I just notice now that I was "can _we_ rebase", I meant "can _you_ rebase". If you're not able to complete this PR, let me know. -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
ijuma commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1766441852 It would be nice to get this over the line. It's one of only 3 tests that are excluded when we run with Java 17 or 21. -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mdedetrich commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1764446152 > @mdedetrich Can we rebase on trunk to resolve the conflicts? Thanks Of course! -- 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
Re: [PR] KAFKA-14132; Replace EasyMock with Mockito in StandaloneHerderTest [kafka]
mimaison commented on PR #12728: URL: https://github.com/apache/kafka/pull/12728#issuecomment-1764076510 @mdedetrich Can we rebase on trunk to resolve the conflicts? Thanks -- 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
