From 5f6b1cce4a8193078e7e1212dee50dde857157a4 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Fri, 11 Apr 2025 18:13:53 +0200 Subject: [PATCH 1/3] Fix support for JVM shutdown hooks When the `ConsoleLauncher` is executed via its `main` method while passing arguments via its `-cp` option, it no longer closes the custom class loader it creates eagerly after test discovery or execution. Instead, it relies on the JVM shutdown to free up any resource held by the class loader which is initiated by calling `System.exit` directly afterward. Fixes #4469. --- .../platform/console/ConsoleLauncher.java | 12 ++- .../console/tasks/ConsoleTestExecutor.java | 17 ++-- .../tasks/CustomClassLoaderCloseStrategy.java | 68 +++++++++++++++ .../CustomContextClassLoaderExecutor.java | 20 ++--- .../console/ConsoleLauncherWrapper.java | 4 +- .../tasks/ConsoleTestExecutorTests.java | 18 ++-- ...CustomContextClassLoaderExecutorTests.java | 35 ++++++-- .../other/OtherwiseNotReferencedClass.java | 14 ++++ .../src/standalone/JupiterIntegration.java | 3 + .../support/tests/StandaloneTests.java | 84 ++++++++++--------- 10 files changed, 197 insertions(+), 78 deletions(-) create mode 100644 junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomClassLoaderCloseStrategy.java create mode 100644 platform-tooling-support-tests/projects/standalone/src/other/OtherwiseNotReferencedClass.java diff --git a/junit-platform-console/src/main/java/org/junit/platform/console/ConsoleLauncher.java b/junit-platform-console/src/main/java/org/junit/platform/console/ConsoleLauncher.java index 240d6216f61e..929ec8c035fe 100644 --- a/junit-platform-console/src/main/java/org/junit/platform/console/ConsoleLauncher.java +++ b/junit-platform-console/src/main/java/org/junit/platform/console/ConsoleLauncher.java @@ -19,6 +19,7 @@ import org.junit.platform.console.options.CommandFacade; import org.junit.platform.console.options.CommandResult; import org.junit.platform.console.tasks.ConsoleTestExecutor; +import org.junit.platform.console.tasks.CustomClassLoaderCloseStrategy; /** * The {@code ConsoleLauncher} is a stand-alone application for launching the @@ -30,17 +31,20 @@ public class ConsoleLauncher { public static void main(String... args) { - CommandResult result = newCommandFacade().run(args); + CommandFacade facade = newCommandFacade(CustomClassLoaderCloseStrategy.KEEP_OPEN); + CommandResult result = facade.run(args); System.exit(result.getExitCode()); } @API(status = INTERNAL, since = "1.0") public static CommandResult run(PrintWriter out, PrintWriter err, String... args) { - return newCommandFacade().run(args, out, err); + CommandFacade facade = newCommandFacade(CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER); + return facade.run(args, out, err); } - private static CommandFacade newCommandFacade() { - return new CommandFacade(ConsoleTestExecutor::new); + private static CommandFacade newCommandFacade(CustomClassLoaderCloseStrategy classLoaderCleanupStrategy) { + return new CommandFacade((discoveryOptions, outputOptions) -> new ConsoleTestExecutor(discoveryOptions, + outputOptions, classLoaderCleanupStrategy)); } } diff --git a/junit-platform-console/src/main/java/org/junit/platform/console/tasks/ConsoleTestExecutor.java b/junit-platform-console/src/main/java/org/junit/platform/console/tasks/ConsoleTestExecutor.java index ed08f30b5adc..f91412cb07a3 100644 --- a/junit-platform-console/src/main/java/org/junit/platform/console/tasks/ConsoleTestExecutor.java +++ b/junit-platform-console/src/main/java/org/junit/platform/console/tasks/ConsoleTestExecutor.java @@ -50,31 +50,38 @@ public class ConsoleTestExecutor { private final TestDiscoveryOptions discoveryOptions; private final TestConsoleOutputOptions outputOptions; private final Supplier launcherSupplier; + private final CustomClassLoaderCloseStrategy classLoaderCloseStrategy; - public ConsoleTestExecutor(TestDiscoveryOptions discoveryOptions, TestConsoleOutputOptions outputOptions) { - this(discoveryOptions, outputOptions, LauncherFactory::create); + public ConsoleTestExecutor(TestDiscoveryOptions discoveryOptions, TestConsoleOutputOptions outputOptions, + CustomClassLoaderCloseStrategy classLoaderCloseStrategy) { + this(discoveryOptions, outputOptions, classLoaderCloseStrategy, LauncherFactory::create); } // for tests only ConsoleTestExecutor(TestDiscoveryOptions discoveryOptions, TestConsoleOutputOptions outputOptions, - Supplier launcherSupplier) { + CustomClassLoaderCloseStrategy classLoaderCloseStrategy, Supplier launcherSupplier) { this.discoveryOptions = discoveryOptions; this.outputOptions = outputOptions; this.launcherSupplier = launcherSupplier; + this.classLoaderCloseStrategy = classLoaderCloseStrategy; } public void discover(PrintWriter out) { - new CustomContextClassLoaderExecutor(createCustomClassLoader()).invoke(() -> { + createCustomContextClassLoaderExecutor().invoke(() -> { discoverTests(out); return null; }); } public TestExecutionSummary execute(PrintWriter out, Optional reportsDir) { - return new CustomContextClassLoaderExecutor(createCustomClassLoader()) // + return createCustomContextClassLoaderExecutor() // .invoke(() -> executeTests(out, reportsDir)); } + private CustomContextClassLoaderExecutor createCustomContextClassLoaderExecutor() { + return new CustomContextClassLoaderExecutor(createCustomClassLoader(), classLoaderCloseStrategy); + } + private void discoverTests(PrintWriter out) { Launcher launcher = launcherSupplier.get(); Optional commandLineTestPrinter = createDetailsPrintingListener(out); diff --git a/junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomClassLoaderCloseStrategy.java b/junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomClassLoaderCloseStrategy.java new file mode 100644 index 000000000000..8a4694ba2156 --- /dev/null +++ b/junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomClassLoaderCloseStrategy.java @@ -0,0 +1,68 @@ +/* + * Copyright 2015-2025 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.platform.console.tasks; + +import static org.apiguardian.api.API.Status.INTERNAL; + +import org.apiguardian.api.API; +import org.junit.platform.commons.JUnitException; + +/** + * Defines the strategy for closing custom class loaders created for test + * discovery and execution. + */ +@API(status = INTERNAL, since = "1.13") +public enum CustomClassLoaderCloseStrategy { + + /** + * Close the custom class loader after calling the + * {@link org.junit.platform.launcher.Launcher} for test discovery or + * execution. + */ + CLOSE_AFTER_CALLING_LAUNCHER { + + @Override + public void handle(ClassLoader customClassLoader) { + if (customClassLoader instanceof AutoCloseable) { + close((AutoCloseable) customClassLoader); + } + } + + private void close(AutoCloseable customClassLoader) { + try { + customClassLoader.close(); + } + catch (Exception e) { + throw new JUnitException("Failed to close custom class loader", e); + } + } + }, + + /** + * Rely on the JVM to release resources held by the custom class loader when + * it terminates. + * + *

