On Sat, 26 Oct 2024 01:16:05 GMT, Alexey Semenyuk <asemen...@openjdk.org> wrote:
> Support `win.norestart` application property in jpackage .cfg file. If found > in the .cfg file and set to `true`, it forces the Windows app launcher not to > restart the launcher process. > > Collateral changes to the tests: > - Added `WindowsHelper.killProcess()` to kill process by PID; > - Added `WindowsHelper.findAppLauncherPID()` that will return PIDs of the > running app launcher processes for the given app launcher name. The > implementation grabbed from `Win8301247Test` test; > - Rework `CfgFile` to make it R/W and capable of writing multiple instances > of the same key; > - Added `JPackageCommand.ignoreFakeRuntime()` to ignore external runtime if > it is a stub. Should speed up local test runs. Looks good with minor comments. src/jdk.jpackage/share/native/applauncher/CfgFile.cpp line 247: > 245: iss >> intValue; > 246: > 247: return intValue != 0; Can you explain what this code does? test/jdk/tools/jpackage/windows/Win8301247Test.java line 1: > 1: /* Update copyright year. test/jdk/tools/jpackage/windows/Win8301247Test.java line 74: > 72: > 73: // Kill the main app launcher process > 74: killProcess(pid); I think this `killProcess` might not be called if `findAppLauncherPID` throws exception when we got only 1 process instead of expected 2. test/jdk/tools/jpackage/windows/WinNoRestartTest.java line 24: > 22: */ > 23: > 24: /* @test Formatting. Delete extra space. test/jdk/tools/jpackage/windows/WinNoRestartTest.java line 108: > 106: > 107: // Get PID of the main app launcher process > 108: final long pid = findAppLauncherPID(cmd, null, If I am reading code correctly `TKit.assertEquals` will throw exception, which means `killProcess` might not be called in case if we expecting 1 process, but we got 2. ------------- PR Review: https://git.openjdk.org/jdk/pull/21726#pullrequestreview-2406605956 PR Review Comment: https://git.openjdk.org/jdk/pull/21726#discussion_r1823574427 PR Review Comment: https://git.openjdk.org/jdk/pull/21726#discussion_r1823605929 PR Review Comment: https://git.openjdk.org/jdk/pull/21726#discussion_r1823615476 PR Review Comment: https://git.openjdk.org/jdk/pull/21726#discussion_r1823607003 PR Review Comment: https://git.openjdk.org/jdk/pull/21726#discussion_r1823614422
