RFR: 8353489: Increase timeout and improve Windows compatibility in test/jdk/java/lang/ProcessBuilder/Basic.java [v2]

[Eirik Bjørsnøs]

On Sat, 3 May 2025 11:46:03 GMT, Oleksii Sylichenko <duke at openjdk.org> wrote:

>> This PR proposes three improvements to the `Basic.java` test:
>> 
>> 1. Increase Timeout
>>    - The current timeout is insufficient when running the test in IntelliJ IDEA.
>>    - I propose increasing it by one minute.
>>    - The timeout value was last modified on May 13, 2010 (commit [56131863a71c](https://github.com/openjdk/jdk/commit/56131863a71ca552d0a881364bd2b3581e13f058)) and has remained unchanged since then.
>> 
>> 2. Fix Incompatibility with Windows (Cygwin vs. Native)
>>     - One of the tests executes the `echo` command.
>>     - This works in Cygwin but fails when running the test in a pure Windows environment (e.g., IntelliJ IDEA).
>>     - I propose replacing echo with `cmd /c echo/`, which produces the same output (a single newline) in Windows, ensuring compatibility.
>> 
>> 3. Prevent Autorun Scripts in the `cmd /c set` Command
>>     - The test runs `cmd /c set`, but in Windows, this may trigger autorun scripts.
>>     - I propose adding the `/d` options to prevent autorun scripts from affecting the test results.
>> 
>> These changes improve test reliability and ensure compatibility across different environments.
>> Testing:
>> - Verified that the test runs successfully in IntelliJ IDEA without timeout issues.
>> - Confirmed that `cmd /c echo/` produces the expected output in Windows.
>> - Ensured that `cmd /d /c set` correctly lists environment variables without executing autorun scripts.
>> 
>> # Detailed Description
>> 
>> ## echo
>> The following test fails in a clean Windows environment (e.g., when run from IntelliJ IDEA without Cygwin):
>> 
>> //----------------------------------------------------------------
>> // Test Runtime.exec(...envp...) with envstrings without any `='
>> //----------------------------------------------------------------
>> 
>> with following error:
>> 
>> Cannot run program "echo": CreateProcess error=2, The system cannot find the file specified
>> 
>> 
>> If we try to run `echo` as a process in Windows, for example from IntelliJ IDEA, we get the error above, because Windows does not have such an executable, but instead this is a command of the Command Processor (CMD).
>> 
>> The `echo.` command with the `.` works the same way as calling `echo` without parameters in Unix — it outputs an empty line. Was implemented using the analogous `echo/` command to avoid compatibility issues.
>> 
>> See: https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/echo
>> 
>> To ensure that the `echo/` command is supported in Cygwin, we n...
>
> Oleksii Sylichenko has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8353489: replace `echo.` with `echo/`

test/jdk/java/lang/ProcessBuilder/Basic.java line 1843:

> 1841:         //----------------------------------------------------------------
> 1842:         try {
> 1843:             String[] cmdp = Windows.is() ? new String[]{"cmd", "/c", "echo/"} : new String[]{"echo"};

A future maintainer without intimate familiarity with Windows commands and syntax would probably appreciate a short comment explaining the command chosen.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23933#discussion_r2072384373