Fixed a bug where SubprocessConnection irretrievably lost newline information from subprocesses (#67)

Author: JackSullivan

olcut-core/src/main/java/com/oracle/labs/mlrg/olcut/util/SubprocessConnection.java

@@ -319,7 +319,7 @@ public synchronized String run(String command) throws IOException, TimeoutExcept
 
             //
             // Read until an empty line is returned
-            collectOutputWithTimeout(results::append);
+            collectOutputWithTimeout(line -> results.append(line).append("\n"));
         } finally {
             processLock.unlock();
         }

olcut-core/src/test/java/com/oracle/labs/mlrg/olcut/util/SubprocessConnectionTest.java

@@ -34,6 +34,7 @@
 import org.junit.jupiter.api.condition.OS;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -50,12 +51,14 @@ public class SubprocessConnectionTest {
     List<String> work;
     List<String> expectedResponse;
 
+    SubprocessConnection multiSubProc;
+
     @BeforeEach
     public void setup() {
         // TODO I know this isn't the correct way to do this
         process = String.format("python3 %s/src/test/resources/com/oracle/labs/mlrg/olcut/util/testProcess.py", System.getProperty("user.dir"));
         work = Arrays.asList("part_a", "part_b", "part_c", "part_d");
-        expectedResponse = Arrays.asList("0:10:part_a", "1:15:part_b", "2:27:part_c", "3:38:part_d");
+        expectedResponse = Arrays.asList("0:10:part_a\n", "1:15:part_b\n", "2:27:part_c\n", "3:38:part_d\n");
         subproc = new SubprocessConnection(process);
         subproc.addSubprocessListener(new SubprocessConnectionListener() {
             private int started=0;
@@ -80,6 +83,8 @@ public void subprocessPostShutdown(SubprocessConnection connection) {
                 System.err.println("Subprocess post shutdown times: " + postShutdown);
             }
         });
+
+        multiSubProc = new SubprocessConnection(String.format("python3 %s/src/test/resources/com/oracle/labs/mlrg/olcut/util/testMultipleProcess.py", System.getProperty("user.dir")));
     }
 
     @Test
@@ -99,7 +104,7 @@ public void testReadTimeout() {
                     resp.set(subproc.run("part_b"));
                 }
         );
-        assertEquals("0:5:part_b", resp.get());
+        assertEquals("0:5:part_b\n", resp.get());
 
         assertThrows(TimeoutException.class,() -> {
             subproc.run("part_a");
@@ -126,6 +131,25 @@ public void testMultipleReadTimeouts() {
             resp.set(subproc.run("part_b"));
         });
 
-        assertEquals("0:5:part_b",resp.get() );
+        assertEquals("0:5:part_b\n",resp.get() );
+    }
+
+    @Test
+    public void testMultipleReturnedRows() {
+        List<String> responses = new ArrayList<>();
+        assertDoesNotThrow(() -> {
+            for (String piece : work) {
+                responses.addAll(Arrays.asList(multiSubProc.run(piece).split("\n")));
+            }
+        });
+        List<String> expected = new ArrayList<>();
+        int[] expectNum = {10, 15, 27, 38};
+        String[] expectPart = {"a", "b", "c", "d"};
+        for (int cnt = 0; cnt < 4; cnt++) {
+            for (int i = 0; i < expectNum[cnt]; i++) {
+                expected.add(String.format("%d:%d:part_%s", cnt, i, expectPart[cnt]));
+            }
+        }
+        assertEquals(expected, responses);
     }
 }

olcut-core/src/test/resources/com/oracle/labs/mlrg/olcut/util/testMultipleProcess.py

@@ -0,0 +1,57 @@
+#
+# Copyright (c) 2004-2022, Oracle and/or its affiliates.
+#
+# Licensed under the 2-clause BSD license.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are met:
+#
+# 1. Redistributions of source code must retain the above copyright notice,
+#    this list of conditions and the following disclaimer.
+#
+# 2. Redistributions in binary form must reproduce the above copyright notice,
+#    this list of conditions and the following disclaimer in the documentation
+#    and/or other materials provided with the distribution.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+# POSSIBILITY OF SUCH DAMAGE.
+
+import time
+import sys
+
+def eprint(s: str):
+    print(s, file=sys.stderr, flush=True)
+
+if __name__ == '__main__':
+
+    print("Ready", flush=True)
+    eprint("Sent Ready")
+    sec = 0
+    cnt = 0
+    # expected results
+    r = {'part_a':10, 'part_b':15, 'part_c':27, 'part_d':38}
+    while True:
+        try:
+            line = input()
+        except EOFError:
+            eprint("EOFError")
+            break
+        if len(line.strip()) == 0:
+            eprint("Received Empty Line")
+        else:
+            return_rows = r[line.strip()]
+            eprint("returning {} rows".format(return_rows))
+            for i in range(return_rows):
+                print("{}:{}:{}".format(cnt, i, line))
+            print("")
+            cnt += 1
+    eprint("Finished all work")