[GitHub] [kafka] clolov commented on a diff in pull request #13122: KAFKA-14594: Move LogDirsCommand to tools module

["clolov (via GitHub)" <gi...@apache.org>]

clolov commented on code in PR #13122:
URL: https://github.com/apache/kafka/pull/13122#discussion_r1097662809


##########
clients/src/test/java/org/apache/kafka/clients/admin/MockAdminClient.java:
##########
@@ -870,7 +870,37 @@ synchronized public AlterReplicaLogDirsResult alterReplicaLogDirs(
     @Override
     synchronized public DescribeLogDirsResult describeLogDirs(Collection<Integer> brokers,

Review Comment:
   This implementation is provided because I wanted to use MockAdminClient rather than use Mockito and mock the Admin interface. I am open to suggestions for improving the current logic.



##########
tools/src/test/java/org/apache/kafka/tools/LogDirsCommandTest.java:
##########
@@ -0,0 +1,68 @@
+package org.apache.kafka.tools;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import org.apache.kafka.clients.admin.MockAdminClient;
+import org.apache.kafka.common.Node;
+import org.junit.jupiter.api.Test;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import java.io.UnsupportedEncodingException;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.concurrent.ExecutionException;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class LogDirsCommandTest {
+
+    @Test
+    public void checkLogDirsCommandOutput() throws UnsupportedEncodingException, TerseException, ExecutionException, JsonProcessingException, InterruptedException {
+        ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
+        PrintStream printStream = new PrintStream(byteArrayOutputStream, false, StandardCharsets.UTF_8.name());
+
+        PrintStream originalStandardOut = System.out;
+        System.setOut(printStream);
+
+        Node broker = new Node(1, "hostname", 9092);
+
+        try (MockAdminClient adminClient = new MockAdminClient(Collections.singletonList(broker), broker)) {

Review Comment:
   I opted to move the unit test as is, rather than improve it. If there is a preference to improve on it, I would either use a Mockito to mock the Admin interface for describeLogDirs or I would contribute a more accurate implementation to the MockAdminClient.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org