gharris1727 commented on code in PR #15389:
URL: https://github.com/apache/kafka/pull/15389#discussion_r1535727231


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/standalone/StandaloneHerderTest.java:
##########
@@ -726,21 +683,21 @@ public void testAccessors() throws Exception {
         
doNothing().when(connectorConfigCb).onCompletion(any(NotFoundException.class), 
isNull());
 
         // Create connector
-        connector = mock(BogusSourceConnector.class);
+        Connector connector = mock(BogusSourceConnector.class);

Review Comment:
   > For tests like testPutConnectorConfig where the function is called twice, 
the test fails as the mock is different for each function call. I'm yet to 
debug why having the same mock is the right way but thought I'd ask you first
   
   It would help if I actually told you what this mock is doing. The strange 
two-calls, boolean argument, multiple configs is related to just this one 
computeIfAbsent call: 
https://github.com/apache/kafka/blob/159d25a7df25975694e2e0eb18a8feb125f7c39e/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java#L755
   
   Each time the `AbstractHerder` (superclass of `StandaloneHerder`) runs a 
validation and processes a config it calls `computeIfAbsent`, and only calls 
newConnector once. This single Connector instance then validates every 
configuration for that worker.
   
   So the first call to `expectConfigValidation` (with argument true) mock out 
the creation, and the following calls (with argument false) just add more 
config invocations to the existing mocked object. If you create a new mocked 
object, those are never used because the original mocked object is re-used.
   
   Rather than returning the mocked object, I think it would be better that a 
single expectConfigValidation call mocked out all of the configs in-advance, 
like `testCreateConnectorAlreadyExists` does with two `config` arguments. The 
`Map` argument in the `false` calls can be moved to the `true` calls.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to