Support JDK25 in java_test_suite with junit5 runner

[evanloriot]

When you try to use junit5 test runner with JDK25, it throws the following exception on startup:

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //path/to/my:NameTest
-----------------------------------------------------------------------------
Error occurred during initialization of VM
java.lang.Error: A command line option has attempted to allow or enable the Security Manager. Enabling a Security Manager is not supported.
	at java.lang.System.initPhase3(java.base@25/System.java:1969)

[valodzka]

current workaround is to set -Djava.security.manager=disallow

also you will need to ignore error messages about exit (see #349 (comment))

[evanloriot]

yeah, passing kwarg jvm_flags = ["-Djava.security.manager=disallow"] to the java_test_suite does indeed fix the issue about the SecurityManager.

and yeah, I am getting the following failure

//path/to/my:NameTest                                          FAILED in 1.6s

with logs:

exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //path/to/my:NameTest
-----------------------------------------------------------------------------
Unable to create a mechanism to prevent `System.exit` being called. Tests may cause `bazel test` to exit prematurely.
Failures: 0
Time: 0.328

System.exit or Runtime.exit was called!
        at java.lang.Runtime.exit(Runtime.java:177)
        at java.lang.System.exit(System.java:1540)
        at com.github.bazel_contrib.contrib_rules_jvm.junit5.JUnit5Runner.exit(JUnit5Runner.java:135)
        at com.github.bazel_contrib.contrib_rules_jvm.junit5.JUnit5Runner.main(JUnit5Runner.java:53)

Seems like the fix introduced a different bug? Or is there a way I can disable this behavior / ignore this failure which is reporting back to bazel test?

[evanloriot]

From the looks of bazelbuild/bazel@5fc2b01#diff-9cd638fbed92feb9cd45dc5235ca6c46282157341488ff5206e1396705b03d53 (which im guessing the PR above is based on), the shutdown hook gets removed at the end of the runner on finally which DOESN'T happen here in contrib as far as I can tell.

[evanloriot]

Would something like the following work?

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 55a1d39..29047c0 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 @@ public class JUnit5Runner {
         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 f088b65..367d906 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,19 @@
 package com.github.bazel_contrib.contrib_rules_jvm.junit5;

 public class NullSystemExitToggle implements SystemExitToggle {
+
+  private Thread shutdownHook;
+
   @Override
   public void prevent() {
-    // No-op
+    // Install a shutdown hook so people can track down what went wrong
+    // if a test calls `System.exit`
+    this.shutdownHook = SystemExitDetectingShutdownHook.newShutdownHook(System.err);
+    Runtime.getRuntime().addShutdownHook(shutdownHook);
   }

   @Override
   public void allow() {
-    // No-op
+    Runtime.getRuntime().removeShutdownHook(shutdownHook);
   }
 }

[evanloriot]

Something along these lines perhaps: #391