From 96e596117e25d108bf9b96ad2899d3610f9915a0 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Fri, 20 Dec 2019 08:45:37 -1000 Subject: [PATCH] Use our test rule instead of the Maven Surefire listener Closes: https://github.com/square/okhttp/issues/4894 --- build.gradle | 1 - .../main/java/okhttp3/OkHttpClientTestRule.kt | 33 ++++++-- ...stallUncaughtExceptionHandlerListener.java | 75 ------------------- .../okhttp3/OkHttpClientTestRuleTest.kt | 49 ++++++++++++ 4 files changed, 76 insertions(+), 82 deletions(-) delete mode 100644 okhttp-testing-support/src/main/java/okhttp3/testing/InstallUncaughtExceptionHandlerListener.java create mode 100644 okhttp-testing-support/src/test/kotlin/okhttp3/OkHttpClientTestRuleTest.kt diff --git a/build.gradle b/build.gradle index 27f6e01af..4666e2e4a 100644 --- a/build.gradle +++ b/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) { diff --git a/okhttp-testing-support/src/main/java/okhttp3/OkHttpClientTestRule.kt b/okhttp-testing-support/src/main/java/okhttp3/OkHttpClientTestRule.kt index 6d8489738..0a5d2f52f 100644 --- a/okhttp-testing-support/src/main/java/okhttp3/OkHttpClientTestRule.kt +++ b/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() 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) { - clientEventsList.add(it) + @Synchronized private fun addEvent(event: String) { + 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() diff --git a/okhttp-testing-support/src/main/java/okhttp3/testing/InstallUncaughtExceptionHandlerListener.java b/okhttp-testing-support/src/main/java/okhttp3/testing/InstallUncaughtExceptionHandlerListener.java deleted file mode 100644 index c1768dcd4..000000000 --- a/okhttp-testing-support/src/main/java/okhttp3/testing/InstallUncaughtExceptionHandlerListener.java +++ /dev/null @@ -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 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()); - } - } - } -} diff --git a/okhttp-testing-support/src/test/kotlin/okhttp3/OkHttpClientTestRuleTest.kt b/okhttp-testing-support/src/test/kotlin/okhttp3/OkHttpClientTestRuleTest.kt new file mode 100644 index 000000000..52faa71b6 --- /dev/null +++ b/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!") + } + } +}