Skip to content

Commit 84848ac

Browse files
authored
[junit5] Issue a warning if System.exit is called during a test (#349)
In very recent JDK versions, the fields we relied on being able to use in the `Java17SystemExitToggle` are no longer present. There's no official way to stop a test from calling `System.exit` without it, but at least we can print a stacktrace so the poor soul trying to debug their tests can figure out what happened. The shutdown hook is lifted directly from Bazel's own JUnit4 runner, and the original copyright header has been retained. Co-authored-by: Simon Mavi Stewart <[email protected]>
1 parent 5ba4ba2 commit 84848ac

File tree

2 files changed

+86
-9
lines changed

2 files changed

+86
-9
lines changed

java/src/com/github/bazel_contrib/contrib_rules_jvm/junit5/JUnit5Runner.java

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ public class JUnit5Runner {
1818
private static final String JAVA17_SYSTEM_EXIT_TOGGLE =
1919
"com.github.bazel_contrib.contrib_rules_jvm.junit5.Java17SystemExitToggle";
2020

21+
private static final Runtime.Version JAVA_12 = Runtime.Version.parse("12");
22+
private static final Runtime.Version JAVA_24 = Runtime.Version.parse("24");
23+
2124
public static void main(String[] args) {
2225
String testSuite = System.getProperty("bazel.test_suite");
2326

@@ -51,15 +54,13 @@ public static void main(String[] args) {
5154
}
5255

5356
private static SystemExitToggle getSystemExitToggle() {
54-
// In Java 8 and lower, the first part of the version is a 1.
55-
// In Java 9 and higher, the first part of the version is the feature version.
56-
// Major versions of early Access builds have an "-ea" suffix.
57-
int featureVersion = Integer.parseInt(System.getProperty("java.version").split("[.-]")[0]);
58-
if (featureVersion == 1) {
59-
featureVersion = 8;
60-
}
57+
Runtime.Version javaVersion = Runtime.version();
6158

62-
if (featureVersion < 12) {
59+
// The `Version.compareTo` javadoc states it returns:
60+
//
61+
// "A negative integer, zero, or a positive integer if this Version is
62+
// less than, equal to, or greater than the given Version"
63+
if (JAVA_12.compareTo(javaVersion) > 0) {
6364
return new Java11SystemExitToggle();
6465
}
6566

@@ -78,7 +79,18 @@ private static SystemExitToggle getSystemExitToggle() {
7879
// this would be a combination of `ReflectiveOperationException` and
7980
// `SecurityException`, but the latter is scheduled for removal so
8081
// relying on it seems like a Bad Idea.
81-
System.err.println("Failed to load Java 17 system exit override: " + e.getMessage());
82+
83+
// In Java 24 the hook we need for the system exit toggle is gone. If
84+
// we're running on a version of Java earlier than that, print a
85+
// warning.
86+
if (!(JAVA_24.compareTo(javaVersion) < 0)) {
87+
System.err.println("Failed to load Java 17 system exit override: " + e.getMessage());
88+
}
89+
90+
// Install a shutdown hook so people can track down what went wrong
91+
// if a test calls `System.exit`
92+
Thread shutdownHook = SystemExitDetectingShutdownHook.newShutdownHook(System.err);
93+
Runtime.getRuntime().addShutdownHook(shutdownHook);
8294

8395
// Fall through
8496
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright 2024 The Bazel Authors. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.package com.google.testing.junit.runner.internal;
14+
15+
// This code has been lifted from:
16+
// https://github.com/bazelbuild/bazel/blob/a74b12a652ba9b28a078e4270bc144973e26b2a1/src/java_tools/junitrunner/java/com/google/testing/junit/runner/internal/SystemExitDetectingShutdownHook.java#L27
17+
18+
package com.github.bazel_contrib.contrib_rules_jvm.junit5;
19+
20+
import java.io.PrintStream;
21+
import java.util.ArrayList;
22+
import java.util.List;
23+
24+
public class SystemExitDetectingShutdownHook {
25+
26+
private SystemExitDetectingShutdownHook() {}
27+
28+
public static Thread newShutdownHook(PrintStream testRunnerOut) {
29+
Runnable hook =
30+
() -> {
31+
boolean foundRuntimeExit = false;
32+
for (StackTraceElement[] stack : Thread.getAllStackTraces().values()) {
33+
List<String> framesStartingWithRuntimeExit = new ArrayList<>();
34+
boolean foundRuntimeExitInThisThread = false;
35+
for (StackTraceElement frame : stack) {
36+
if (!foundRuntimeExitInThisThread
37+
&& frame.getClassName().equals("java.lang.Runtime")
38+
&& frame.getMethodName().equals("exit")) {
39+
foundRuntimeExitInThisThread = true;
40+
}
41+
if (foundRuntimeExitInThisThread) {
42+
framesStartingWithRuntimeExit.add(frameString(frame));
43+
}
44+
}
45+
if (foundRuntimeExitInThisThread) {
46+
foundRuntimeExit = true;
47+
testRunnerOut.println("\nSystem.exit or Runtime.exit was called!");
48+
testRunnerOut.println(String.join("\n", framesStartingWithRuntimeExit));
49+
}
50+
}
51+
if (foundRuntimeExit) {
52+
// We must call halt rather than exit, because exit would lead to a deadlock. We use a
53+
// hopefully unique exit code to make it easier to identify this case.
54+
Runtime.getRuntime().halt(121);
55+
}
56+
};
57+
return new Thread(hook, "SystemExitDetectingShutdownHook");
58+
}
59+
60+
private static String frameString(StackTraceElement frame) {
61+
return String.format(
62+
" at %s.%s(%s:%d)",
63+
frame.getClassName(), frame.getMethodName(), frame.getFileName(), frame.getLineNumber());
64+
}
65+
}

0 commit comments

Comments
 (0)