Anonymous avatar Anonymous committed c50774e Merge

merging

Comments (0)

Files changed (10)

 f8e34816f4e1960217a3646aeccb642f033fbb00 atlassian-processutils-1.5.9
 
 e54e5674e9626102e04a3ac9df648e4c54674841 atlassian-processutils-1.5.9-bamboo1
+e0a507592573a59083726d43baa12c4b95555c8a atlassian-processutils-1.5.10
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
-    <modelVersion>4.0.0</modelVersion>
-
-    <parent>
-        <groupId>com.atlassian.pom</groupId>
-        <artifactId>atlassian-public-pom</artifactId>
-        <version>29.2</version>
-    </parent>
-
-    <groupId>com.atlassian.utils</groupId>
-    <artifactId>atlassian-processutils</artifactId>
-    <version>1.5.10-SNAPSHOT</version>
-    <name>Atlassian Process Utils</name>
-
-    <scm>
-        <connection>scm:hg:ssh://hg@bitbucket.org/atlassian/atlassian-processutils</connection>
-        <developerConnection>scm:hg:ssh://hg@bitbucket.org/atlassian/atlassian-processutils</developerConnection>
-        <url>https://bitbucket.org/atlassian/atlassian-processutils/overview</url>
-    </scm>
-
-    <dependencies>
-        <dependency>
-            <groupId>org.jvnet.winp</groupId>
-            <artifactId>winp</artifactId>
-            <version>1.17-atlassian1</version>
-        </dependency>
-        <dependency>
-            <groupId>log4j</groupId>
-            <artifactId>log4j</artifactId>
-            <version>1.2.15</version>
-            <exclusions>
-                <exclusion>
-                    <groupId>com.sun.jdmk</groupId>
-                    <artifactId>jmxtools</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>com.sun.jmx</groupId>
-                    <artifactId>jmxri</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>javax.jms</groupId>
-                    <artifactId>jms</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>javax.mail</groupId>
-                    <artifactId>mail</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>javax.activation</groupId>
-                    <artifactId>activation</artifactId>
-                </exclusion>
-            </exclusions>
-        </dependency>
-        <dependency>
-            <groupId>junit</groupId>
-            <artifactId>junit</artifactId>
-            <version>4.10</version>
-            <scope>test</scope>
-        </dependency>
-        <dependency>
-            <groupId>org.mockito</groupId>
-            <artifactId>mockito-all</artifactId>
-            <version>1.9.5</version>
-            <scope>test</scope>
-        </dependency>
-    </dependencies>
-</project>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>com.atlassian.pom</groupId>
+        <artifactId>atlassian-public-pom</artifactId>
+        <version>29.2</version>
+    </parent>
+
+    <groupId>com.atlassian.utils</groupId>
+    <artifactId>atlassian-processutils</artifactId>
+    <version>1.5.11-SNAPSHOT</version>
+    <name>Atlassian Process Utils</name>
+
+    <scm>
+        <connection>scm:hg:ssh://hg@bitbucket.org/atlassian/atlassian-processutils</connection>
+        <developerConnection>scm:hg:ssh://hg@bitbucket.org/atlassian/atlassian-processutils</developerConnection>
+        <url>https://bitbucket.org/atlassian/atlassian-processutils/overview</url>
+    </scm>
+
+    <dependencies>
+        <dependency>
+            <groupId>org.jvnet.winp</groupId>
+            <artifactId>winp</artifactId>
+            <version>1.17-atlassian1</version>
+        </dependency>
+        <dependency>
+            <groupId>log4j</groupId>
+            <artifactId>log4j</artifactId>
+            <version>1.2.15</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>com.sun.jdmk</groupId>
+                    <artifactId>jmxtools</artifactId>
+                </exclusion>
+                <exclusion>
+                    <groupId>com.sun.jmx</groupId>
+                    <artifactId>jmxri</artifactId>
+                </exclusion>
+                <exclusion>
+                    <groupId>javax.jms</groupId>
+                    <artifactId>jms</artifactId>
+                </exclusion>
+                <exclusion>
+                    <groupId>javax.mail</groupId>
+                    <artifactId>mail</artifactId>
+                </exclusion>
+                <exclusion>
+                    <groupId>javax.activation</groupId>
+                    <artifactId>activation</artifactId>
+                </exclusion>
+            </exclusions>
+        </dependency>
+
+        <dependency>
+            <groupId>commons-lang</groupId>
+            <artifactId>commons-lang</artifactId>
+            <version>2.6</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+            <version>4.10</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-all</artifactId>
+            <version>1.9.5</version>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
+</project>

