On Thu, 1 May 2025 19:24:13 GMT, Roger Riggs <rri...@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
>> 
>> 
>> ## cmd.exe /c set
>>  The following test fails in a Windows environment under specific conditions:
>> 
>> //----------------------------------------------------------------
>> // Test Runtime.exec(...envp...)
>> // Check for sort order of environment variables on Windows.
>> //----------------------------------------------------------------
>> 
>> with following error:
>> 
>>>'+=+
>> BAZ=GORP
>> CONDA_BAT=C:\.......\\miniconda3\\condabin\\conda.bat
>> CONDA_EXE=C:\........\\miniconda3\\Scripts\\conda.exe
>> CONDA_SHLVL=0
>> FOO=BAR
>> PATH=C:\.........\\miniconda3\\condabin;
>> QUUX=
>> SystemRoot=C:\\WIN...
>
> 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"};
> 
> Why the extra "." in the echo string?

In Windows CMD, the `echo` command, when called without arguments, displays the 
current state of the command echoing mode — either enabled (`ON`) or disabled 
(`OFF`). For the case when this test is run from IntelliJ IDEA.

And the `echo.` command with the `.` works the same way as calling `echo` 
without parameters in Unix — it outputs an empty line.

And to ensure that the `echo.` command is supported in Cygwin, we need to 
invoke it via `cmd /c echo.`.

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

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

Reply via email to