Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
MIME-Version: 1.0
In-Reply-To: <20160108202133.568553A00E7@svn01-us-west.apache.org>
References: <20160108202133.568553A00E7@svn01-us-west.apache.org>
Date: Sat, 9 Jan 2016 16:56:45 +0000
Message-ID: 
 <CAOGo0VbkY5GZjONQXhw6pc9q01ESp+hnvhe_cybgCM1+Q5-TiA@mail.gmail.com>
Subject: Re: svn commit: r1723790 -
 /commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/issues/Exec65Test.java
From: sebb <sebbaz@gmail.com>
To: dev@commons.apache.org
Content-Type: text/plain; charset=UTF-8

On 8 January 2016 at 20:21,  <sgoeschl@apache.org> wrote:
> Author: sgoeschl
> Date: Fri Jan  8 20:21:33 2016
> New Revision: 1723790
>
> URL: http://svn.apache.org/viewvc?rev=1723790&view=rev
> Log:
> [EXEC-65] Fixing test execution for Windows platform
>
> Modified:
>     commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/issues/Exec65Test.java
>
> Modified: commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/issues/Exec65Test.java
> URL: http://svn.apache.org/viewvc/commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/issues/Exec65Test.java?rev=1723790&r1=1723789&r2=1723790&view=diff
> ==============================================================================
> --- commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/issues/Exec65Test.java (original)
> +++ commons/proper/exec/trunk/src/test/java/org/apache/commons/exec/issues/Exec65Test.java Fri Jan  8 20:21:33 2016
> @@ -24,8 +24,9 @@ import org.apache.commons.exec.ExecuteWa
>  import org.apache.commons.exec.OS;
>  import org.apache.commons.exec.PumpStreamHandler;
>  import org.apache.commons.exec.TestUtil;
> -import org.junit.Ignore;
> +import org.junit.Rule;
>  import org.junit.Test;
> +import org.junit.rules.TestName;
>
>  import java.io.File;
>
> @@ -38,67 +39,82 @@ import static org.junit.Assert.assertTru
>   */
>  public class Exec65Test {
>
> -    private static final int TIMEOUT = 3000;
> +    private static final int TEST_TIMEOUT = 15000;
> +    private static final int WATCHDOG_TIMEOUT = 3000;
> +    private static final String OS_NAME = System.getProperty("os.name");
> +
>      private final File testDir = new File("src/test/scripts");
>
> -    @Test(expected = ExecuteException.class, timeout = 15000)
> -    public void testExec65WitSleepUsingCommandLine() throws Exception
> -    {
> -        if(OS.isFamilyUnix())
> -        {
> +    @Rule public TestName name = new TestName();
> +
> +    @Test(expected = ExecuteException.class, timeout = TEST_TIMEOUT)
> +    public void testExec65WitSleepUsingSleepCommandDirectly() throws Exception {
> +
> +        if (OS.isFamilyUnix()) {
> +            final ExecuteWatchdog watchdog = new ExecuteWatchdog(WATCHDOG_TIMEOUT);
>              final DefaultExecutor executor = new DefaultExecutor();
> -            executor.setStreamHandler(new PumpStreamHandler(System.out, System.err));
> -            final ExecuteWatchdog watchdog = new ExecuteWatchdog(TIMEOUT);
> -            executor.setWatchdog(watchdog);
>              final CommandLine command = new CommandLine("sleep");
>              command.addArgument("60");
> +            executor.setStreamHandler(new PumpStreamHandler(System.out, System.err));
> +            executor.setWatchdog(watchdog);
>
>              executor.execute(command);
> +        } else {
> +            String msg = String.format("The test '%s' does not support the following OS : %s", name.getMethodName(), OS_NAME);
> +            System.out.println(msg);
> +            throw new ExecuteException(msg, 0);
>          }
>      }
>
> -    @Test(expected = ExecuteException.class, timeout = 15000)
> -    public void testExec65WithSleepUsingShellScript() throws Exception
> -    {
> +    @Test(expected = ExecuteException.class, timeout = TEST_TIMEOUT)
> +    public void testExec65WithSleepUsingShellScript() throws Exception {
> +
>          final DefaultExecutor executor = new DefaultExecutor();
>          executor.setStreamHandler(new PumpStreamHandler(System.out, System.err));
> -        final ExecuteWatchdog watchdog = new ExecuteWatchdog(TIMEOUT);
> -        executor.setWatchdog(watchdog);
> +        executor.setWatchdog(new ExecuteWatchdog(WATCHDOG_TIMEOUT));
>          final CommandLine command = new CommandLine(TestUtil.resolveScriptForOS(testDir + "/sleep"));
>
>          executor.execute(command);
>      }
>
> -    @Ignore("This test does not work under Linux but nicely on Mac OS X")
> -    @Test(timeout = 15000)
> -    public void testExec65WithSleepUsingShellScriptAndRuntimeDirectly() throws Exception
> -    {
> +    /**
> +     * This is the original code snippet from the JIRA to show that
> +     * killing the process actually works with JDK only but it does
> +     * not re-direct any streams.
> +     */
> +    @Test(timeout = TEST_TIMEOUT)
> +    public void testExec65WithSleepUsingShellScriptAndJDKOnly() throws Exception {
> +
>          Process process = Runtime.getRuntime().exec(TestUtil.resolveScriptForOS(testDir + "/sleep").getAbsolutePath());
> -        Thread.sleep(3000);
> +        Thread.sleep(WATCHDOG_TIMEOUT);
>
>          process.destroy();
> -
> -        process.waitFor();
> +        while (process.isAlive()) {
> +            Thread.sleep(100);
> +        }

This requires Java 1.8

Besides, it's unnecessary.

I've reverted to the previous fix I made.

>          assertTrue(process.exitValue() != 0);
>      }
>
>      /**
>       * Please note that this tests make assumptions about the environment. It assumes
> -     * that user "root" exists and that the current user is not a "sudoer" already.
> +     * that user "root" exists and that the current user is not a "sudoer" already
> +     * (thereby requiring a password).
>       */
> -    @Test(expected = ExecuteException.class, timeout = 15000)
> -    public void testExec65WithSudoUsingShellScript() throws Exception
> -    {
> -        if(OS.isFamilyUnix())
> -        {
> +    @Test(expected = ExecuteException.class, timeout = TEST_TIMEOUT)
> +    public void testExec65WithSudoUsingShellScript() throws Exception {
> +
> +        if (OS.isFamilyUnix()) {
>              final DefaultExecutor executor = new DefaultExecutor();
>              executor.setStreamHandler(new PumpStreamHandler(System.out, System.err, System.in));
> -            final ExecuteWatchdog watchdog = new ExecuteWatchdog(TIMEOUT);
> -            executor.setWatchdog(watchdog);
> +            executor.setWatchdog(new ExecuteWatchdog(WATCHDOG_TIMEOUT));
>              final CommandLine command = new CommandLine(TestUtil.resolveScriptForOS(testDir + "/issues/exec-65"));
>
>              executor.execute(command);
> +        } else {
> +            String msg = String.format("The test '%s' does not support the following OS : %s", name.getMethodName(), OS_NAME);
> +            System.out.println(msg);
> +            throw new ExecuteException(msg, 0);
>          }
>      }
>  }
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org