refactor(jkube-kit/common): Extract errorStream processing in Externa…

…lCommand enabling overrides

Related to #2453

In order to read command's errorStream during failure, we need to
refactor ExternalCommand to expose this logic so that child classes can
override this logic as per the default behavior (i.e logging it as a
warn message)

Signed-off-by: Rohan Kumar <rohaan@redhat.com>

jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/ExternalCommand.java

@@ -27,6 +27,7 @@
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.function.Consumer;
 
 import org.apache.commons.lang3.StringUtils;
 
@@ -128,18 +129,12 @@ protected String getCommandAsString() {
     protected abstract String[] getArgs();
 
     private void outputStreamPump(final InputStream inputStream) throws IOException {
-        try (BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream))) {
-            for (; ; ) {
-                String line = reader.readLine();
-                if (line == null) {
-                    break;
-                }
-                processLine(line);
-            }
+        try {
+            readInputStream(inputStream, this::processLine);
         } catch (IOException e) {
             throw new IOException(String.format("Failed to read process '%s' output: %s",
-                                                getCommandAsString(),
-                                                e.getMessage()), e);
+                getCommandAsString(),
+                e.getMessage()), e);
         }
     }
 
@@ -149,23 +144,33 @@ protected void processLine(String line) {
 
     private Future<IOException> startStreamPump(final InputStream errorStream) {
         return executor.submit(() -> {
-            try (BufferedReader reader = new BufferedReader(new InputStreamReader(errorStream))) {
-                for (; ; ) {
-                    String line = reader.readLine();
-                    if (line == null) {
-                        break;
-                    }
-                    synchronized (log) {
-                        log.warn(line);
-                    }
-                }
+            try {
+                readInputStream(errorStream, this::processError);
                 return null;
             } catch (IOException e) {
                 return e;
             }
         });
     }
 
+    private void readInputStream(final InputStream inputStream, Consumer<String> lineConsumer) throws IOException {
+        try (BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream))) {
+            for (; ; ) {
+                String line = reader.readLine();
+                if (line == null) {
+                    break;
+                }
+                lineConsumer.accept(line);
+            }
+        }
+    }
+
+    protected void processError(String line) {
+        synchronized (log) {
+            log.warn(line);
+        }
+    }
+
     private void stopStreamPump(Future<IOException> future) throws IOException {
         try {
             IOException e = future.get(2, TimeUnit.SECONDS);

jkube-kit/common/src/test/java/org/eclipse/jkube/kit/common/ExternalCommandTest.java

@@ -13,16 +13,24 @@
  */
 package org.eclipse.jkube.kit.common;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 
 import java.io.File;
 import java.io.IOException;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIOException;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 
 class ExternalCommandTest {
-  private final KitLogger kitLogger = new KitLogger.SilentLogger();
+  private KitLogger kitLogger;
+
+  @BeforeEach
+  void setUp() {
+    kitLogger = spy(new KitLogger.SilentLogger());
+  }
 
   @Test
   void execute_whenCommandCompletedSuccessfully_thenPrintResult() throws IOException {
@@ -47,6 +55,36 @@ void execute_whenCommandFailed_thenThrowException() {
         .withMessage("Process 'ls idontexist' exited with status 2");
   }
 
+  @Test
+  void execute_whenCommandFailed_thenLoggerError() {
+    // Given
+    TestCommand testCommand = new TestCommand(kitLogger, new String[] {"java", "idontexist"});
+
+    // When + Then
+    assertThatIOException()
+        .isThrownBy(testCommand::execute)
+        .withMessage("Process 'java idontexist' exited with status 1");
+    verify(kitLogger).warn("Error: Could not find or load main class idontexist");
+  }
+
+  @Test
+  void execute_whenCommandFailedButProcessErrorOverridden_thenErrorNotBlank() {
+    // Given
+    final StringBuilder errorBuilder = new StringBuilder();
+    TestCommand overriddenProcessErrorTestCommand = new TestCommand(kitLogger, new String[] {"java", "idontexist"}, null) {
+      @Override
+      protected void processError(String errorLine) {
+        errorBuilder.append(errorLine);
+      }
+    };
+
+    // When + Then
+    assertThatIOException()
+        .isThrownBy(overriddenProcessErrorTestCommand::execute)
+        .withMessage("Process 'java idontexist' exited with status 1");
+    assertThat(errorBuilder.toString()).contains("Error: Could not find or load main class idontexist");
+  }
+
   @Test
   void execute_whenWorkDirProvided_thenUseWorkDir(@TempDir File temporaryFolder) throws IOException {
     // Given
@@ -59,7 +97,6 @@ void execute_whenWorkDirProvided_thenUseWorkDir(@TempDir File temporaryFolder) t
     assertThat(new File(temporaryFolder, "foo")).exists();
   }
 
-
   private static class TestCommand extends ExternalCommand {
     private String result;
     private final String[] args;
@@ -79,7 +116,7 @@ protected String[] getArgs() {
 
     @Override
     protected void processLine(String line) {
-        result = line;
+      result = line;
     }
 
     public String getResult() {