From 9c611ce9cce36b2618361f3fccd73cd8aa6dcf5f Mon Sep 17 00:00:00 2001 From: Evan Loriot <12630925+evanloriot@users.noreply.github.com> Date: Sat, 15 Nov 2025 11:14:55 -0800 Subject: [PATCH 1/2] Fix shutdown hooks for JUnit5 for JDK24+. Previously, all tests would exit with System.exit or Runtime.exit was called because the shutdown hook was not removed at test teardown. --- .../junit5/JUnit5Runner.java | 16 +++---- .../junit5/NullSystemExitToggle.java | 23 +++++++++- .../junit5/NullSystemExitToggleTest.java | 45 +++++++++++++++++++ 3 files changed, 71 insertions(+), 13 deletions(-) create mode 100644 java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggleTest.java diff --git a/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/JUnit5Runner.java b/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/JUnit5Runner.java index 55a1d390..29047c04 100644 --- a/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/JUnit5Runner.java +++ b/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/JUnit5Runner.java @@ -87,19 +87,13 @@ private static SystemExitToggle getSystemExitToggle() { System.err.println("Failed to load Java 17 system exit override: " + e.getMessage()); } - // Install a shutdown hook so people can track down what went wrong - // if a test calls `System.exit` - Thread shutdownHook = SystemExitDetectingShutdownHook.newShutdownHook(System.err); - Runtime.getRuntime().addShutdownHook(shutdownHook); - // Fall through - } + System.err.println( + "Unable to create a mechanism to prevent `System.exit` being called. " + + "Tests may cause `bazel test` to exit prematurely."); - System.err.println( - "Unable to create a mechanism to prevent `System.exit` being called. " - + "Tests may cause `bazel test` to exit prematurely."); - - return new NullSystemExitToggle(); + return new NullSystemExitToggle(); + } } private static void detectJUnit5Classes() { diff --git a/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggle.java b/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggle.java index f088b659..34b85af9 100644 --- a/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggle.java +++ b/java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggle.java @@ -1,13 +1,32 @@ package com.github.bazel_contrib.contrib_rules_jvm.junit5; +import java.util.Optional; + public class NullSystemExitToggle implements SystemExitToggle { + + private Optional shutdownHook; + + public NullSystemExitToggle() { + this.shutdownHook = Optional.empty(); + } + @Override public void prevent() { - // No-op + if (!shutdownHook.isEmpty()) { + throw new RuntimeException("SystemExitDetectingShutdownHook already added"); + } + // Install a shutdown hook so people can track down what went wrong + // if a test calls `System.exit` + Thread shutdownHook = SystemExitDetectingShutdownHook.newShutdownHook(System.err); + this.shutdownHook = Optional.of(shutdownHook); + Runtime.getRuntime().addShutdownHook(shutdownHook); } @Override public void allow() { - // No-op + if (shutdownHook.isEmpty()) { + throw new RuntimeException("SystemExitDetectingShutdownHook never added"); + } + Runtime.getRuntime().removeShutdownHook(shutdownHook.get()); } } diff --git a/java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggleTest.java b/java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggleTest.java new file mode 100644 index 00000000..5e150903 --- /dev/null +++ b/java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggleTest.java @@ -0,0 +1,45 @@ +package com.github.bazel_contrib.contrib_rules_jvm.junit5; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +public class NullSystemExitToggleTest { + + @Test + public void prevent_alreadyPrevented_throwsRuntimeException() { + // Arrange. + NullSystemExitToggle toggle = new NullSystemExitToggle(); + + // Act and Assert. + toggle.prevent(); + assertThrows(RuntimeException.class, () -> toggle.prevent()); + + // Cleanup. + toggle.allow(); + } + + + @Test + public void allow_alreadyAllowed_throwsRuntimeException() { + // Arrange. + NullSystemExitToggle toggle = new NullSystemExitToggle(); + + // Act and Assert. + assertThrows(RuntimeException.class, () -> toggle.allow()); + } + + @Test + public void normalFlow_success() { + // Arrange. + NullSystemExitToggle toggle = new NullSystemExitToggle(); + + // Act and (implicit) Assert. + try { + toggle.prevent(); + } finally { + toggle.allow(); + } + } + +} From 83942ab12e8e0e5aab77850aac5afcf066613a3e Mon Sep 17 00:00:00 2001 From: Evan Loriot <12630925+evanloriot@users.noreply.github.com> Date: Tue, 18 Nov 2025 21:33:30 -0800 Subject: [PATCH 2/2] fix formatting --- .../contrib_rules_jvm/junit5/NullSystemExitToggleTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggleTest.java b/java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggleTest.java index 5e150903..f9e00969 100644 --- a/java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggleTest.java +++ b/java/test/com/github/bazel_contrib/contrib_rules_jvm/junit5/NullSystemExitToggleTest.java @@ -19,7 +19,6 @@ public void prevent_alreadyPrevented_throwsRuntimeException() { toggle.allow(); } - @Test public void allow_alreadyAllowed_throwsRuntimeException() { // Arrange. @@ -41,5 +40,4 @@ public void normalFlow_success() { toggle.allow(); } } - }