Use our test rule instead of the Maven Surefire listener

Closes: https://github.com/square/okhttp/issues/4894

build.gradle

@@ -137,7 +137,6 @@ subprojects { project ->
   def platformJavaHome = System.getProperty('test.java.home')
 
   test {
-    jvmArgs += "-Dlistener=okhttp3.testing.InstallUncaughtExceptionHandlerListener"
     jvmArgs += "-Dokhttp.platform=$platform"
 
     if (platformJavaHome != null) {

okhttp-testing-support/src/main/java/okhttp3/OkHttpClientTestRule.kt

@@ -24,13 +24,20 @@ import org.junit.runners.model.Statement
 import java.net.InetAddress
 import java.util.concurrent.TimeUnit
 
-/** Apply this rule to tests that need an OkHttpClient instance. */
+/**
+ * Apply this rule to all tests. It adds additional checks for leaked resources and uncaught
+ * exceptions.
+ *
+ * Use [newClient] as a factory for a OkHttpClient instances. These instances are specifically
+ * configured for testing.
+ */
 class OkHttpClientTestRule : TestRule {
   private val clientEventsList = mutableListOf<String>()
   private var testClient: OkHttpClient? = null
+  private var uncaughtException: Throwable? = null
 
   fun wrap(eventListener: EventListener) = object : EventListener.Factory {
-    override fun create(call: Call): EventListener = ClientRuleEventListener(eventListener) { addEvent(it) }
+    override fun create(call: Call) = ClientRuleEventListener(eventListener) { addEvent(it) }
   }
 
   /**
@@ -48,7 +55,7 @@ class OkHttpClientTestRule : TestRule {
       client = OkHttpClient.Builder()
           .dns(SINGLE_INET_ADDRESS_DNS) // Prevent unexpected fallback addresses.
           .eventListenerFactory(object : EventListener.Factory {
-            override fun create(call: Call): EventListener = ClientRuleEventListener { addEvent(it) }
+            override fun create(call: Call) = ClientRuleEventListener { addEvent(it) }
           })
           .build()
       testClient = client
@@ -60,8 +67,14 @@ class OkHttpClientTestRule : TestRule {
     return newClient().newBuilder()
   }
 
-  @Synchronized private fun addEvent(it: String) {
+  @Synchronized private fun addEvent(event: String) {
-    clientEventsList.add(it)
+    clientEventsList.add(event)
+  }
+
+  @Synchronized private fun initUncaughtException(throwable: Throwable) {
+    if (uncaughtException == null) {
+      uncaughtException = throwable
+    }
   }
 
   fun ensureAllConnectionsReleased() {
@@ -75,7 +88,7 @@ class OkHttpClientTestRule : TestRule {
   private fun ensureAllTaskQueuesIdle() {
     for (queue in TaskRunner.INSTANCE.activeQueues()) {
       assertThat(queue.awaitIdle(TimeUnit.MILLISECONDS.toNanos(1000L)))
-          .withFailMessage("Queue still active after 1000ms")
+          .withFailMessage("Queue still active after 1000 ms")
           .isTrue()
     }
   }
@@ -83,13 +96,21 @@ class OkHttpClientTestRule : TestRule {
   override fun apply(base: Statement, description: Description): Statement {
     return object : Statement() {
       override fun evaluate() {
+        val defaultUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler()
+        Thread.setDefaultUncaughtExceptionHandler { _, throwable ->
+          initUncaughtException(throwable)
+        }
         try {
           base.evaluate()
+          if (uncaughtException != null) {
+            throw AssertionError("uncaught exception thrown during test", uncaughtException)
+          }
           logEventsIfFlaky(description)
         } catch (t: Throwable) {
           logEvents()
           throw t
         } finally {
+          Thread.setDefaultUncaughtExceptionHandler(defaultUncaughtExceptionHandler)
           ensureAllConnectionsReleased()
           releaseClient()
           ensureAllTaskQueuesIdle()

okhttp-testing-support/src/main/java/okhttp3/testing/InstallUncaughtExceptionHandlerListener.java

@@ -1,75 +0,0 @@
-/*
- * Copyright (C) 2015 Square, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package okhttp3.testing;
-
-import java.io.PrintWriter;
-import java.io.StringWriter;
-import java.util.LinkedHashMap;
-import java.util.Map;
-import org.junit.internal.Throwables;
-import org.junit.runner.Description;
-import org.junit.runner.Result;
-import org.junit.runner.notification.RunListener;
-
-/**
- * A {@link org.junit.runner.notification.RunListener} used to install an aggressive default {@link
- * java.lang.Thread.UncaughtExceptionHandler} similar to the one found on Android. No exceptions
- * should escape from OkHttp that might cause apps to be killed or tests to fail on Android.
- */
-public class InstallUncaughtExceptionHandlerListener extends RunListener {
-
-  private Thread.UncaughtExceptionHandler oldDefaultUncaughtExceptionHandler;
-  private Description lastTestStarted;
-  private final Map<Throwable, String> exceptions = new LinkedHashMap<>();
-
-  @Override public void testRunStarted(Description description) {
-    System.err.println("Installing aggressive uncaught exception handler");
-    oldDefaultUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler();
-    Thread.setDefaultUncaughtExceptionHandler((thread, throwable) -> {
-      StringWriter errorText = new StringWriter(256);
-      errorText.append("Uncaught exception in OkHttp thread \"");
-      errorText.append(thread.getName());
-      errorText.append("\"\n");
-      throwable.printStackTrace(new PrintWriter(errorText));
-      errorText.append("\n");
-      if (lastTestStarted != null) {
-        errorText.append("Last test to start was: ");
-        errorText.append(lastTestStarted.getDisplayName());
-        errorText.append("\n");
-      }
-      System.err.print(errorText.toString());
-
-      synchronized (exceptions) {
-        exceptions.put(throwable, lastTestStarted.getDisplayName());
-      }
-    });
-  }
-
-  @Override public void testStarted(Description description) {
-    lastTestStarted = description;
-  }
-
-  @Override public void testRunFinished(Result result) throws Exception {
-    Thread.setDefaultUncaughtExceptionHandler(oldDefaultUncaughtExceptionHandler);
-    System.err.println("Uninstalled aggressive uncaught exception handler");
-
-    synchronized (exceptions) {
-      if (!exceptions.isEmpty()) {
-        throw Throwables.rethrowAsException(exceptions.keySet().iterator().next());
-      }
-    }
-  }
-}

okhttp-testing-support/src/test/kotlin/okhttp3/OkHttpClientTestRuleTest.kt

@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2019 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package okhttp3
+
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.Assert.fail
+import org.junit.Test
+import org.junit.runner.Description
+import org.junit.runners.model.Statement
+
+class OkHttpClientTestRuleTest {
+  @Test fun uncaughtException() {
+    val testRule = OkHttpClientTestRule()
+    val description = Description.createTestDescription(
+        OkHttpClientTestRuleTest::class.java, "test")
+    val statement = testRule.apply(object : Statement() {
+      override fun evaluate() {
+        val thread = object : Thread() {
+          override fun run() {
+            throw RuntimeException("boom!")
+          }
+        }
+        thread.start()
+        thread.join()
+      }
+    }, description)
+
+    try {
+      statement.evaluate()
+      fail()
+    } catch (expected: AssertionError) {
+      assertThat(expected).hasMessage("uncaught exception thrown during test")
+      assertThat(expected.cause).hasMessage("boom!")
+    }
+  }
+}