RFR: 8349288: runtime/os/windows/TestAvailableProcessors.java fails on localized Windows platform [v2]

[David Holmes]

On Mon, 10 Feb 2025 08:48:59 GMT, Kazuhisa Takakuri <ktakakuri at openjdk.org> wrote:

>> To resolve runtime/os/windows/TestAvailableProcessors.java failure, I made "systeminfo.exe" executed with "chcp 437". This ensures that the English message "OS Version: " is output on localized windows platforms.
>> I added the path C:\Windows\System32 to make chcp command recognized on the GHA Windows test machine. Without this addition, GHA will fail.
>> After this fix, I verified that the test passed.
>> 
>> https://github.com/openjdk/jdk/pull/22142
>> I refer to this fix.
>> tools/jpackage/windows/Win8301247Test.java does not run in GHA, so the problems with recognition of chcp command did not occur before.
>> 
>> Thanks.
>
> Kazuhisa Takakuri has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8349288: runtime/os/windows/TestAvailableProcessors.java fails on localized Windows platform

Changes requested by dholmes (Reviewer).

test/hotspot/jtreg/runtime/os/windows/TestAvailableProcessors.java line 66:

> 64: 
> 65:         List<String> command = new ArrayList<>();
> 66:         //Execution command to prevent garbled characters

Suggestion:

        // Force language to English before running systeminfo to get the OS version

test/hotspot/jtreg/runtime/os/windows/TestAvailableProcessors.java line 68:

> 66:         //Execution command to prevent garbled characters
> 67:         command.addAll(List.of("cmd.exe", "/c", "set", "PATH=%PATH%;C:\\Windows\\System32;C:\\Windows\\System32\\wbem", "&&"));
> 68:         command.addAll(List.of("chcp", "437", ">nul", "2>&1", "&&"));

I see the use of `chcp` in the test `./tools/jpackage/helpers/jdk/jpackage/test/Executor.java` so that is okay. However it doesn't set the path and it does not seem necessary to do so, and potentially assumes what the paths are anyway. So I think you can simplify this a little.

test/hotspot/jtreg/runtime/os/windows/TestAvailableProcessors.java line 69:

> 67:         command.addAll(List.of("cmd.exe", "/c", "set", "PATH=%PATH%;C:\\Windows\\System32;C:\\Windows\\System32\\wbem", "&&"));
> 68:         command.addAll(List.of("chcp", "437", ">nul", "2>&1", "&&"));
> 69:         //Execute command to obtain OS Version

Suggestion:

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

PR Review: https://git.openjdk.org/jdk/pull/23536#pullrequestreview-2610660390
PR Review Comment: https://git.openjdk.org/jdk/pull/23536#discussion_r1951891541
PR Review Comment: https://git.openjdk.org/jdk/pull/23536#discussion_r1951893753
PR Review Comment: https://git.openjdk.org/jdk/pull/23536#discussion_r1951891668