From dce50596ff38c406543a1cd9ffda2843f610b894 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Mon, 30 Dec 2024 14:28:32 -0500 Subject: [PATCH 1/2] - Don't exclude the "PATH" var from the environment when running app launchers in jpackage tests. Removal of the "PATH" var was a part of the JDK-8254920 fix. But this removal is redundant as JDK-8254920 was fixed by making jpackage app launcher not to look up for dll-s in the PATH. There are a few jpackage tests running app launchers without removing the PATH var from the environment - WinChildProcessTest, AppLauncherEnvTest. They proves that removal of the "PATH" var from the environment when testing app launchers is redundant. - Add support to remove any var from the environment in Executor class. - Add support for boolean property 'jpackage.test.clear-app-launcher-java-env-vars'. If set to true, "JAVA_TOOL_OPTIONS" and "_JAVA_OPTIONS" will be removed from the environment when running app launchers. This is needed to support local jpackage test configurations when JRE used to run jpackage tests relies on these variables, but they should not be applied to runtimes of app launchers. --- .../helpers/jdk/jpackage/test/Executor.java | 17 +++++++++-------- .../helpers/jdk/jpackage/test/HelloApp.java | 19 +++++++++++++++---- .../jpackage/share/AppLauncherEnvTest.java | 4 +++- .../jpackage/windows/WinChildProcessTest.java | 6 ++++-- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java index 7a0878d826b43..d3240a301bc9f 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java @@ -53,7 +53,6 @@ public static Executor of(String... cmdline) { public Executor() { saveOutputType = new HashSet<>(Set.of(SaveOutputType.NONE)); - removePathEnvVar = false; winEnglishOutput = false; } @@ -86,8 +85,8 @@ public Executor setExecutable(JavaTool v) { return setExecutable(v.getPath()); } - public Executor setRemovePathEnvVar(boolean value) { - removePathEnvVar = value; + public Executor removeEnvVar(String envVarName) { + removeEnvVars.add(Objects.requireNonNull(envVarName)); return this; } @@ -372,10 +371,12 @@ private Result runExecutable() throws IOException, InterruptedException { builder.directory(directory.toFile()); sb.append(String.format("; in directory [%s]", directory)); } - if (removePathEnvVar) { - // run this with cleared Path in Environment - TKit.trace("Clearing PATH in environment"); - builder.environment().remove("PATH"); + if (!removeEnvVars.isEmpty()) { + final var envComm = Comm.compare(builder.environment().keySet(), removeEnvVars); + builder.environment().keySet().removeAll(envComm.common()); + envComm.common().forEach(envVar -> { + TKit.trace(String.format("Clearing %s in environment", envVar)); + }); } trace("Execute " + sb.toString() + "..."); @@ -504,7 +505,7 @@ private static void trace(String msg) { private Path executable; private Set saveOutputType; private Path directory; - private boolean removePathEnvVar; + private Set removeEnvVars = new HashSet<>(); private boolean winEnglishOutput; private String winTmpDir = null; diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java index 0c7476e863dc4..359a6c2fb6ce3 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java @@ -472,14 +472,14 @@ private Executor getExecutor(String...args) { } } - final List launcherArgs = List.of(args); - return new Executor() + final var executor = new Executor() .setDirectory(outputFile.getParent()) .saveOutput(saveOutput) .dumpOutput() - .setRemovePathEnvVar(removePathEnvVar) .setExecutable(executablePath) - .addArguments(launcherArgs); + .addArguments(List.of(args)); + + return configureEnvironment(executor); } private boolean launcherNoExit; @@ -496,6 +496,14 @@ public static AppOutputVerifier assertApp(Path helloAppLauncher) { return new AppOutputVerifier(helloAppLauncher); } + public static Executor configureEnvironment(Executor executor) { + if (CLEAR_JAVA_ENV_VARS) { + executor.removeEnvVar("JAVA_TOOL_OPTIONS"); + executor.removeEnvVar("_JAVA_OPTIONS"); + } + return executor; + } + static final String OUTPUT_FILENAME = "appOutput.txt"; private final JavaAppDesc appDesc; @@ -505,4 +513,7 @@ public static AppOutputVerifier assertApp(Path helloAppLauncher) { private static final String CLASS_NAME = HELLO_JAVA.getFileName().toString().split( "\\.", 2)[0]; + + private static final boolean CLEAR_JAVA_ENV_VARS = Optional.ofNullable( + TKit.getConfigProperty("clear-app-launcher-java-env-vars")).map(Boolean::parseBoolean).orElse(false); } diff --git a/test/jdk/tools/jpackage/share/AppLauncherEnvTest.java b/test/jdk/tools/jpackage/share/AppLauncherEnvTest.java index a16ff9c18f96d..61ed679c29149 100644 --- a/test/jdk/tools/jpackage/share/AppLauncherEnvTest.java +++ b/test/jdk/tools/jpackage/share/AppLauncherEnvTest.java @@ -30,6 +30,7 @@ import jdk.jpackage.test.JPackageCommand; import jdk.jpackage.test.Annotations.Test; import jdk.jpackage.test.Executor; +import static jdk.jpackage.test.HelloApp.configureEnvironment; import jdk.jpackage.test.TKit; /** @@ -53,6 +54,7 @@ public static void test() throws Exception { JPackageCommand cmd = JPackageCommand .helloAppImage(TEST_APP_JAVA + "*Hello") + .ignoreFakeRuntime() .addArguments("--java-options", "-D" + testAddDirProp + "=$APPDIR"); @@ -62,7 +64,7 @@ public static void test() throws Exception { final int attempts = 3; final int waitBetweenAttemptsSeconds = 5; - List output = new Executor() + List output = configureEnvironment(new Executor()) .saveOutput() .setExecutable(cmd.appLauncherPath().toAbsolutePath()) .addArguments("--print-env-var=" + envVarName) diff --git a/test/jdk/tools/jpackage/windows/WinChildProcessTest.java b/test/jdk/tools/jpackage/windows/WinChildProcessTest.java index 5565d3dc50352..b46f84f8c48bb 100644 --- a/test/jdk/tools/jpackage/windows/WinChildProcessTest.java +++ b/test/jdk/tools/jpackage/windows/WinChildProcessTest.java @@ -40,6 +40,7 @@ import java.nio.file.Path; import jdk.jpackage.test.JPackageCommand; +import static jdk.jpackage.test.HelloApp.configureEnvironment; import jdk.jpackage.test.Annotations.Test; import jdk.jpackage.test.Executor; import jdk.jpackage.test.TKit; @@ -54,14 +55,15 @@ public static void test() { long childPid = 0; try { JPackageCommand cmd = JPackageCommand - .helloAppImage(TEST_APP_JAVA + "*Hello"); + .helloAppImage(TEST_APP_JAVA + "*Hello") + .ignoreFakeRuntime(); // Create the image of the third party application launcher cmd.executeAndAssertImageCreated(); // Start the third party application launcher and dump and save the // output of the application - List output = new Executor().saveOutput().dumpOutput() + List output = configureEnvironment(new Executor()).saveOutput().dumpOutput() .setExecutable(cmd.appLauncherPath().toAbsolutePath()) .execute(0).getOutput(); String pidStr = output.get(0); From a865317ab4b7a9245033afb5482b29a97af45c07 Mon Sep 17 00:00:00 2001 From: Alexey Semenyuk Date: Wed, 8 Jan 2025 18:24:44 -0500 Subject: [PATCH 2/2] Fix copyright year --- test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java | 2 +- test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java | 2 +- test/jdk/tools/jpackage/share/AppLauncherEnvTest.java | 2 +- test/jdk/tools/jpackage/windows/WinChildProcessTest.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java index d3240a301bc9f..706e26668b191 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java index 359a6c2fb6ce3..15fb41ca1e243 100644 --- a/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java +++ b/test/jdk/tools/jpackage/helpers/jdk/jpackage/test/HelloApp.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/test/jdk/tools/jpackage/share/AppLauncherEnvTest.java b/test/jdk/tools/jpackage/share/AppLauncherEnvTest.java index 61ed679c29149..52016e6f4ab6a 100644 --- a/test/jdk/tools/jpackage/share/AppLauncherEnvTest.java +++ b/test/jdk/tools/jpackage/share/AppLauncherEnvTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it diff --git a/test/jdk/tools/jpackage/windows/WinChildProcessTest.java b/test/jdk/tools/jpackage/windows/WinChildProcessTest.java index b46f84f8c48bb..5b0ff9b1f01b4 100644 --- a/test/jdk/tools/jpackage/windows/WinChildProcessTest.java +++ b/test/jdk/tools/jpackage/windows/WinChildProcessTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it