You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2014/07/25 19:57:03 UTC

[07/11] git commit: Consume process stderr in addition to stdout so it doesn't block.

Consume process stderr in addition to stdout so it doesn't block.

If there is any error output throw an exception.


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/a1cead9b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/a1cead9b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/a1cead9b

Branch: refs/heads/master
Commit: a1cead9b72615d9ee4a6146ab29ec7ba60d1cafd
Parents: 3f5cfc3
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Fri Jul 25 11:10:53 2014 +0300
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Fri Jul 25 11:11:47 2014 +0300

----------------------------------------------------------------------
 .../brooklyn/qa/longevity/MonitorUtils.java     | 31 +++++++-----
 .../brooklyn/qa/longevity/MonitorUtilsTest.java | 52 ++++++++++++++++----
 2 files changed, 62 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/a1cead9b/usage/qa/src/main/java/brooklyn/qa/longevity/MonitorUtils.java
----------------------------------------------------------------------
diff --git a/usage/qa/src/main/java/brooklyn/qa/longevity/MonitorUtils.java b/usage/qa/src/main/java/brooklyn/qa/longevity/MonitorUtils.java
index 89643ac..65f0d00 100644
--- a/usage/qa/src/main/java/brooklyn/qa/longevity/MonitorUtils.java
+++ b/usage/qa/src/main/java/brooklyn/qa/longevity/MonitorUtils.java
@@ -52,8 +52,6 @@ public class MonitorUtils {
 
     private static final Logger LOG = LoggerFactory.getLogger(MonitorUtils.class);
 
-    private static final int checkPeriodMs = 1000;
-
     private static volatile int ownPid = -1;
 
     /**
@@ -275,25 +273,34 @@ public class MonitorUtils {
 
     /**
      * Waits for the given process to complete, consuming its stdout and returning it as a string.
-     * <p/>
-     * Does not just use Groovy's:
-     * process.waitFor()
-     * return process.text
-     * <p/>
-     * Because that code hangs for bing output streams.
+     * If there is any output on stderr an exception will be thrown.
      */
     public static String waitFor(Process process) {
         ByteArrayOutputStream bytesOut = new ByteArrayOutputStream();
-        StreamGobbler gobbler = new StreamGobbler(process.getInputStream(), bytesOut, null);
-        gobbler.start();
+        @SuppressWarnings("resource") //Closeable doesn't seem appropriate for StreamGobbler since it isn't expected to be called every time
+        StreamGobbler gobblerOut = new StreamGobbler(process.getInputStream(), bytesOut, null);
+        gobblerOut.start();
+
+        ByteArrayOutputStream bytesErr = new ByteArrayOutputStream();
+        @SuppressWarnings("resource")
+        StreamGobbler gobblerErr = new StreamGobbler(process.getErrorStream(), bytesErr, null);
+        gobblerErr.start();
+
         try {
             process.waitFor();
-            gobbler.blockUntilFinished();
+            gobblerOut.blockUntilFinished();
+            gobblerErr.blockUntilFinished();
+
+            if (bytesErr.size() > 0) {
+                throw new IllegalStateException("Process printed to stderr: " + new String(bytesErr.toByteArray()));
+            }
+
             return new String(bytesOut.toByteArray());
         } catch (Exception e) {
             throw Throwables.propagate(e);
         } finally {
-            if (gobbler.isAlive()) gobbler.interrupt();
+            if (gobblerOut.isAlive()) gobblerOut.interrupt();
+            if (gobblerErr.isAlive()) gobblerErr.interrupt();
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/a1cead9b/usage/qa/src/test/java/brooklyn/qa/longevity/MonitorUtilsTest.java
----------------------------------------------------------------------
diff --git a/usage/qa/src/test/java/brooklyn/qa/longevity/MonitorUtilsTest.java b/usage/qa/src/test/java/brooklyn/qa/longevity/MonitorUtilsTest.java
index c62aaef..e2b9d1f 100644
--- a/usage/qa/src/test/java/brooklyn/qa/longevity/MonitorUtilsTest.java
+++ b/usage/qa/src/test/java/brooklyn/qa/longevity/MonitorUtilsTest.java
@@ -24,12 +24,15 @@ import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 
 import java.io.File;
+import java.io.IOException;
 import java.net.URL;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
 import org.testng.annotations.Test;
 
+import brooklyn.util.os.Os;
 import brooklyn.util.text.Strings;
 
 import com.google.common.base.Charsets;
@@ -38,20 +41,43 @@ import com.google.common.io.Files;
 
 public class MonitorUtilsTest {
 
-    @Test(enabled=false, groups="UNIX") // Demonstrates that process.waitFor() hangs for big output streams
+    @Test(enabled=false, timeOut=1000) // Demonstrates that process.waitFor() hangs for big output streams
     public void testExecuteAndWaitFor() throws Exception {
-        String bigstr = Strings.repeat("a", 100000);
-        Process process = MonitorUtils.exec("echo " + bigstr);
+        Process process = createDumpingProcess(false);
         process.waitFor();
-        fail();
+        fail("Should block while waiting to consume process output");
     }
 
-    @Test(groups="UNIX")
-    public void testExecuteAndWaitForConsumingOutputStream() {
-        String bigstr = Strings.repeat("a", 100000);
-        Process process = MonitorUtils.exec("echo " + bigstr);
+    @Test(enabled=false, timeOut=1000) // Demonstrates that process.waitFor() hangs for big err streams
+    public void testExecuteAndWaitForErr() throws Exception {
+        Process process = createDumpingProcess(true);
+        process.waitFor();
+        fail("Should block while waiting to consume process output");
+    }
+
+    @Test(timeOut=1000)
+    public void testExecuteAndWaitForConsumingOutputStream() throws Exception {
+        Process process = createDumpingProcess(false);
         String out = MonitorUtils.waitFor(process);
-        assertTrue(out.contains(bigstr), "out.size="+out.length());
+        assertTrue(out.length() > 100000, "out.size="+out.length());
+    }
+
+    @Test(timeOut=1000, expectedExceptions=IllegalStateException.class)
+    public void testExecuteAndWaitForConsumingErrorStream() throws Exception {
+        Process process = createDumpingProcess(true);
+        MonitorUtils.waitFor(process);
+    }
+
+    private Process createDumpingProcess(boolean writeToErr) throws IOException {
+        String errSuffix = writeToErr ? " >&2" : "";
+        //Windows limits the length of the arguments so echo multiple times instead
+        String bigstr = Strings.repeat("a", 8000);
+        String bigcmd = Strings.repeat(getSilentPrefix() + "echo " + bigstr + errSuffix + Os.LINE_SEPARATOR, 15);
+        File file = Os.newTempFile("test-consume", ".bat");
+        file.setExecutable(true);
+        Files.write(bigcmd, file, Charsets.UTF_8);
+        Process process = MonitorUtils.exec(file.getAbsolutePath());
+        return process;
     }
 
     @Test(groups="UNIX")
@@ -127,4 +153,12 @@ public class MonitorUtilsTest {
         assertEquals(memUsage3.getInstanceCounts(), Collections.emptyMap());
     }
 
+    private String getSilentPrefix() {
+        if (Os.isMicrosoftWindows()) {
+            return "@";
+        } else {
+            return "";
+        }
+    }
+
 }