This mode is only safe to use when calling {@link System#exit(int)} + * afterward. + */ + KEEP_OPEN { + @Override + public void handle(ClassLoader customClassLoader) { + // do nothing + } + }; + + /** + * Handle the class loader according to the strategy. + */ + public abstract void handle(ClassLoader classLoader); + +} diff --git a/junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutor.java b/junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutor.java index 0313fe16ca5c..9e22232de826 100644 --- a/junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutor.java +++ b/junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutor.java @@ -13,17 +13,18 @@ import java.util.Optional; import java.util.function.Supplier; -import org.junit.platform.commons.JUnitException; - /** * @since 1.0 */ class CustomContextClassLoaderExecutor { private final Optional customClassLoader; + private final CustomClassLoaderCloseStrategy closeStrategy; - CustomContextClassLoaderExecutor(Optional customClassLoader) { + CustomContextClassLoaderExecutor(Optional customClassLoader, + CustomClassLoaderCloseStrategy closeStrategy) { this.customClassLoader = customClassLoader; + this.closeStrategy = closeStrategy; } T invoke(Supplier supplier) { @@ -43,18 +44,7 @@ private T replaceThreadContextClassLoaderAndInvoke(ClassLoader customClassLo } finally { Thread.currentThread().setContextClassLoader(originalClassLoader); - if (customClassLoader instanceof AutoCloseable) { - close((AutoCloseable) customClassLoader); - } - } - } - - private static void close(AutoCloseable customClassLoader) { - try { - customClassLoader.close(); - } - catch (Exception e) { - throw new JUnitException("Failed to close custom class loader", e); + closeStrategy.handle(customClassLoader); } } diff --git a/platform-tests/src/test/java/org/junit/platform/console/ConsoleLauncherWrapper.java b/platform-tests/src/test/java/org/junit/platform/console/ConsoleLauncherWrapper.java index 03fb3528f543..af21efc3f12d 100644 --- a/platform-tests/src/test/java/org/junit/platform/console/ConsoleLauncherWrapper.java +++ b/platform-tests/src/test/java/org/junit/platform/console/ConsoleLauncherWrapper.java @@ -19,6 +19,7 @@ import org.junit.platform.console.options.CommandFacade; import org.junit.platform.console.tasks.ConsoleTestExecutor; +import org.junit.platform.console.tasks.CustomClassLoaderCloseStrategy; /** * @since 1.0 @@ -30,7 +31,8 @@ class ConsoleLauncherWrapper { private final ConsoleTestExecutor.Factory consoleTestExecutorFactory; ConsoleLauncherWrapper() { - this(ConsoleTestExecutor::new); + this((discoveryOptions, outputOptions) -> new ConsoleTestExecutor(discoveryOptions, outputOptions, + CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER)); } private ConsoleLauncherWrapper(ConsoleTestExecutor.Factory consoleTestExecutorFactory) { diff --git a/platform-tests/src/test/java/org/junit/platform/console/tasks/ConsoleTestExecutorTests.java b/platform-tests/src/test/java/org/junit/platform/console/tasks/ConsoleTestExecutorTests.java index f3c99037a180..9be6b2ce0495 100644 --- a/platform-tests/src/test/java/org/junit/platform/console/tasks/ConsoleTestExecutorTests.java +++ b/platform-tests/src/test/java/org/junit/platform/console/tasks/ConsoleTestExecutorTests.java @@ -52,7 +52,8 @@ void printsSummary() { dummyTestEngine.addTest("succeedingTest", SUCCEEDING_TEST); dummyTestEngine.addTest("failingTest", FAILING_BLOCK); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, + CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).contains("Test run finished after", "2 tests found", "0 tests skipped", @@ -65,7 +66,8 @@ void printsDetailsIfTheyAreNotHidden() { dummyTestEngine.addTest("failingTest", FAILING_BLOCK); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, + CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).contains("Test execution started."); @@ -77,7 +79,8 @@ void printsNoDetailsIfTheyAreHidden() { dummyTestEngine.addTest("failingTest", FAILING_BLOCK); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, + CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).doesNotContain("Test execution started."); @@ -90,7 +93,8 @@ void printsFailuresEvenIfDetailsAreHidden() { dummyTestEngine.addTest("failingTest", FAILING_BLOCK); dummyTestEngine.addContainer("failingContainer", FAILING_BLOCK); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, + CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).contains("Failures (2)", "failingTest", "failingContainer"); @@ -104,7 +108,8 @@ void usesCustomClassLoaderIfAdditionalClassPathEntriesArePresent() { dummyTestEngine.addTest("failingTest", () -> assertSame(oldClassLoader, getDefaultClassLoader(), "should fail")); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, + CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).contains("failingTest", "should fail", "1 tests failed"); @@ -118,7 +123,8 @@ void usesSameClassLoaderIfNoAdditionalClassPathEntriesArePresent() { dummyTestEngine.addTest("failingTest", () -> assertNotSame(oldClassLoader, getDefaultClassLoader(), "should fail")); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, + CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).contains("failingTest", "should fail", "1 tests failed"); diff --git a/platform-tests/src/test/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutorTests.java b/platform-tests/src/test/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutorTests.java index 0c0d6e9ec935..e3d8b8ab7b4d 100644 --- a/platform-tests/src/test/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutorTests.java +++ b/platform-tests/src/test/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutorTests.java @@ -11,6 +11,7 @@ package org.junit.platform.console.tasks; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -28,9 +29,10 @@ class CustomContextClassLoaderExecutorTests { @Test - void invokeWithoutCustomClassLoaderDoesNotSetClassLoader() throws Exception { + void invokeWithoutCustomClassLoaderDoesNotSetClassLoader() { var originalClassLoader = Thread.currentThread().getContextClassLoader(); - var executor = new CustomContextClassLoaderExecutor(Optional.empty()); + var executor = new CustomContextClassLoaderExecutor(Optional.empty(), + CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER); int result = executor.invoke(() -> { assertSame(originalClassLoader, Thread.currentThread().getContextClassLoader()); @@ -42,10 +44,11 @@ void invokeWithoutCustomClassLoaderDoesNotSetClassLoader() throws Exception { } @Test - void invokeWithCustomClassLoaderSetsCustomAndResetsToOriginal() throws Exception { + void invokeWithCustomClassLoaderSetsCustomAndResetsToOriginal() { var originalClassLoader = Thread.currentThread().getContextClassLoader(); ClassLoader customClassLoader = URLClassLoader.newInstance(new URL[0]); - var executor = new CustomContextClassLoaderExecutor(Optional.of(customClassLoader)); + var executor = new CustomContextClassLoaderExecutor(Optional.of(customClassLoader), + CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER); int result = executor.invoke(() -> { assertSame(customClassLoader, Thread.currentThread().getContextClassLoader()); @@ -57,7 +60,7 @@ void invokeWithCustomClassLoaderSetsCustomAndResetsToOriginal() throws Exception } @Test - void invokeWithCustomClassLoaderAndEnsureItIsClosedAfterUsage() throws Exception { + void invokeWithCustomClassLoaderAndEnsureItIsClosedAfterUsage() { var closed = new AtomicBoolean(false); ClassLoader localClassLoader = new URLClassLoader(new URL[0]) { @Override @@ -66,11 +69,31 @@ public void close() throws IOException { super.close(); } }; - var executor = new CustomContextClassLoaderExecutor(Optional.of(localClassLoader)); + var executor = new CustomContextClassLoaderExecutor(Optional.of(localClassLoader), + CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER); int result = executor.invoke(() -> 4711); assertEquals(4711, result); assertTrue(closed.get()); } + + @Test + void invokeWithCustomClassLoaderAndKeepItOpenAfterUsage() { + var closed = new AtomicBoolean(false); + ClassLoader localClassLoader = new URLClassLoader(new URL[0]) { + @Override + public void close() throws IOException { + closed.set(true); + super.close(); + } + }; + var executor = new CustomContextClassLoaderExecutor(Optional.of(localClassLoader), + CustomClassLoaderCloseStrategy.KEEP_OPEN); + + int result = executor.invoke(() -> 4711); + + assertEquals(4711, result); + assertFalse(closed.get()); + } } diff --git a/platform-tooling-support-tests/projects/standalone/src/other/OtherwiseNotReferencedClass.java b/platform-tooling-support-tests/projects/standalone/src/other/OtherwiseNotReferencedClass.java new file mode 100644 index 000000000000..81be14e5346c --- /dev/null +++ b/platform-tooling-support-tests/projects/standalone/src/other/OtherwiseNotReferencedClass.java @@ -0,0 +1,14 @@ +/* + * Copyright 2015-2025 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package other; + +public class OtherwiseNotReferencedClass { +} diff --git a/platform-tooling-support-tests/projects/standalone/src/standalone/JupiterIntegration.java b/platform-tooling-support-tests/projects/standalone/src/standalone/JupiterIntegration.java index e8bbce2489cb..a2a1267e3f44 100644 --- a/platform-tooling-support-tests/projects/standalone/src/standalone/JupiterIntegration.java +++ b/platform-tooling-support-tests/projects/standalone/src/standalone/JupiterIntegration.java @@ -19,6 +19,9 @@ class JupiterIntegration { @Test void successful() { + Runtime.getRuntime().addShutdownHook(new Thread(() -> { + new other.OtherwiseNotReferencedClass(); + })); } @Test diff --git a/platform-tooling-support-tests/src/test/java/platform/tooling/support/tests/StandaloneTests.java b/platform-tooling-support-tests/src/test/java/platform/tooling/support/tests/StandaloneTests.java index 6cbb5bc931e1..7ccee58473f9 100644 --- a/platform-tooling-support-tests/src/test/java/platform/tooling/support/tests/StandaloneTests.java +++ b/platform-tooling-support-tests/src/test/java/platform/tooling/support/tests/StandaloneTests.java @@ -30,7 +30,6 @@ import java.util.stream.Stream; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; @@ -155,8 +154,7 @@ void printVersionViaModule(@FilePrefix("java") OutputFiles outputFiles) throws E @Test @Order(1) @Execution(SAME_THREAD) - void compile(@FilePrefix("javac") OutputFiles javacOutputFiles, @FilePrefix("jar") OutputFiles jarOutputFiles) - throws Exception { + void compile(@FilePrefix("javac") OutputFiles javacOutputFiles) throws Exception { var result = ProcessStarters.javaCommand("javac") // .workingDir(workspace) // .addArguments("-Xlint:-options") // @@ -164,6 +162,7 @@ void compile(@FilePrefix("javac") OutputFiles javacOutputFiles, @FilePrefix("jar .addArguments("-proc:none") // .addArguments("-d", workspace.resolve("bin").toString()) // .addArguments("--class-path", MavenRepo.jar("junit-platform-console-standalone").toString()) // + .addArguments(workspace.resolve("src/other/OtherwiseNotReferencedClass.java").toString()) // .addArguments(workspace.resolve("src/standalone/JupiterIntegration.java").toString()) // .addArguments(workspace.resolve("src/standalone/JupiterParamsIntegration.java").toString()) // .addArguments(workspace.resolve("src/standalone/SuiteIntegration.java").toString()) // @@ -174,17 +173,6 @@ void compile(@FilePrefix("javac") OutputFiles javacOutputFiles, @FilePrefix("jar assertEquals(0, result.exitCode()); assertTrue(result.stdOut().isEmpty()); assertTrue(result.stdErr().isEmpty()); - - // create "tests.jar" that'll be picked-up by "testWithJarredTestClasses()" later - var jarFolder = Files.createDirectories(workspace.resolve("jar")); - var jarResult = ProcessStarters.javaCommand("jar") // - .workingDir(workspace) // - .addArguments("--create") // - .addArguments("--file", jarFolder.resolve("tests.jar").toString()) // - .addArguments("-C", workspace.resolve("bin").toString(), ".") // - .redirectOutput(jarOutputFiles) // - .startAndWait(); - assertEquals(0, jarResult.exitCode()); } @Test @@ -431,23 +419,7 @@ void execute(@FilePrefix("console-launcher") OutputFiles outputFiles) throws Exc assertEquals(1, result.exitCode()); - var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt")); - var expectedErrLines = Files.readAllLines(workspace.resolve("expected-err.txt")); - assertLinesMatch(expectedOutLines, result.stdOutLines()); - var actualErrLines = result.stdErrLines(); - if (actualErrLines.getFirst().contains("stty: /dev/tty: No such device or address")) { - // Happens intermittently on GitHub Actions on Windows - actualErrLines = new ArrayList<>(actualErrLines); - actualErrLines.removeFirst(); - } - assertLinesMatch(expectedErrLines, actualErrLines); - - var jupiterVersion = Helper.version("junit-jupiter-engine"); - var vintageVersion = Helper.version("junit-vintage-engine"); - assertTrue(result.stdErr().contains("junit-jupiter" - + " (group ID: org.junit.jupiter, artifact ID: junit-jupiter-engine, version: " + jupiterVersion)); - assertTrue(result.stdErr().contains("junit-vintage" - + " (group ID: org.junit.vintage, artifact ID: junit-vintage-engine, version: " + vintageVersion)); + assertOutputOnCurrentJvm(result); } @Test @@ -530,27 +502,57 @@ private static List getExpectedErrLinesOnJava8(Path workspace) throws IO @Test @Order(6) @Execution(SAME_THREAD) - @Disabled("https://github.com/junit-team/junit5/issues/1724") - void executeWithJarredTestClasses(@FilePrefix("console-launcher") OutputFiles outputFiles) throws Exception { - var jar = MavenRepo.jar("junit-platform-console-standalone"); - var path = new ArrayList(); - // path.add("bin"); // "exploded" test classes are found, see also test() above - path.add(workspace.resolve("standalone/jar/tests.jar").toAbsolutePath().toString()); - path.add(jar.toString()); + void executeWithJarredTestClasses(@FilePrefix("jar") OutputFiles jarOutputFiles, + @FilePrefix("console-launcher") OutputFiles outputFiles) throws Exception { + var jar = workspace.resolve("tests.jar"); + var jarResult = ProcessStarters.javaCommand("jar") // + .workingDir(workspace) // + .addArguments("--create") // + .addArguments("--file", jar.toAbsolutePath().toString()) // + .addArguments("-C", workspace.resolve("bin").toString(), ".") // + .redirectOutput(jarOutputFiles) // + .startAndWait(); + + assertEquals(0, jarResult.exitCode()); + var result = ProcessStarters.java() // + .workingDir(workspace) // + .putEnvironment("NO_COLOR", "1") // --disable-ansi-colors .addArguments("--show-version") // .addArguments("-enableassertions") // .addArguments("-Djava.util.logging.config.file=logging.properties") // - .addArguments("--class-path", String.join(File.pathSeparator, path)) // - .addArguments("org.junit.platform.console.ConsoleLauncher") // + .addArguments("-Djunit.platform.launcher.interceptors.enabled=true") // + .addArguments("-jar", MavenRepo.jar("junit-platform-console-standalone").toString()) // .addArguments("execute") // .addArguments("--scan-class-path") // .addArguments("--disable-banner") // .addArguments("--include-classname", "standalone.*") // - .addArguments("--fail-if-no-tests") // + .addArguments("--classpath", jar.toAbsolutePath().toString()) // .redirectOutput(outputFiles) // .startAndWait(); assertEquals(1, result.exitCode()); + + assertOutputOnCurrentJvm(result); + } + + private static void assertOutputOnCurrentJvm(ProcessResult result) throws IOException { + var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt")); + var expectedErrLines = Files.readAllLines(workspace.resolve("expected-err.txt")); + assertLinesMatch(expectedOutLines, result.stdOutLines()); + var actualErrLines = result.stdErrLines(); + if (actualErrLines.getFirst().contains("stty: /dev/tty: No such device or address")) { + // Happens intermittently on GitHub Actions on Windows + actualErrLines = new ArrayList<>(actualErrLines); + actualErrLines.removeFirst(); + } + assertLinesMatch(expectedErrLines, actualErrLines); + + var jupiterVersion = Helper.version("junit-jupiter-engine"); + var vintageVersion = Helper.version("junit-vintage-engine"); + assertTrue(result.stdErr().contains("junit-jupiter" + + " (group ID: org.junit.jupiter, artifact ID: junit-jupiter-engine, version: " + jupiterVersion)); + assertTrue(result.stdErr().contains("junit-vintage" + + " (group ID: org.junit.vintage, artifact ID: junit-vintage-engine, version: " + vintageVersion)); } } From 1ba6fd70daeae99e58e0b38fc0e70978936fbf78 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Mon, 14 Apr 2025 10:26:02 +0200 Subject: [PATCH 2/3] Make `CLOSE_AFTER_CALLING_LAUNCHER` the default to reduce no. of changes --- .../console/tasks/ConsoleTestExecutor.java | 10 ++++++++++ .../CustomContextClassLoaderExecutor.java | 4 ++++ .../console/ConsoleLauncherWrapper.java | 4 +--- .../tasks/ConsoleTestExecutorTests.java | 18 ++++++------------ .../CustomContextClassLoaderExecutorTests.java | 9 +++------ 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/junit-platform-console/src/main/java/org/junit/platform/console/tasks/ConsoleTestExecutor.java b/junit-platform-console/src/main/java/org/junit/platform/console/tasks/ConsoleTestExecutor.java index f91412cb07a3..eb0f3874a8e3 100644 --- a/junit-platform-console/src/main/java/org/junit/platform/console/tasks/ConsoleTestExecutor.java +++ b/junit-platform-console/src/main/java/org/junit/platform/console/tasks/ConsoleTestExecutor.java @@ -52,6 +52,10 @@ public class ConsoleTestExecutor { private final Supplier launcherSupplier; private final CustomClassLoaderCloseStrategy classLoaderCloseStrategy; + public ConsoleTestExecutor(TestDiscoveryOptions discoveryOptions, TestConsoleOutputOptions outputOptions) { + this(discoveryOptions, outputOptions, CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER); + } + public ConsoleTestExecutor(TestDiscoveryOptions discoveryOptions, TestConsoleOutputOptions outputOptions, CustomClassLoaderCloseStrategy classLoaderCloseStrategy) { this(discoveryOptions, outputOptions, classLoaderCloseStrategy, LauncherFactory::create); @@ -59,6 +63,12 @@ public ConsoleTestExecutor(TestDiscoveryOptions discoveryOptions, TestConsoleOut // for tests only ConsoleTestExecutor(TestDiscoveryOptions discoveryOptions, TestConsoleOutputOptions outputOptions, + Supplier launcherSupplier) { + this(discoveryOptions, outputOptions, CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, + launcherSupplier); + } + + private ConsoleTestExecutor(TestDiscoveryOptions discoveryOptions, TestConsoleOutputOptions outputOptions, CustomClassLoaderCloseStrategy classLoaderCloseStrategy, Supplier launcherSupplier) { this.discoveryOptions = discoveryOptions; this.outputOptions = outputOptions; diff --git a/junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutor.java b/junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutor.java index 9e22232de826..4e1a4e0f3dda 100644 --- a/junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutor.java +++ b/junit-platform-console/src/main/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutor.java @@ -21,6 +21,10 @@ class CustomContextClassLoaderExecutor { private final Optional customClassLoader; private final CustomClassLoaderCloseStrategy closeStrategy; + CustomContextClassLoaderExecutor(Optional customClassLoader) { + this(customClassLoader, CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER); + } + CustomContextClassLoaderExecutor(Optional customClassLoader, CustomClassLoaderCloseStrategy closeStrategy) { this.customClassLoader = customClassLoader; diff --git a/platform-tests/src/test/java/org/junit/platform/console/ConsoleLauncherWrapper.java b/platform-tests/src/test/java/org/junit/platform/console/ConsoleLauncherWrapper.java index af21efc3f12d..03fb3528f543 100644 --- a/platform-tests/src/test/java/org/junit/platform/console/ConsoleLauncherWrapper.java +++ b/platform-tests/src/test/java/org/junit/platform/console/ConsoleLauncherWrapper.java @@ -19,7 +19,6 @@ import org.junit.platform.console.options.CommandFacade; import org.junit.platform.console.tasks.ConsoleTestExecutor; -import org.junit.platform.console.tasks.CustomClassLoaderCloseStrategy; /** * @since 1.0 @@ -31,8 +30,7 @@ class ConsoleLauncherWrapper { private final ConsoleTestExecutor.Factory consoleTestExecutorFactory; ConsoleLauncherWrapper() { - this((discoveryOptions, outputOptions) -> new ConsoleTestExecutor(discoveryOptions, outputOptions, - CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER)); + this(ConsoleTestExecutor::new); } private ConsoleLauncherWrapper(ConsoleTestExecutor.Factory consoleTestExecutorFactory) { diff --git a/platform-tests/src/test/java/org/junit/platform/console/tasks/ConsoleTestExecutorTests.java b/platform-tests/src/test/java/org/junit/platform/console/tasks/ConsoleTestExecutorTests.java index 9be6b2ce0495..f3c99037a180 100644 --- a/platform-tests/src/test/java/org/junit/platform/console/tasks/ConsoleTestExecutorTests.java +++ b/platform-tests/src/test/java/org/junit/platform/console/tasks/ConsoleTestExecutorTests.java @@ -52,8 +52,7 @@ void printsSummary() { dummyTestEngine.addTest("succeedingTest", SUCCEEDING_TEST); dummyTestEngine.addTest("failingTest", FAILING_BLOCK); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, - CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).contains("Test run finished after", "2 tests found", "0 tests skipped", @@ -66,8 +65,7 @@ void printsDetailsIfTheyAreNotHidden() { dummyTestEngine.addTest("failingTest", FAILING_BLOCK); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, - CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).contains("Test execution started."); @@ -79,8 +77,7 @@ void printsNoDetailsIfTheyAreHidden() { dummyTestEngine.addTest("failingTest", FAILING_BLOCK); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, - CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).doesNotContain("Test execution started."); @@ -93,8 +90,7 @@ void printsFailuresEvenIfDetailsAreHidden() { dummyTestEngine.addTest("failingTest", FAILING_BLOCK); dummyTestEngine.addContainer("failingContainer", FAILING_BLOCK); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, - CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).contains("Failures (2)", "failingTest", "failingContainer"); @@ -108,8 +104,7 @@ void usesCustomClassLoaderIfAdditionalClassPathEntriesArePresent() { dummyTestEngine.addTest("failingTest", () -> assertSame(oldClassLoader, getDefaultClassLoader(), "should fail")); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, - CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).contains("failingTest", "should fail", "1 tests failed"); @@ -123,8 +118,7 @@ void usesSameClassLoaderIfNoAdditionalClassPathEntriesArePresent() { dummyTestEngine.addTest("failingTest", () -> assertNotSame(oldClassLoader, getDefaultClassLoader(), "should fail")); - var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, - CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER, () -> createLauncher(dummyTestEngine)); + var task = new ConsoleTestExecutor(discoveryOptions, outputOptions, () -> createLauncher(dummyTestEngine)); task.execute(new PrintWriter(stringWriter), Optional.empty()); assertThat(stringWriter.toString()).contains("failingTest", "should fail", "1 tests failed"); diff --git a/platform-tests/src/test/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutorTests.java b/platform-tests/src/test/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutorTests.java index e3d8b8ab7b4d..9307d4a26754 100644 --- a/platform-tests/src/test/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutorTests.java +++ b/platform-tests/src/test/java/org/junit/platform/console/tasks/CustomContextClassLoaderExecutorTests.java @@ -31,8 +31,7 @@ class CustomContextClassLoaderExecutorTests { @Test void invokeWithoutCustomClassLoaderDoesNotSetClassLoader() { var originalClassLoader = Thread.currentThread().getContextClassLoader(); - var executor = new CustomContextClassLoaderExecutor(Optional.empty(), - CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER); + var executor = new CustomContextClassLoaderExecutor(Optional.empty()); int result = executor.invoke(() -> { assertSame(originalClassLoader, Thread.currentThread().getContextClassLoader()); @@ -47,8 +46,7 @@ void invokeWithoutCustomClassLoaderDoesNotSetClassLoader() { void invokeWithCustomClassLoaderSetsCustomAndResetsToOriginal() { var originalClassLoader = Thread.currentThread().getContextClassLoader(); ClassLoader customClassLoader = URLClassLoader.newInstance(new URL[0]); - var executor = new CustomContextClassLoaderExecutor(Optional.of(customClassLoader), - CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER); + var executor = new CustomContextClassLoaderExecutor(Optional.of(customClassLoader)); int result = executor.invoke(() -> { assertSame(customClassLoader, Thread.currentThread().getContextClassLoader()); @@ -69,8 +67,7 @@ public void close() throws IOException { super.close(); } }; - var executor = new CustomContextClassLoaderExecutor(Optional.of(localClassLoader), - CustomClassLoaderCloseStrategy.CLOSE_AFTER_CALLING_LAUNCHER); + var executor = new CustomContextClassLoaderExecutor(Optional.of(localClassLoader)); int result = executor.invoke(() -> 4711); From 476114c3d94776b630e776880893ada472c08e52 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Tue, 29 Apr 2025 12:29:06 +0200 Subject: [PATCH 3/3] Add to release notes --- .../docs/asciidoc/release-notes/release-notes-5.13.0-M3.adoc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.0-M3.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.0-M3.adoc index 533bee62be63..97646340a4fc 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.0-M3.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.13.0-M3.adoc @@ -16,7 +16,10 @@ repository on GitHub. [[release-notes-5.13.0-M3-junit-platform-bug-fixes]] ==== Bug Fixes -* ❓ +* Reintroduce support for JVM shutdown hooks when using the `-cp`/`--classpath` option of + the `ConsoleLauncher`. Prior to this release, the created class loader was closed prior + to JVM shutdown hooks being invoked, which caused hooks to fail with a + `ClassNotFoundException` when loading classes during shutdown. [[release-notes-5.13.0-M3-junit-platform-deprecations-and-breaking-changes]] ==== Deprecations and Breaking Changes