RFR: 8325525: Create jtreg test case for JDK-8325203 [v2]

Alexander Matveev almatvee at openjdk.org
Wed Jul 3 20:08:21 UTC 2024


On Wed, 3 Jul 2024 04:11:40 GMT, Vanitha B P <duke at openjdk.org> wrote:

>> Created jtreg test case for [JDK-8325203](https://bugs.openjdk.org/browse/JDK-8325203) issue.
>> 
>> The JpackageTest created tests that the child process started from the app launched by jpackage launcher is not automatically terminated when the the launcher is terminated.
>
> Vanitha B P has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8325525 Addressed review comments

test/jdk/tools/jpackage/apps/ThirdPartyAppLauncher.java line 27:

> 25: import java.util.logging.Logger;
> 26: 
> 27: public class ThirdPartyAppLauncher {

Maybe `ChildProcessAppLauncher` to match test name?

test/jdk/tools/jpackage/apps/ThirdPartyAppLauncher.java line 32:

> 30: 
> 31:     public static void main(String[] args) throws IOException {
> 32:         ProcessBuilder processBuilder = new ProcessBuilder("regedit");

`regedit` will trigger elevation prompt if UAC is enabled, so this test will not be able to run without user interaction. My suggestion is to use something that does not requires elevation. For example: `calc.exe`. Also, use full path to executable, otherwise it is not clear what will be run. I think `System.getenv("SYSTEM")` should give path to `%WINDIR%\system32` where `calc` is located.

test/jdk/tools/jpackage/share/JpackageTest.java line 51:

> 49: import jdk.jpackage.test.TKit;
> 50: 
> 51: public class JpackageTest {

Can we put it under `test/jdk/tools/jpackage/windows`, since it is Windows only test? Also, it needs better name. For example: `WinChildProcessTest`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19536#discussion_r1664685484
PR Review Comment: https://git.openjdk.org/jdk/pull/19536#discussion_r1664702036
PR Review Comment: https://git.openjdk.org/jdk/pull/19536#discussion_r1664681991


More information about the core-libs-dev mailing list