[NO ISSUE] Fix logging for ncservice-based tests

Change-Id: Id264aede0f62558ad6e34355047c623a1d594692
Reviewed-on: https://asterix-gerrit.ics.uci.edu/3311
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Ian Maxon <imaxon@uci.edu>
diff --git a/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/NCServiceIT.java b/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/NCServiceIT.java
index 5cbc5b4..cd0359f 100644
--- a/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/NCServiceIT.java
+++ b/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/NCServiceIT.java
@@ -21,11 +21,8 @@
 import java.io.File;
 import java.io.IOException;
 import java.net.InetAddress;
-import java.nio.file.Files;
-import java.nio.file.StandardOpenOption;
 import java.util.Iterator;
 
-import org.apache.commons.io.FileUtils;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
 import org.apache.http.client.HttpClient;
@@ -58,13 +55,9 @@
     @BeforeClass
     public static void setUp() throws Exception {
         cluster = new HyracksVirtualCluster(new File(APP_HOME), null);
-        File tempConf = new File(TARGET_DIR, "cc.conf");
-        FileUtils.copyFile(new File(RESOURCE_DIR, "cc.conf"), tempConf);
-        Files.write(tempConf.toPath(), ("log.dir: " + LOG_DIR).getBytes(), StandardOpenOption.APPEND);
-        File log4jPath = new File(FileUtil.joinPath("..", "..", "src", "test", "resources", "log4j2-hyracks-test.xml"));
 
-        cluster.addNCService(new File(RESOURCE_DIR, "nc-red.conf"), new File(LOG_DIR, "nc-red.log"), log4jPath);
-        cluster.addNCService(new File(RESOURCE_DIR, "nc-blue.conf"), new File(LOG_DIR, "nc-blue.log"), log4jPath);
+        cluster.addNCService(new File(RESOURCE_DIR, "nc-red.conf"), null);
+        cluster.addNCService(new File(RESOURCE_DIR, "nc-blue.conf"), null);
 
         try {
             Thread.sleep(2000);
@@ -72,7 +65,7 @@
         }
 
         // Start CC
-        cluster.start(tempConf, new File(LOG_DIR, "cc.log"), log4jPath);
+        cluster.start(new File(RESOURCE_DIR, "cc.conf"), null);
 
         try {
             Thread.sleep(10000);
diff --git a/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksCCProcess.java b/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksCCProcess.java
index a79d033..0b529fb 100644
--- a/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksCCProcess.java
+++ b/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksCCProcess.java
@@ -25,18 +25,14 @@
 
 public class HyracksCCProcess extends HyracksServerProcess {
 
-    public HyracksCCProcess(File configFile, File logFile, File appHome, File workingDir) {
+    HyracksCCProcess(File configFile, File logFile, File appHome, File workingDir) {
+        super(" cc");
         this.configFile = configFile;
         this.logFile = logFile;
         this.appHome = appHome;
         this.workingDir = workingDir;
     }
 
-    public HyracksCCProcess(File configFile, File logFile, File appHome, File workingDir, File log4jPath) {
-        this(configFile, logFile, appHome, workingDir);
-        args.add("-Dlog4j.configurationFile=file://" + log4jPath.getAbsolutePath());
-    }
-
     @Override
     protected String getMainClassName() {
         return CCDriver.class.getName();
diff --git a/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksNCServiceProcess.java b/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksNCServiceProcess.java
index d0e0244..958eb9a 100644
--- a/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksNCServiceProcess.java
+++ b/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksNCServiceProcess.java
@@ -20,23 +20,21 @@
 
 import java.io.File;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.hyracks.control.nc.service.NCService;
 
 public class HyracksNCServiceProcess extends HyracksServerProcess {
+    private static final AtomicInteger ncServiceCounter = new AtomicInteger();
 
-    public HyracksNCServiceProcess(File configFile, File logFile, File appHome, File workingDir) {
+    HyracksNCServiceProcess(File configFile, File logFile, File appHome, File workingDir) {
+        super("nc" + ncServiceCounter.incrementAndGet());
         this.configFile = configFile;
         this.logFile = logFile;
         this.appHome = appHome;
         this.workingDir = workingDir;
     }
 
-    public HyracksNCServiceProcess(File configFile, File logFile, File appHome, File workingDir, File log4jPath) {
-        this(configFile, logFile, appHome, workingDir);
-        args.add("-Dlog4j.configurationFile=file://" + log4jPath.getAbsolutePath());
-    }
-
     @Override
     protected String getMainClassName() {
         return NCService.class.getName();
diff --git a/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksServerProcess.java b/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksServerProcess.java
index a8c363b..59ab4f7 100644
--- a/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksServerProcess.java
+++ b/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksServerProcess.java
@@ -18,9 +18,12 @@
  */
 package org.apache.hyracks.test.server.process;
 
+import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -32,14 +35,21 @@
 abstract class HyracksServerProcess {
     private static final Logger LOGGER = LogManager.getLogger();
 
+    protected final String processName;
     protected Process process;
+    protected Thread pipeThread;
     protected File configFile = null;
     protected File logFile = null;
     protected File appHome = null;
     protected File workingDir = null;
     protected List<String> args = new ArrayList<>();
 
+    protected HyracksServerProcess(String processName) {
+        this.processName = processName;
+    }
+
     public void start() throws IOException {
+
         String[] cmd = buildCommand();
         if (LOGGER.isInfoEnabled()) {
             LOGGER.info("Starting command: " + Arrays.toString(cmd));
@@ -47,34 +57,45 @@
 
         ProcessBuilder pb = new ProcessBuilder(cmd);
         pb.redirectErrorStream(true);
+        pb.directory(workingDir);
         if (logFile != null) {
-            if (LOGGER.isInfoEnabled()) {
-                LOGGER.info("Logging to: " + logFile.getCanonicalPath());
-            }
+            LOGGER.info("Logging to: " + logFile.getCanonicalPath());
             logFile.getParentFile().mkdirs();
             try (FileWriter writer = new FileWriter(logFile, true)) {
                 writer.write("---------------------\n");
             }
             pb.redirectOutput(ProcessBuilder.Redirect.appendTo(logFile));
+            process = pb.start();
         } else {
-            if (LOGGER.isInfoEnabled()) {
-                LOGGER.info("Logfile not set, subprocess will output to stdout");
-            }
+            pb.redirectOutput(ProcessBuilder.Redirect.PIPE);
+            process = pb.start();
+            pipeThread = new Thread(() -> {
+                try (BufferedReader reader =
+                        new BufferedReader(new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8))) {
+                    String line;
+                    while ((line = reader.readLine()) != null) {
+                        System.out.println(processName + ": " + line);
+                    }
+                } catch (IOException e) {
+                    LOGGER.debug("exception reading process pipe", e);
+                }
+            });
+            pipeThread.start();
         }
-        pb.directory(workingDir);
-        process = pb.start();
     }
 
     public void stop() {
         process.destroy();
         try {
             boolean success = process.waitFor(30, TimeUnit.SECONDS);
-            if (LOGGER.isWarnEnabled()) {
-                LOGGER.warn("Killing unresponsive NC Process");
-            }
             if (!success) {
+                LOGGER.warn("Killing unresponsive NC Process");
                 process.destroyForcibly();
             }
+            if (pipeThread != null) {
+                pipeThread.interrupt();
+                pipeThread.join();
+            }
         } catch (InterruptedException e) {
             Thread.currentThread().interrupt();
         }
@@ -106,7 +127,7 @@
             cList.add(configFile.getAbsolutePath());
         }
         addCmdLineArgs(cList);
-        return cList.toArray(new String[cList.size()]);
+        return cList.toArray(new String[0]);
     }
 
     protected void addJvmArgs(List<String> cList) {
diff --git a/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksVirtualCluster.java b/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksVirtualCluster.java
index 6c77628..062d429 100644
--- a/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksVirtualCluster.java
+++ b/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/test/server/process/HyracksVirtualCluster.java
@@ -65,22 +65,6 @@
     }
 
     /**
-     * Creates and starts an NCService.
-     *
-     * @param configFile
-     *            - full path to an ncservice.conf. May be null to accept all defaults.
-     * @throws IOException
-     *             - if there are errors starting the process.
-     */
-    public HyracksNCServiceProcess addNCService(File configFile, File logFile, File log4jConfig) throws IOException {
-        HyracksNCServiceProcess proc =
-                new HyracksNCServiceProcess(configFile, logFile, appHome, workingDir, log4jConfig);
-        proc.start();
-        ncProcs.add(proc);
-        return proc;
-    }
-
-    /**
      * Starts the CC, initializing the cluster. Expects that any NCs referenced
      * in the cluster configuration have already been started with addNCService().
      *
@@ -97,22 +81,6 @@
     }
 
     /**
-     * Starts the CC, initializing the cluster. Expects that any NCs referenced
-     * in the cluster configuration have already been started with addNCService().
-     *
-     * @param ccConfigFile
-     *            - full path to a cluster conf file. May be null to accept all
-     *            defaults, although this is seldom useful since there are no NCs.
-     * @throws IOException
-     *             - if there are errors starting the process.
-     */
-    public HyracksCCProcess start(File ccConfigFile, File logFile, File log4jConfig) throws IOException {
-        ccProc = new HyracksCCProcess(ccConfigFile, logFile, appHome, workingDir, log4jConfig);
-        ccProc.start();
-        return ccProc;
-    }
-
-    /**
      * Stops all processes in the cluster.
      * QQQ Someday this should probably do a graceful stop of NCs rather than
      * killing the NCService.
diff --git a/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/cc.conf b/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/cc.conf
index 9c80c7d..9b1a1cd 100644
--- a/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/cc.conf
+++ b/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/cc.conf
@@ -30,3 +30,4 @@
 console.listen.port = 12345
 
 [common]
+log.dir=target/NCServiceIT
diff --git a/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/nc-blue.conf b/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/nc-blue.conf
index 6eb38dd..baccd46 100644
--- a/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/nc-blue.conf
+++ b/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/nc-blue.conf
@@ -18,3 +18,4 @@
 [ncservice]
 address=127.0.0.1
 port=9091
+logdir=-
\ No newline at end of file
diff --git a/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/nc-red.conf b/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/nc-red.conf
index 286bd32..7616b37 100644
--- a/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/nc-red.conf
+++ b/hyracks-fullstack/hyracks/hyracks-server/src/test/resources/NCServiceIT/nc-red.conf
@@ -18,3 +18,4 @@
 [ncservice]
 address=127.0.0.1
 port=9090
+logdir=-
\ No newline at end of file