yashmayya commented on code in PR #12615:
URL: https://github.com/apache/kafka/pull/12615#discussion_r969737990
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerTaskTest.java:
##########
@@ -78,82 +78,41 @@ public void tearDown() {
public void standardStartup() {
ConnectorTaskId taskId = new ConnectorTaskId("foo", 0);
- WorkerTask workerTask = partialMockBuilder(WorkerTask.class)
- .withConstructor(
- ConnectorTaskId.class,
- TaskStatus.Listener.class,
- TargetState.class,
- ClassLoader.class,
- ConnectMetrics.class,
- RetryWithToleranceOperator.class,
- Time.class,
- StatusBackingStore.class
- )
- .withArgs(taskId, statusListener, TargetState.STARTED, loader,
metrics,
+ WorkerTask workerTask = mock(WorkerTask.class, withSettings()
Review Comment:
> Mocking the class we're testing is a bit of an anti-pattern.
💯 agree. The only reason I left it as is was with the intention of not
introducing unrelated changes in this PR and keeping it easier to review.
> How about we introduce a TestWorkerTask class that defaults to no-ops for
all the abstract WorkerTask methods, and can be overridden on a per-test-case
basis to define other behavior? Thinking something like this, using
stopBeforeStarting as an example:
This sounds like a really good idea, I've gone ahead and implemented it.
Thanks @C0urante!
--
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
