HubertWo commented on a change in pull request #7350:
URL: https://github.com/apache/pinot/pull/7350#discussion_r694121311
##########
File path:
pinot-clients/pinot-java-client/src/test/java/org/apache/pinot/client/ConnectionFactoryTest.java
##########
@@ -18,32 +18,44 @@
*/
package org.apache.pinot.client;
-import java.util.ArrayList;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Properties;
+
+import org.mockito.Mockito;
import org.testng.Assert;
import org.testng.annotations.Test;
-
/**
* Tests for the connection factory
*/
public class ConnectionFactoryTest {
@Test
public void testZkConnection() {
- // TODO Write test
// Create a dummy Helix structure
+ DynamicBrokerSelector dynamicBrokerSelector =
Mockito.mock(DynamicBrokerSelector.class);
+ PinotClientTransport pinotClientTransport =
Mockito.mock(PinotClientTransport.class);
+
// Create the connection
+ Connection connection = ConnectionFactory.fromZookeeper(
+ dynamicBrokerSelector,
+ pinotClientTransport);
+
// Check that the broker list has the right length and has the same servers
+ Assert.assertEquals(connection.getBrokerList(), List.of());
Review comment:
Testing ```DynamicBrokerSelector``` is problematic since ```ZkClient```
tries to connect in constructor. I decided to mock ```DynamicBrokerSelector```
and found that ```brokerList``` in connection is not set at all. Please check
changes in ```Connection.class```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
