Feature/process utils jdk 8 (#344)

* test: Add tests around ProcessUtils.waitForWithTimeout

* feat: Upgrade ProcessUtils to use jdk 8 use waitFor(long timeout, TimeUnit unit)

src/main/java/net/bramp/ffmpeg/FFcommon.java

@@ -72,7 +72,6 @@ protected BufferedReader wrapErrorInReader(Process p) {
 
   protected void throwOnError(Process p) throws IOException {
     try {
-      // TODO In java 8 use waitFor(long timeout, TimeUnit unit)
       if (ProcessUtils.waitForWithTimeout(p, 1, TimeUnit.SECONDS) != 0) {
         // TODO Parse the error
         throw new IOException(path + " returned non-zero exit status. Check stdout.");
@@ -84,7 +83,6 @@ protected void throwOnError(Process p) throws IOException {
 
   protected void throwOnError(Process p, FFmpegProbeResult result) throws IOException {
     try {
-      // TODO In java 8 use waitFor(long timeout, TimeUnit unit)
       if (ProcessUtils.waitForWithTimeout(p, 1, TimeUnit.SECONDS) != 0) {
         // TODO Parse the error
         final FFmpegError ffmpegError = null == result ? null : result.getError();

src/main/java/net/bramp/ffmpeg/io/ProcessUtils.java

@@ -9,39 +9,10 @@
  * @author bramp
  */
 public final class ProcessUtils {
-
   private ProcessUtils() {
     throw new AssertionError("No instances for you!");
   }
 
-  private static class ProcessThread extends Thread {
-    final Process p;
-    boolean finished = false;
-    int exitValue = -1;
-
-    private ProcessThread(Process p) {
-      this.p = p;
-    }
-
-    @Override
-    public void run() {
-      try {
-        exitValue = p.waitFor();
-        finished = true;
-      } catch (InterruptedException e) {
-        Thread.currentThread().interrupt();
-      }
-    }
-
-    public boolean hasFinished() {
-      return finished;
-    }
-
-    public int exitValue() {
-      return exitValue;
-    }
-  }
-
   /**
    * Waits until a process finishes or a timeout occurs
    *
@@ -54,20 +25,17 @@ public int exitValue() {
   public static int waitForWithTimeout(final Process p, long timeout, TimeUnit unit)
       throws TimeoutException {
 
-    ProcessThread t = new ProcessThread(p);
-    t.start();
     try {
-      unit.timedJoin(t, timeout);
+      p.waitFor(timeout, unit);
 
     } catch (InterruptedException e) {
-      t.interrupt();
       Thread.currentThread().interrupt();
     }
 
-    if (!t.hasFinished()) {
+    if (p.isAlive()) {
       throw new TimeoutException("Process did not finish within timeout");
     }
 
-    return t.exitValue();
+    return p.exitValue();
   }
 }

src/test/java/net/bramp/ffmpeg/FFprobeTest.java

@@ -389,15 +389,46 @@ public void testProbeDivideByZero() throws IOException {
   }
 
   @Test
-  public void shouldThrowOnErrorWithFFmpegProbeResult() throws InterruptedException {
-    Mockito.when(mockProcess.waitFor()).thenReturn(-1);
+  public void shouldThrowOnErrorWithFFmpegProbeResult() {
+    Mockito.doReturn(1).when(mockProcess).exitValue();
+
     final FFmpegError error = new FFmpegError();
     final FFmpegProbeResult result = new FFmpegProbeResult();
     result.error = error;
     FFmpegException e = assertThrows(FFmpegException.class, () -> ffprobe.throwOnError(mockProcess, result));
     assertEquals(error, e.getError());
   }
 
+  @Test
+  public void shouldThrowOnErrorEvenIfProbeResultHasNoError() {
+    Mockito.doReturn(1).when(mockProcess).exitValue();
+
+    final FFmpegProbeResult result = new FFmpegProbeResult();
+    FFmpegException e = assertThrows(FFmpegException.class, () -> ffprobe.throwOnError(mockProcess, result));
+    assertNull(e.getError());
+  }
+
+  @Test
+  public void shouldThrowOnErrorEvenIfProbeResultIsNull() {
+    Mockito.doReturn(1).when(mockProcess).exitValue();
+
+    FFmpegException e = assertThrows(FFmpegException.class, () -> ffprobe.throwOnError(mockProcess, null));
+    assertNull(e.getError());
+  }
+
+  @Test
+  public void testShouldThrowErrorWithoutMock() throws IOException {
+    FFprobe probe = new FFprobe();
+    FFmpegException e = assertThrows(FFmpegException.class, () -> probe.probe("doesnotexist.mp4"));
+
+    assertNotNull(e);
+    assertNotNull(e.getError());
+
+    // Intentionally not comparing the values, as those might change for different ffmpeg versions
+    assertNotNull(e.getError().getString());
+    assertNotEquals(0, e.getError().getCode());
+  }
+
   @Test
   public void testProbeSideDataList() throws IOException {
     FFmpegProbeResult info = ffprobe.probe(Samples.side_data_list);

src/test/java/net/bramp/ffmpeg/io/ProcessUtilsTest.java

@@ -0,0 +1,37 @@
+package net.bramp.ffmpeg.io;
+
+import net.bramp.ffmpeg.FFmpeg;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+
+public class ProcessUtilsTest {
+    @Test
+    public void testProcessFinishesBeforeTimeout() throws Exception {
+        String ffmpeg = new FFmpeg().getPath();
+        ProcessBuilder processBuilder = new ProcessBuilder(ffmpeg, "-y", "-v", "quiet", "-f", "lavfi", "-i", "testsrc=duration=1:size=1280x720:rate=10", "-c:v", "libx264", "-t", "1", "output.mp4");
+        Process process = processBuilder.start();
+
+        int exitValue = ProcessUtils.waitForWithTimeout(process, 5, TimeUnit.SECONDS);
+
+        assertEquals(0, exitValue);
+    }
+
+    @Test
+    public void testProcessDoesNotFinishBeforeTimeout() throws IOException {
+        String ffmpeg = new FFmpeg().getPath();
+        ProcessBuilder processBuilder = new ProcessBuilder(ffmpeg, "-y", "-v", "quiet", "-f", "lavfi", "-i", "testsrc=duration=10:size=1280x720:rate=30", "-c:v", "libx264", "-t", "10", "output.mp4");
+        Process process = processBuilder.start();
+
+        assertThrows(TimeoutException.class, () -> {
+            ProcessUtils.waitForWithTimeout(process, 1, TimeUnit.MILLISECONDS);
+        });
+
+        process.destroy();
+    }
+}