commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sgoes...@apache.org
Subject svn commit: r999068 - in /commons/proper/exec/trunk/src: main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java test/java/org/apache/commons/exec/DefaultExecutorTest.java
Date Mon, 20 Sep 2010 19:19:18 GMT
Author: sgoeschl
Date: Mon Sep 20 19:19:18 2010
New Revision: 999068

URL: http://svn.apache.org/viewvc?rev=999068&view=rev
Log:
Added a testExecuteWatchdogAsync() test

Modified:
    commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java
    commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java

Modified: commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java
URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java?rev=999068&r1=999067&r2=999068&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java
(original)
+++ commons/proper/exec/trunk/src/main/java/org/apache/commons/exec/DefaultExecuteResultHandler.java
Mon Sep 20 19:19:18 2010
@@ -24,31 +24,41 @@ package org.apache.commons.exec;
  */
 public class DefaultExecuteResultHandler implements ExecuteResultHandler {
 
-    private static final int SLEEP_TIME_MS = 100;
+    /** the interval polling the result */
+    private static final int SLEEP_TIME_MS = 50;
 
     /** Keep track if the process is still running */
-    private boolean hasResult;
+    private volatile boolean hasResult;
 
     /** The exit value of the finished process */
-    private int exitValue;
+    private volatile int exitValue;
 
     /** Any offending exception */
-    private ExecuteException exception;
+    private volatile ExecuteException exception;
+
+    /**
+     * Constructor.
+     */
+    public DefaultExecuteResultHandler() {
+        this.hasResult = false;
+        this.exitValue = Executor.INVALID_EXITVALUE;
+    }
 
     /**
      * @see org.apache.commons.exec.ExecuteResultHandler#onProcessComplete(int)
      */
-    synchronized public void onProcessComplete(int exitValue) {
+    public void onProcessComplete(int exitValue) {
         this.exitValue = exitValue;
+        this.exception = null;
         this.hasResult = true;
     }
 
     /**
      * @see org.apache.commons.exec.ExecuteResultHandler#onProcessFailed(org.apache.commons.exec.ExecuteException)
      */
-    synchronized public void onProcessFailed(ExecuteException e) {
+    public void onProcessFailed(ExecuteException e) {
+        this.exitValue = e.getExitValue();            
         this.exception = e;
-        exitValue = e.getExitValue();
         this.hasResult = true;
     }
 
@@ -58,8 +68,12 @@ public class DefaultExecuteResultHandler
      * @return Returns the exception.
      * @throws IllegalStateException if the process has not exited yet
      */
-    synchronized public ExecuteException getException() {
-        if(!hasResult) throw new IllegalStateException("The process has not exited yet therefore
no result is available ...");
+    public ExecuteException getException() {
+
+        if(!hasResult) {
+            throw new IllegalStateException("The process has not exited yet therefore no
result is available ...");
+        }
+
         return exception;
     }
 
@@ -69,17 +83,21 @@ public class DefaultExecuteResultHandler
      * @return Returns the exitValue.
      * @throws IllegalStateException if the process has not exited yet
      */
-    synchronized public int getExitValue() {
-        if(!hasResult) throw new IllegalStateException("The process has not exited yet therefore
no result is available ...");
+    public int getExitValue() {
+
+        if(!hasResult) {
+            throw new IllegalStateException("The process has not exited yet therefore no
result is available ...");
+        }
+
         return exitValue;
     }
 
     /**
-     * Has the process exited and a result is available?
+     * Has the process exited and a result is available, i.e. exitCode or exception?
      *
      * @return true if a result of the execution is available
      */
-    synchronized public boolean hasResult() {
+    public boolean hasResult() {
         return hasResult;
     }
 
@@ -90,24 +108,37 @@ public class DefaultExecuteResultHandler
      * not yet terminated, the calling thread will be blocked until the
      * process exits.
      *
-     * @return     the exit value of the process.
      * @exception  InterruptedException if the current thread is
      *             {@linkplain Thread#interrupt() interrupted} by another
      *             thread while it is waiting, then the wait is ended and
      *             an {@link InterruptedException} is thrown.
-     * @exception  ExecuteException re-thrown exception if the process 
-     *             execution has failed due to ExecuteException
      */
-    public int waitFor() throws InterruptedException, ExecuteException {
-        while (!this.hasResult()) {
+    public void waitFor() throws InterruptedException {
+
+        while (!hasResult()) {
             Thread.sleep(SLEEP_TIME_MS);
         }
+    }
 
-        if(getException() == null) {
-            return getExitValue();
-        }
-        else {
-            throw getException();
+    /**
+     * Causes the current thread to wait, if necessary, until the
+     * process has terminated. This method returns immediately if
+     * the process has already terminated. If the process has
+     * not yet terminated, the calling thread will be blocked until the
+     * process exits.
+     *
+     * @param timeout the maximum time to wait in milliseconds
+     * @exception  InterruptedException if the current thread is
+     *             {@linkplain Thread#interrupt() interrupted} by another
+     *             thread while it is waiting, then the wait is ended and
+     *             an {@link InterruptedException} is thrown.
+     */
+    public void waitFor(long timeout) throws InterruptedException {
+
+        long until = System.currentTimeMillis() + timeout;
+
+        while (!hasResult() && (System.currentTimeMillis() < until)) {
+            Thread.sleep(SLEEP_TIME_MS);
         }
     }
 }
\ No newline at end of file

Modified: commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java
URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java?rev=999068&r1=999067&r2=999068&view=diff
==============================================================================
--- commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java
(original)
+++ commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/DefaultExecutorTest.java
Mon Sep 20 19:19:18 2010
@@ -84,6 +84,10 @@ public class DefaultExecutorTest extends
         this.baos.close();
     }
 
+    // ======================================================================
+    // Start of regression tests
+    // ======================================================================
+
     /**
      * The simplest possible test - start a script and
      * check that the output was pumped into our
@@ -158,29 +162,38 @@ public class DefaultExecutorTest extends
         assertFalse(exec.isFailure(exitValue));
     }
 
+    /**
+     * Start a asynchronous process which returns an success
+     * exit value.
+     *
+     * @throws Exception the test failed
+     */
     public void testExecuteAsync() throws Exception {
         CommandLine cl = new CommandLine(testScript);
         DefaultExecuteResultHandler resultHandler = new DefaultExecuteResultHandler();  
     
         exec.execute(cl, resultHandler);
-        resultHandler.waitFor();
+        resultHandler.waitFor(2000);
+        assertTrue(resultHandler.hasResult());
+        assertNull(resultHandler.getException());
         assertFalse(exec.isFailure(resultHandler.getExitValue()));
         assertEquals("FOO..", baos.toString().trim());
     }
 
+    /**
+     * Start a asynchronous process which returns an error
+     * exit value.
+     *
+     * @throws Exception the test failed
+     */
     public void testExecuteAsyncWithError() throws Exception {
         CommandLine cl = new CommandLine(errorTestScript);
         DefaultExecuteResultHandler resultHandler = new DefaultExecuteResultHandler();
         exec.execute(cl, resultHandler);
-        try {
-            resultHandler.waitFor();
-        }
-        catch(ExecuteException e) {
-            assertTrue(exec.isFailure(e.getExitValue()));
-            assertNotNull(resultHandler.getException());
-            assertEquals("FOO..", baos.toString().trim());
-            return;
-        }
-        fail("Expecting an ExecuteException");
+        resultHandler.waitFor(2000);
+        assertTrue(resultHandler.hasResult());
+        assertTrue(exec.isFailure(resultHandler.getExitValue()));
+        assertNotNull(resultHandler.getException());
+        assertEquals("FOO..", baos.toString().trim());
     }
 
     /**
@@ -198,14 +211,18 @@ public class DefaultExecutorTest extends
         // wait for script to run
         Thread.sleep(2000);
         assertTrue("Watchdog should watch the process", watchdog.isWatching());
-        // terminate it using the watchdog
+        // terminate it manually using the watchdog
         watchdog.destroyProcess();
+        // wait until the result of the process execution is propagated
+        handler.waitFor();
         assertTrue("Watchdog should have killed the process", watchdog.killedProcess());
-        assertFalse(watchdog.isWatching());
+        assertFalse("Watchdog is no longer watching the process", watchdog.isWatching());
+        assertTrue("ResultHandler received a result", handler.hasResult());
+        assertNotNull("ResultHandler received an exception as result", handler.getException());
     }
 
     /**
-     * Start a async process and try to terminate it manually but
+     * Start a asynchronous process and try to terminate it manually but
      * the process was already terminated by the watchdog. This is
      * basically a race condition between infrastructure and user
      * code.
@@ -214,20 +231,24 @@ public class DefaultExecutorTest extends
      */
     public void testExecuteAsyncWithTooLateUserTermination() throws Exception {
         CommandLine cl = new CommandLine(foreverTestScript);
+        DefaultExecuteResultHandler handler = new DefaultExecuteResultHandler();
         ExecuteWatchdog watchdog = new ExecuteWatchdog(3000);
         exec.setWatchdog(watchdog);
-        DefaultExecuteResultHandler handler = new DefaultExecuteResultHandler();
         exec.execute(cl, handler);
         // wait for script to be terminated by the watchdog
         Thread.sleep(6000);
         // try to terminate the already terminated process
         watchdog.destroyProcess();
-        assertTrue("Watchdog should have killed the process already",watchdog.killedProcess());
-        assertFalse(watchdog.isWatching());
+        // wait until the result of the process execution is propagated
+        handler.waitFor();
+        assertTrue("Watchdog should have killed the process already", watchdog.killedProcess());
+        assertFalse("Watchdog is no longer watching the process", watchdog.isWatching());
+        assertTrue("ResultHandler received a result", handler.hasResult());
+        assertNotNull("ResultHandler received an exception as result", handler.getException());
     }
 
     /**
-     * Start a script looping forever and check if the ExecuteWatchdog
+     * Start a script looping forever (synchronously) and check if the ExecuteWatchdog
      * kicks in killing the run away process. To make killing a process
      * more testable the "forever" scripts write each second a '.'
      * into "./target/forever.txt" (a marker file). After a test run
@@ -235,7 +256,7 @@ public class DefaultExecutorTest extends
      *
      * @throws Exception the test failed
      */
-    public void testExecuteWatchdog() throws Exception {
+    public void testExecuteWatchdogSync() throws Exception {
 
         long timeout = 10000;
 
@@ -251,8 +272,8 @@ public class DefaultExecutorTest extends
         catch(ExecuteException e) {
             Thread.sleep(timeout);
             int nrOfInvocations = getOccurrences(readFile(this.foreverOutputFile), '.');
-            assertTrue("killing the subprocess did not work : " + nrOfInvocations, nrOfInvocations
> 5 && nrOfInvocations <= 11);
             assertTrue( executor.getWatchdog().killedProcess() );
+            assertTrue("killing the subprocess did not work : " + nrOfInvocations, nrOfInvocations
> 5 && nrOfInvocations <= 11);
             return;
         }
         catch(Throwable t) {
@@ -264,6 +285,35 @@ public class DefaultExecutorTest extends
     }
 
     /**
+     * Start a script looping forever (asynchronously) and check if the
+     * ExecuteWatchdog kicks in killing the run away process. To make killing
+     * a process more testable the "forever" scripts write each second a '.'
+     * into "./target/forever.txt" (a marker file). After a test run
+     * we should have a few dots in there.
+     *
+     * @throws Exception the test failed
+     */
+    public void testExecuteWatchdogAsync() throws Exception {
+
+        long timeout = 10000;
+
+        CommandLine cl = new CommandLine(foreverTestScript);
+        DefaultExecuteResultHandler handler = new DefaultExecuteResultHandler();
+        DefaultExecutor executor = new DefaultExecutor();
+        executor.setWorkingDirectory(new File("."));
+        executor.setWatchdog(new ExecuteWatchdog(timeout));
+
+        executor.execute(cl, handler);
+        handler.waitFor();
+
+        assertTrue("Killed process should be true", executor.getWatchdog().killedProcess()
);
+        int nrOfInvocations = getOccurrences(readFile(this.foreverOutputFile), '.');
+        assertTrue("Killing the process did not work : " + nrOfInvocations, nrOfInvocations
> 5 && nrOfInvocations <= 11);
+        assertTrue("ResultHandler received a result", handler.hasResult());
+        assertNotNull("ResultHandler received an exception as result", handler.getException());
+    }
+
+    /**
      * Try to start an non-existing application which should result
      * in an exception.
      *
@@ -279,7 +329,7 @@ public class DefaultExecutorTest extends
             // expected
             return;
         }
-        fail("Got no exception when executing an non-existing application");            
   
+        fail("Got no exception when executing an non-existing application");
     }
 
     /**
@@ -382,15 +432,8 @@ public class DefaultExecutorTest extends
       // terminate it and the process destroyer is detached
       watchdog.destroyProcess();
       assertTrue(watchdog.killedProcess());
-
-      try {
-          handler.waitFor();
-          fail("Expecting an ExecutionException");
-      }
-      catch(ExecuteException e) {
-          // nothing to do
-      }
-
+      handler.waitFor();
+      assertNotNull(handler.getException());
       assertEquals("Processor Destroyer size should be 0", 0, processDestroyer.size());
       assertFalse("Process destroyer should not exist as shutdown hook", processDestroyer.isAddedAsShutdownHook());
     }
@@ -853,7 +896,7 @@ public class DefaultExecutorTest extends
      *
      * @throws Exception the test failed
      */
-    public void testExecuteStability() throws Exception {
+    public void _testExecuteStability() throws Exception {
 
         // make a plain-vanilla test
         for(int i=0; i<100; i++) {
@@ -875,12 +918,8 @@ public class DefaultExecutorTest extends
             ExecuteWatchdog watchdog = new ExecuteWatchdog(500);
             exec.setWatchdog(watchdog);
             exec.execute(cl, env, resultHandler);
-            try {
-                resultHandler.waitFor();
-            }
-            catch(ExecuteException e) {
-                // nothing to do
-            }
+            resultHandler.waitFor();
+            assertNotNull(resultHandler.getException());
             baos.reset();
         }
     }



Mime
View raw message