Added tests for sonar-process Monitor Class

5 files changed, 142 insertions, 36 deletions

diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/Monitor.java b/server/process/sonar-process/src/main/java/org/sonar/process/Monitor.java
index fd5c60ac502..18b4f11d927 100644
--- a/server/process/sonar-process/src/main/java/org/sonar/process/Monitor.java
+++ b/server/process/sonar-process/src/main/java/org/sonar/process/Monitor.java
@@ -35,6 +35,7 @@ public class Monitor extends Thread implements Terminable {
 
   private static final long PING_DELAY_MS = 3000L;
 
+  private long pingDelayMs = PING_DELAY_MS;
   private volatile List<ProcessWrapper> processes;
   private final ScheduledFuture<?> watch;
   private final ScheduledExecutorService monitorExecutionService;
@@ -46,7 +47,16 @@ public class Monitor extends Thread implements Terminable {
     super("Process Monitor");
     processes = new ArrayList<ProcessWrapper>();
     monitorExecutionService = Executors.newScheduledThreadPool(1);
-    watch = monitorExecutionService.scheduleAtFixedRate(new ProcessWatch(), 0L, PING_DELAY_MS, TimeUnit.MILLISECONDS);
+    watch = monitorExecutionService.scheduleAtFixedRate(new ProcessWatch(), 0L, getPingDelayMs(), TimeUnit.MILLISECONDS);
+  }
+
+  private long getPingDelayMs() {
+    return pingDelayMs;
+  }
+
+  public Monitor setPingDelayMs(long pingDelayMs) {
+    this.pingDelayMs = pingDelayMs;
+    return this;
   }
 
   private class ProcessWatch extends Thread {
@@ -88,7 +98,7 @@ public class Monitor extends Thread implements Terminable {
   public void run() {
     try {
       boolean ok = true;
-      while (ok) {
+      while (isRunning && ok) {
         for (ProcessWrapper process : processes) {
           if (!ProcessUtils.isAlive(process.process())) {
             LOGGER.info("{} is down, stopping all other processes", process.getName());
@@ -107,8 +117,12 @@ public class Monitor extends Thread implements Terminable {
     }
   }
 
+  volatile Boolean isRunning = true;
+
   @Override
   public void terminate() {
+    LOGGER.debug("Monitoring thread is terminating");
+
     if (!monitorExecutionService.isShutdown()) {
       monitorExecutionService.shutdownNow();
     }
@@ -120,6 +134,17 @@ public class Monitor extends Thread implements Terminable {
       processes.get(i).terminate();
     }
     processes.clear();
-    interrupt();
+    interruptAndWait();
+  }
+
+  private void interruptAndWait() {
+    this.interrupt();
+    try {
+      if (this.isAlive()) {
+        this.join();
+      }
+    } catch (InterruptedException e) {
+      //Expected to be interrupted :)
+    }
   }
 }
diff --git a/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java b/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java
index c3c9949efc1..09f422ba09b 100644
--- a/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java
+++ b/server/process/sonar-process/src/main/java/org/sonar/process/ProcessWrapper.java
@@ -184,7 +184,7 @@ public class ProcessWrapper extends Thread implements Terminable {
       waitUntilFinish(outputGobbler);
       waitUntilFinish(errorGobbler);
       ProcessUtils.closeStreams(process);
-      this.interrupt();
+      interruptAndWait();
     }
   }
 
@@ -310,6 +310,7 @@ public class ProcessWrapper extends Thread implements Terminable {
       } finally {
         killer.shutdownNow();
       }
+      interruptAndWait();
     } else {
       // process is not monitored through JMX, but killing it though
       ProcessUtils.destroyQuietly(process);
@@ -340,6 +341,17 @@ public class ProcessWrapper extends Thread implements Terminable {
     return false;
   }
 
+  private void interruptAndWait() {
+    this.interrupt();
+    try {
+      if (this.isAlive()) {
+        this.join();
+      }
+    } catch (InterruptedException e) {
+      //Expected to be interrupted :)
+    }
+  }
+
   private static class StreamGobbler extends Thread {
     private final InputStream is;
     private final Logger logger;
diff --git a/server/process/sonar-process/src/test/java/org/sonar/process/MonitorTest.java b/server/process/sonar-process/src/test/java/org/sonar/process/MonitorTest.java
new file mode 100644
index 00000000000..95146a8d1f1
--- /dev/null
+++ b/server/process/sonar-process/src/test/java/org/sonar/process/MonitorTest.java
@@ -0,0 +1,59 @@
+package org.sonar.process;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.fest.assertions.Assertions.assertThat;
+
+public class MonitorTest extends ProcessTest {
+
+
+  Monitor monitor;
+
+  @Before
+  public void setUpMonitor() throws Exception {
+    monitor = new Monitor();
+  }
+
+  @After
+  public void downMonitor() throws Exception {
+    if (monitor != null) {
+      monitor.interrupt();
+      monitor = null;
+    }
+  }
+
+  @Test
+  public void monitor_can_start_and_stop() {
+    assertThat(monitor.isAlive()).isFalse();
+    monitor.start();
+    assertThat(monitor.isAlive()).isTrue();
+    monitor.terminate();
+    assertThat(monitor.isAlive()).isFalse();
+  }
+
+  @Test(timeout = 2500L)
+  public void monitor_should_interrupt_process() throws Exception {
+    // 0 start the dummyProcess
+    ProcessWrapper process = new ProcessWrapper("DummyOkProcess")
+      .addProperty(MonitoredProcess.NAME_PROPERTY, "DummyOkProcess")
+      .addClasspath(dummyAppJar.getAbsolutePath())
+      .setWorkDir(temp.getRoot())
+      .setTempDirectory(temp.getRoot())
+      .setJmxPort(freePort)
+      .setClassName(DUMMY_OK_APP);
+
+    assertThat(process.execute());
+
+
+    // 1 start my monitor & register process
+    monitor.start();
+    monitor.registerProcess(process);
+
+    // 2 terminate monitor, assert process is terminated
+    monitor.terminate();
+    assertThat(monitor.isAlive()).isFalse();
+    assertThat(process.isAlive()).isFalse();
+  }
+}
\ No newline at end of file
diff --git a/server/process/sonar-process/src/test/java/org/sonar/process/ProcessTest.java b/server/process/sonar-process/src/test/java/org/sonar/process/ProcessTest.java
new file mode 100644
index 00000000000..bcd066aae5d
--- /dev/null
+++ b/server/process/sonar-process/src/test/java/org/sonar/process/ProcessTest.java
@@ -0,0 +1,41 @@
+package org.sonar.process;
+
+import org.apache.commons.io.FileUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.ServerSocket;
+
+public abstract class ProcessTest {
+
+  @Rule
+  public TemporaryFolder temp = new TemporaryFolder();
+
+  public static final String DUMMY_OK_APP = "org.sonar.application.DummyOkProcess";
+
+  int freePort;
+  File dummyAppJar;
+  Process proc;
+
+  @Before
+  public void setup() throws IOException {
+    ServerSocket socket = new ServerSocket(0);
+    freePort = socket.getLocalPort();
+    socket.close();
+
+    dummyAppJar = FileUtils.toFile(getClass().getResource("/sonar-dummy-app.jar"));
+  }
+
+
+  @After
+  public void tearDown() {
+    if (proc != null) {
+      proc.destroy();
+    }
+  }
+
+}
diff --git a/server/process/sonar-process/src/test/java/org/sonar/process/ProcessWrapperTest.java b/server/process/sonar-process/src/test/java/org/sonar/process/ProcessWrapperTest.java
index f7564f0200f..2a7f10ce3b7 100644
--- a/server/process/sonar-process/src/test/java/org/sonar/process/ProcessWrapperTest.java
+++ b/server/process/sonar-process/src/test/java/org/sonar/process/ProcessWrapperTest.java
@@ -21,47 +21,16 @@ package org.sonar.process;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.FilenameUtils;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
 
 import java.io.File;
-import java.io.IOException;
-import java.net.ServerSocket;
 import java.util.Properties;
 
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
 
 
-public class ProcessWrapperTest {
-
-  @Rule
-  public TemporaryFolder temp = new TemporaryFolder();
-
-  private static final String DUMMY_OK_APP = "org.sonar.application.DummyOkProcess";
-
-  int freePort;
-  File dummyAppJar;
-  Process proc;
-
-  @Before
-  public void setup() throws IOException {
-    ServerSocket socket = new ServerSocket(0);
-    freePort = socket.getLocalPort();
-    socket.close();
-
-    dummyAppJar = FileUtils.toFile(getClass().getResource("/sonar-dummy-app.jar"));
-  }
-
-  @After
-  public void tearDown() {
-    if (proc != null) {
-      proc.destroy();
-    }
-  }
+public class ProcessWrapperTest extends ProcessTest {
 
   @Test
   public void has_dummy_app() {