On Wed, 24 Jul 2024 12:20:05 GMT, Vanitha B P <d...@openjdk.org> wrote:
> tools/jpackage/windows/WinChildProcessTest.java was failing intermittently,
> fixed the issue and changes are tested.
Changes requested by asemenyuk (Reviewer).
@sashamatveev please review
test/jdk/tools/jpackage/apps/ChildProcessAppLauncher.java line 33:
> 31: var lock = new Object();
> 32: synchronized (lock) {
> 33: System.out.println("Process id=" +
> ProcessHandle.current().pid() + " is about to block");
This log message goes nowhere and doesn't make sense. The app launcher is not
built with `--win-console` option, so it doesn't have a console, and stdout is
not attached to anything that can be captured in the test output. When
sketching a workaround in JBS comments section I didn't realize this log
message would not be captured in the test output. I'd remove it for simplicity.
test/jdk/tools/jpackage/windows/WinChildProcessTest.java line 81:
> 79: boolean isAlive = processHandle.isPresent()
> 80: && processHandle.get().isAlive();
> 81: System.out.println("Is Alive " + isAlive);
This log statement is redundant. The following `TKit.assertTrue()` call
produces enough information in the log:
[03:45:39.326] TRACE: assertEquals(0): Check command
[test\output\WinChildProcessTest\WinChildProcessTest.exe](1) exited with 0 code
Is Alive true
[03:45:39.326] TRACE: assertTrue(): Check is child process is alive
It is irrelevant to this PR and is an oversight from the previous review.
However, I believe it is OK to make this small change in this PR.
-------------
PR Review: https://git.openjdk.org/jdk/pull/20312#pullrequestreview-2197244491
PR Comment: https://git.openjdk.org/jdk/pull/20312#issuecomment-2248468758
PR Review Comment: https://git.openjdk.org/jdk/pull/20312#discussion_r1690129976
PR Review Comment: https://git.openjdk.org/jdk/pull/20312#discussion_r1690115428