src/main/java/com/atlassian/utils/process/ExternalProcessImpl.java

  * This class manages the execution of an external process, using separate threads to process
  * the process' IO requirements.
  */
+@SuppressWarnings("unused")
 public class ExternalProcessImpl implements ExternalProcess {
 
     private static final Logger LOG = Logger.getLogger(ExternalProcessImpl.class);
     }
 
     private String quoteString(String value) {
-        StringBuilder builder = new StringBuilder()
-                .append("\"")
-                .append(value.replace("\"", "\\\""))
-                .append("\"");
-        return builder.toString();
+        return "\"" + value.replace("\"", "\\\"") + "\"";
     }
 
     /*
             this.process = createProcess(command, environment, workingDir);
             setupIOPumps();
         } catch (IOException e) {
-            processException = new ProcessException(e);
+            processException = new ProcessNotStartedException(command.get(0) + " could not be started", e);
         }
     }
 
     }
 
     /**
-     * Finish process execution. This method should be called after you have called the
-     * start() method.
+     * Finish process execution. This method should be called after you have called the {@link #start()} method.
      */
     public void finish() {
-        if (process == null || finished.get()) {
+        if (finished.get()) {
             return;
         }
 
      * @return true if the process has finished.
      */
     public boolean finish(int maxWait) {
-        if (process == null || finished.get()) {
+        if (finished.get()) {
             return true;
         }
 
     }
 
     private void wrapUpProcess() {
-        if (process == null || finished.get()) {
+        if (finished.get()) {
             return;
         }
 
         int exitCode = -1;
         boolean processIncomplete = true;
         boolean interrupted = false;
-        try {
-            exitCode = process.exitValue();
-            processIncomplete = false;
-        } catch (IllegalThreadStateException itse) {
-            // process still running - could be a race to have the process finish so wait a little to be sure
-            while (processIncomplete && System.currentTimeMillis() - getTimeoutTime() < 10) {
-                // we are currently before the end of the period (within 10ms slack), so process probably not ready yet
-                try {
-                    Thread.sleep(100);
-                    exitCode = process.exitValue();
-                    processIncomplete = false;
-                } catch (InterruptedException ie) {
-                    processIncomplete = true;
-                    interrupted = true;
-                    break;
-                } catch (IllegalThreadStateException e) {
-                    // ignore and try in the next loop
+        if (process != null) {
+            try {
+                exitCode = process.exitValue();
+                processIncomplete = false;
+            } catch (IllegalThreadStateException itse) {
+                // process still running - could be a race to have the process finish so wait a little to be sure
+                while (processIncomplete && System.currentTimeMillis() - getTimeoutTime() < 10) {
+                    // we are currently before the end of the period (within 10ms slack), so process probably not ready yet
+                    try {
+                        Thread.sleep(100);
+                        exitCode = process.exitValue();
+                        processIncomplete = false;
+                    } catch (InterruptedException ie) {
+                        processIncomplete = true;
+                        interrupted = true;
+                        break;
+                    } catch (IllegalThreadStateException e) {
+                        // ignore and try in the next loop
+                    }
                 }
             }
         }
         if (finished.compareAndSet(false, true)) {
             internalCancel(exitCode);
 
-            if (processIncomplete && !interrupted) {
+            if (processException == null && processIncomplete && !interrupted) {
                 processException = new ProcessTimeoutException("process timed out");
             }
 

src/main/java/com/atlassian/utils/process/ProcessNotStartedException.java

+package com.atlassian.utils.process;
+
+/**
+ * {@link ProcessException} specialization thrown when an {@link ExternalProcess} cannot be
+ * {@link ExternalProcess#start() started}.
+ *
+ * @since 1.5.10
+ */
+public class ProcessNotStartedException extends ProcessException {
+
+    public ProcessNotStartedException(String message, Throwable cause) {
+        super(message, cause);
+    }
+}

src/test/java/com/atlassian/utils/process/ExternalProcessImplTest.java

 package com.atlassian.utils.process;
 
+import org.apache.commons.lang.SystemUtils;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
 
 import java.util.Arrays;
 import java.util.concurrent.TimeUnit;
 
-import static org.hamcrest.CoreMatchers.instanceOf;
-import static org.hamcrest.CoreMatchers.is;
+import static com.atlassian.utils.process.ExternalProcessTestUtils.*;
 import static org.junit.Assert.*;
-import static com.atlassian.utils.process.ExternalProcessTestUtils.*;
+import static org.junit.Assume.assumeTrue;
+import static org.mockito.Mockito.*;
 
 @SuppressWarnings("ThrowableResultOfMethodCallIgnored")
 public class ExternalProcessImplTest {
         process.execute();
 
         ProcessException exception = process.getHandler().getException();
-        assertThat(exception, is(instanceOf(ProcessTimeoutException.class)));
+        assertTrue(exception instanceof ProcessTimeoutException);
     }
 
-    @Test(timeout = 10000 * 1000) //Test timeout in case execution doesn't stop timeout like it should
+    @Test(timeout = 10 * 1000) //Test timeout in case execution doesn't stop timeout like it should
     public void testIdleTimeout() {
         //Ensure the timeout here is less than the timeout on the test
-        ExternalProcess process = createProcessBuilderForIdleTimeoutTests(500L, TimeUnit.MILLISECONDS)
+        ExternalProcess process = createProcessBuilderForIdleTimeoutTests()
                 .handlers(new StringOutputHandler())
                 .build();
         process.execute();
 
         ProcessException exception = process.getHandler().getException();
-        assertThat(exception, is(instanceOf(ProcessTimeoutException.class)));
-    }
-
-    private ExternalProcess buildProcessWithBlockingInputHandler() {
-        return createProcessBuilderThatExecutesFor(1)
-                .handlers(createBlockingInputHandler(10, TimeUnit.SECONDS), new StringOutputHandler())
-                .build();
+        assertTrue(exception instanceof ProcessTimeoutException);
     }
 
     @Test(timeout = 10 * 1000) // Test timeout in case execution doesn't stop as it should
 
     @Test(timeout = 10 * 1000)
     public void testNativeWindowsKilling() {
-        if (isWindows()) {
-            ExternalProcess process = new ExternalProcessBuilder()
-                    .command(Arrays.asList("pause"))
-                    .handlers(new StringOutputHandler())
-                    .build();
-            process.start();
-            process.cancel();
-        }
+        assumeTrue(SystemUtils.IS_OS_WINDOWS);
+
+        ExternalProcess process = new ExternalProcessBuilder()
+                .command(Arrays.asList("pause"))
+                .handlers(new StringOutputHandler())
+                .build();
+        process.start();
+        process.cancel();
     }
 
     /**
 
         process.execute();
 
-        // second process executio should trigger an IllegalStateException
+        //Second execution should trigger an IllegalStateException
         process.execute();
     }
 
-    private boolean isWindows() {
-        return System.getProperty("os.name").toLowerCase().contains("windows");
+    @Test
+    public void testProcessCreationFailureHandling() {
+        String command = SystemUtils.IS_OS_WINDOWS ?
+                "C:\\some\\nonexistent\\path\\bogus.exe" :
+                "/some/nonexistent/path/bogus";
+
+        ProcessHandler handler = mock(ProcessHandler.class);
+        ExternalProcess process = new ExternalProcessBuilder()
+                .command(Arrays.asList(command))
+                .handler(handler)
+                .suppressSpecialWindowsBehaviour()
+                .build();
+        process.start();
+        process.finish();
+
+        ArgumentCaptor<ProcessException> exceptionCaptor = ArgumentCaptor.forClass(ProcessException.class);
+
+        //Verify that ProcessHandler.complete was invoked, to ensure the exception caught in start() isn't swallowed
+        verify(handler).complete(eq(-1), eq(false), exceptionCaptor.capture());
+        verifyNoMoreInteractions(handler);
+
+        //Verify that an exception was provided
+        ProcessException exception = exceptionCaptor.getValue();
+        assertTrue(exception instanceof ProcessNotStartedException);
+        assertEquals(command + " could not be started", exception.getMessage());
+    }
+
+    private static ExternalProcess buildProcessWithBlockingInputHandler() {
+        return createProcessBuilderThatExecutesFor(1)
+                .handlers(createBlockingInputHandler(10, TimeUnit.SECONDS), new StringOutputHandler())
+                .build();
     }
 }

src/test/java/com/atlassian/utils/process/ExternalProcessMonitorTest.java

 
 import java.io.InputStream;
 import java.util.Arrays;
-import java.util.concurrent.TimeUnit;
 
 import static com.atlassian.utils.process.ExternalProcessTestUtils.*;
-import static org.hamcrest.CoreMatchers.instanceOf;
-import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.same;
 import static org.mockito.Mockito.never;
 
         process.execute();
 
-        assertThat(process.getHandler().getException(), is(instanceOf(ProcessTimeoutException.class)));
+        assertTrue(process.getHandler().getException() instanceof ProcessTimeoutException);
         verify(monitor).onBeforeStart(same(process));
         verify(monitor).onAfterFinished(same(process));
     }
 
     @Test
     public void testMonitorCalledOnIdleTimeout() {
-        process = createProcessBuilderForIdleTimeoutTests(500, TimeUnit.MILLISECONDS)
+        process = createProcessBuilderForIdleTimeoutTests()
                 .addMonitor(monitor)
                 .handlers(new StringOutputHandler())
                 .build();
 
         process.execute();
 
-        assertThat(process.getHandler().getException(), is(instanceOf(ProcessTimeoutException.class)));
+        assertTrue(process.getHandler().getException() instanceof ProcessTimeoutException);
         verify(monitor).onBeforeStart(same(process));
         verify(monitor).onAfterFinished(same(process));
     }

src/test/java/com/atlassian/utils/process/ExternalProcessTestUtils.java

 package com.atlassian.utils.process;
 
+import org.apache.commons.lang.SystemUtils;
+
 import java.io.OutputStream;
 import java.util.Arrays;
 import java.util.concurrent.TimeUnit;
     public static ExternalProcessBuilder createPingBuilder(int count, long timeoutSeconds, String target) {
         ExternalProcessBuilder builder = new ExternalProcessBuilder();
         String countString = Integer.toString(count);
-        if (isWindows()) {
+        if (SystemUtils.IS_OS_WINDOWS) {
+            //ping on Windows uses -n for the count and -w for the timeout in _milliseconds_
             builder.command(Arrays.asList("ping", "-n", countString, "-w", Long.toString(timeoutSeconds * 1000), target));
+        } else if (SystemUtils.IS_OS_MAC_OSX) {
+            //ping on MacOS uses -c and -W like Linux/Unix, but -W accepts a timeout in _milliseconds_
+            builder.command(Arrays.asList("ping", "-c", countString, "-W", Long.toString(timeoutSeconds * 1000), target));
         } else {
+            //ping on Linux (at least Ubuntu) uses -W for the timeout in _seconds_
             builder.command(Arrays.asList("ping", "-c", countString, "-W", Long.toString(timeoutSeconds), target));
         }
         return builder;
         return createPingBuilder(1, durationSeconds, "123.45.67.89");
     }
 
-    public static ExternalProcessBuilder createProcessBuilderForIdleTimeoutTests(long duration, TimeUnit timeUnit) {
-        // ensure the process idles a bit longer than the idle timeout
-        long durationMillis = timeUnit.toMillis(duration);
-        long durationSeconds = (long) Math.ceil(durationMillis / 1000.0d);
-        return createProcessBuilderThatIdlesFor(durationSeconds)
-                .idleTimeout(durationMillis)
-                .executionTimeout(durationMillis + 250L);
+    public static ExternalProcessBuilder createProcessBuilderForIdleTimeoutTests() {
+        return createProcessBuilderThatIdlesFor(2)
+                .idleTimeout(500L)
+                .executionTimeout(2000L);
     }
-
-    private static boolean isWindows() {
-        return System.getProperty("os.name").toLowerCase().contains("windows");
-    }
-
 }
Add a comment to this file

src/test/java/com/atlassian/utils/process/ProcessBuilderEncodingTest.java

File contents unchanged.

src/test/java/com/atlassian/utils/process/RecursiveApp.java

         return level + LEVEL_END_SUFFIX;
     }
 
-
-
     public static void main(String[] args) throws IOException, InterruptedException {
         int currentLevel = Integer.parseInt(args[0]);
         int maxLevel = Integer.parseInt(args[1]);
             while (System.currentTimeMillis() < end) {
                 try {
                     Thread.sleep(timeOut);
-                } catch (InterruptedException e) {
+                } catch (InterruptedException ignored) {
                 }
             }
         }
         System.out.println(getEndMessage(currentLevel));
-
     }
-
 }

src/test/java/com/atlassian/utils/process/RecursiveKillTest.java

 package com.atlassian.utils.process;
 
-import junit.framework.Assert;
-import org.junit.Assume;
+import org.apache.commons.lang.SystemUtils;
 import org.junit.Test;
 import org.jvnet.winp.WinProcess;
 
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
+import static junit.framework.Assert.*;
+import static org.junit.Assume.assumeTrue;
+
 public class RecursiveKillTest {
 
     @Test(timeout = 10 * 1000)
     public void testSimpleRecursiveKill() throws InterruptedException {
-
         //we can only live on windows...
-        Assume.assumeTrue(System.getProperty("os.name").toLowerCase().indexOf("win") != -1);
+        assumeTrue(SystemUtils.IS_OS_WINDOWS);
 
         ExternalProcessBuilder processBuilder = new ExternalProcessBuilder()
                 .command(Arrays.asList("java", "com.atlassian.utils.process.RecursiveApp", Integer.toString(0), Integer.toString(5), Integer.toString(8)))
                 .executionTimeout(10 * 10000L);
 
 
-        StringProcessHandler stringHandler = new StringProcessHandler();
+        StringProcessHandler processHandler = new StringProcessHandler();
         final List<Integer> pids = new ArrayList<Integer>();
         final List<Integer> levels = new ArrayList<Integer>();
         final List<Integer> ended = new ArrayList<Integer>();
         final CountDownLatch latchForStart = new CountDownLatch(6);
-        stringHandler.setOutputHandler(new BaseOutputHandler() {
+        processHandler.setOutputHandler(new BaseOutputHandler() {
 
             private final StringWriter writer = new StringWriter();
 
                 IOUtils.closeQuietly(writer);
             }
 
-            public String getOutput() {
-                return writer.toString();
-            }
-
             public void process(InputStream output) throws ProcessException {
                 InputStreamReader reader = null;
                 try {
                 }
             }
         });
-        processBuilder.handler(stringHandler);
+        processBuilder.handler(processHandler);
         ExternalProcess proc = processBuilder.build();
 
         proc.start();
         long start = System.currentTimeMillis();
         latchForStart.await(8, TimeUnit.SECONDS);
-        Assert.assertTrue("Process startup time exceeded ", ((System.currentTimeMillis() - start) < 8000));
-        Assert.assertEquals("Initial depth ", 6, levels.size());
-
+        assertTrue("Process startup time exceeded", ((System.currentTimeMillis() - start) < 8000));
+        assertEquals("6 processes should have been started", 6, levels.size());
 
         proc.cancel();
-
         try {
             Thread.sleep(500);
-
         } catch (InterruptedException e) {
         }
 
         int count = getRecursiveAppCount(pids);
-        Assert.assertEquals("Final depth ", 0, count);
+        assertEquals("All processes should have been killed", 0, count);
 
-        Assert.assertEquals("Assert that no processes finished naturally ", 0, ended.size());
-        String output = stringHandler.getOutput();
-        Assert.assertFalse("Assert that no processes finished naturally ", output.contains(RecursiveApp.getEndMessage(0)));
-        Assert.assertFalse("Assert that no processes finished naturally ", output.contains(RecursiveApp.getEndMessage(1)));
-        Assert.assertFalse("Assert that no processes finished naturally ", output.contains(RecursiveApp.getEndMessage(2)));
-        Assert.assertFalse("Assert that no processes finished naturally ", output.contains(RecursiveApp.getEndMessage(3)));
-        Assert.assertFalse("Assert that no processes finished naturally ", output.contains(RecursiveApp.getEndMessage(4)));
-        Assert.assertFalse("Assert that no processes finished naturally ", output.contains(RecursiveApp.getEndMessage(5)));
-
+        assertEquals("No processes should have completed normally", 0, ended.size());
+        String output = processHandler.getOutput();
+        assertFalse("Root process should not have completed", output.contains(RecursiveApp.getEndMessage(0)));
+        assertFalse("Level 1 process should not have completed", output.contains(RecursiveApp.getEndMessage(1)));
+        assertFalse("Level 2 process should not have completed", output.contains(RecursiveApp.getEndMessage(2)));
+        assertFalse("Level 3 process should not have completed", output.contains(RecursiveApp.getEndMessage(3)));
+        assertFalse("Level 4 process should not have completed", output.contains(RecursiveApp.getEndMessage(4)));
+        assertFalse("Level 5 process should not have completed", output.contains(RecursiveApp.getEndMessage(5)));
     }
 
-    private int getRecursiveAppCount(List<Integer> pids) {
+    private static int getRecursiveAppCount(List<Integer> pids) {
         int count = 0;
         for (WinProcess process : WinProcess.all()) {
-
             if (pids.contains(process.getPid())) {
                 count++;
             }
-
         }
         return count;
     }
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.