diff --git a/okhttp-sse/src/test/java/okhttp3/internal/sse/EventSourceHttpTest.java b/okhttp-sse/src/test/java/okhttp3/internal/sse/EventSourceHttpTest.java index be0e5e69d..d96f38e84 100644 --- a/okhttp-sse/src/test/java/okhttp3/internal/sse/EventSourceHttpTest.java +++ b/okhttp-sse/src/test/java/okhttp3/internal/sse/EventSourceHttpTest.java @@ -24,7 +24,6 @@ import okhttp3.mockwebserver.MockWebServer; import okhttp3.sse.EventSource; import okhttp3.sse.EventSources; import org.junit.After; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -35,11 +34,7 @@ public final class EventSourceHttpTest { @Rule public final OkHttpClientTestRule clientTestRule = new OkHttpClientTestRule(); private final EventSourceRecorder listener = new EventSourceRecorder(); - private OkHttpClient client; - - @Before public void setUp() { - client = clientTestRule.newClient(); - } + private OkHttpClient client = clientTestRule.newClient(); @After public void after() { listener.assertExhausted(); diff --git a/okhttp-testing-support/src/main/java/okhttp3/OkHttpClientTestRule.kt b/okhttp-testing-support/src/main/java/okhttp3/OkHttpClientTestRule.kt index 1315914a3..ed0ef26d7 100644 --- a/okhttp-testing-support/src/main/java/okhttp3/OkHttpClientTestRule.kt +++ b/okhttp-testing-support/src/main/java/okhttp3/OkHttpClientTestRule.kt @@ -22,31 +22,36 @@ import org.junit.rules.TestRule import org.junit.runner.Description import org.junit.runners.model.Statement import java.net.InetAddress -import java.util.concurrent.ConcurrentLinkedDeque import java.util.concurrent.TimeUnit /** Apply this rule to tests that need an OkHttpClient instance. */ class OkHttpClientTestRule : TestRule { private val clientEventsList = mutableListOf() - private var prototype: OkHttpClient? = null + private var testClient: OkHttpClient? = null /** - * Returns an OkHttpClient for all tests to use as a starting point. + * Returns an OkHttpClient for tests to use as a starting point. * - * The shared instance allows all tests to share a single connection pool, which prevents idle - * connections from consuming unnecessary resources while connections wait to be evicted. + * The returned client installs a default event listener that gathers debug information. This will + * be logged if the test fails. * * This client is also configured to be slightly more deterministic, returning a single IP * address for all hosts, regardless of the actual number of IP addresses reported by DNS. */ fun newClient(): OkHttpClient { - return newClientBuilder().build() + var client = testClient + if (client == null) { + client = OkHttpClient.Builder() + .dns(SINGLE_INET_ADDRESS_DNS) // Prevent unexpected fallback addresses. + .eventListener(ClientRuleEventListener { addEvent(it) }) + .build() + testClient = client + } + return client } fun newClientBuilder(): OkHttpClient.Builder { - return checkNotNull(prototype) { "don't create clients in test initialization!" } - .newBuilder() - .eventListener(ClientRuleEventListener { addEvent(it) }) + return newClient().newBuilder() } @Synchronized private fun addEvent(it: String) { @@ -54,7 +59,7 @@ class OkHttpClientTestRule : TestRule { } fun ensureAllConnectionsReleased() { - prototype?.let { + testClient?.let { val connectionPool = it.connectionPool connectionPool.evictAll() assertThat(connectionPool.connectionCount()).isEqualTo(0) @@ -72,7 +77,6 @@ class OkHttpClientTestRule : TestRule { override fun apply(base: Statement, description: Description): Statement { return object : Statement() { override fun evaluate() { - acquireClient() try { base.evaluate() logEventsIfFlaky(description) @@ -86,15 +90,8 @@ class OkHttpClientTestRule : TestRule { } } - private fun acquireClient() { - prototype = prototypes.poll() ?: freshClient() - } - private fun releaseClient() { - prototype?.let { - prototypes.push(it) - prototype = null - } + testClient?.dispatcher?.executorService?.shutdown() } } } @@ -119,40 +116,16 @@ class OkHttpClientTestRule : TestRule { } } - /** - * Called if a test is known to be leaky. - */ - fun abandonClient() { - prototype?.let { - prototype = null - it.dispatcher.executorService.shutdownNow() - it.connectionPool.evictAll() - } - } - companion object { - /** - * Quick and dirty pool of OkHttpClient instances. Each has its own independent dispatcher and - * connection pool. This way we can reuse expensive resources while preventing concurrent tests - * from interfering with each other. - */ - internal val prototypes = ConcurrentLinkedDeque() - /** * A network that resolves only one IP address per host. Use this when testing route selection * fallbacks to prevent the host machine's various IP addresses from interfering. */ - internal val SINGLE_INET_ADDRESS_DNS = object : Dns { + private val SINGLE_INET_ADDRESS_DNS = object : Dns { override fun lookup(hostname: String): List { val addresses = Dns.SYSTEM.lookup(hostname) return listOf(addresses[0]) } } - - private fun freshClient(): OkHttpClient { - return OkHttpClient.Builder() - .dns(SINGLE_INET_ADDRESS_DNS) // Prevent unexpected fallback addresses. - .build() - } } } diff --git a/okhttp/src/main/java/okhttp3/internal/cache/DiskLruCache.kt b/okhttp/src/main/java/okhttp3/internal/cache/DiskLruCache.kt index 60086c6fe..b9321de78 100644 --- a/okhttp/src/main/java/okhttp3/internal/cache/DiskLruCache.kt +++ b/okhttp/src/main/java/okhttp3/internal/cache/DiskLruCache.kt @@ -94,7 +94,7 @@ class DiskLruCache internal constructor( maxSize: Long, /** Used for asynchronous journal rebuilds. */ - taskRunner: TaskRunner = TaskRunner.INSTANCE + taskRunner: TaskRunner ) : Closeable, Flushable { /** The maximum number of bytes that this cache should use to store its data. */ @get:Synchronized @set:Synchronized var maxSize: Long = maxSize diff --git a/okhttp/src/test/java/okhttp3/CallKotlinTest.kt b/okhttp/src/test/java/okhttp3/CallKotlinTest.kt index df7e3f2f8..a1c1f970d 100644 --- a/okhttp/src/test/java/okhttp3/CallKotlinTest.kt +++ b/okhttp/src/test/java/okhttp3/CallKotlinTest.kt @@ -19,7 +19,6 @@ import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.testing.PlatformRule import org.assertj.core.api.Assertions.assertThat -import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.rules.TestRule @@ -32,11 +31,7 @@ class CallKotlinTest { @JvmField @Rule val server = MockWebServer() @JvmField @Rule val clientTestRule = OkHttpClientTestRule() - private lateinit var client: OkHttpClient - - @Before fun setUp() { - client = clientTestRule.newClient() - } + private var client = clientTestRule.newClient() @Test fun legalToExecuteTwiceCloning() { diff --git a/okhttp/src/test/java/okhttp3/CallTest.java b/okhttp/src/test/java/okhttp3/CallTest.java index 857bea310..2a50b527b 100644 --- a/okhttp/src/test/java/okhttp3/CallTest.java +++ b/okhttp/src/test/java/okhttp3/CallTest.java @@ -110,7 +110,9 @@ public final class CallTest { private RecordingEventListener listener = new RecordingEventListener(); private HandshakeCertificates handshakeCertificates = localhost(); - private OkHttpClient client; + private OkHttpClient client = clientTestRule.newClientBuilder() + .eventListener(listener) + .build(); private RecordingCallback callback = new RecordingCallback(); private TestLogHandler logHandler = new TestLogHandler(); private Cache cache = new Cache(new File("/cache/"), Integer.MAX_VALUE, fileSystem); @@ -120,9 +122,6 @@ public final class CallTest { platform.assumeNotOpenJSSE(); logger.addHandler(logHandler); - client = clientTestRule.newClientBuilder() - .eventListener(listener) - .build(); } @After public void tearDown() throws Exception { diff --git a/okhttp/src/test/java/okhttp3/ConnectionReuseTest.java b/okhttp/src/test/java/okhttp3/ConnectionReuseTest.java index a3445d781..ea1a635f9 100644 --- a/okhttp/src/test/java/okhttp3/ConnectionReuseTest.java +++ b/okhttp/src/test/java/okhttp3/ConnectionReuseTest.java @@ -24,7 +24,6 @@ import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.SocketPolicy; import okhttp3.testing.PlatformRule; import okhttp3.tls.HandshakeCertificates; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TestRule; @@ -43,11 +42,7 @@ public final class ConnectionReuseTest { @Rule public final OkHttpClientTestRule clientTestRule = new OkHttpClientTestRule(); private HandshakeCertificates handshakeCertificates = localhost(); - private OkHttpClient client; - - @Before public void setUp() { - client = clientTestRule.newClient(); - } + private OkHttpClient client = clientTestRule.newClient(); @Test public void connectionsAreReused() throws Exception { server.enqueue(new MockResponse().setBody("a")); diff --git a/okhttp/src/test/java/okhttp3/ConscryptTest.kt b/okhttp/src/test/java/okhttp3/ConscryptTest.kt index 47381de1d..b2a53ef0b 100644 --- a/okhttp/src/test/java/okhttp3/ConscryptTest.kt +++ b/okhttp/src/test/java/okhttp3/ConscryptTest.kt @@ -35,11 +35,10 @@ class ConscryptTest { @JvmField @Rule val clientTestRule = OkHttpClientTestRule() - private lateinit var client: OkHttpClient + private val client = clientTestRule.newClient() @Before fun setUp() { platform.assumeConscrypt() - client = clientTestRule.newClient() } @Test diff --git a/okhttp/src/test/java/okhttp3/CookiesTest.java b/okhttp/src/test/java/okhttp3/CookiesTest.java index 758a0936c..44ac99fd1 100644 --- a/okhttp/src/test/java/okhttp3/CookiesTest.java +++ b/okhttp/src/test/java/okhttp3/CookiesTest.java @@ -30,7 +30,6 @@ import java.util.Map; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -45,11 +44,7 @@ public class CookiesTest { @Rule public final MockWebServer server = new MockWebServer(); @Rule public final OkHttpClientTestRule clientTestRule = new OkHttpClientTestRule(); - private OkHttpClient client; - - @Before public void setUp() { - client = clientTestRule.newClient(); - } + private OkHttpClient client = clientTestRule.newClient(); @Test public void testNetscapeResponse() throws Exception { diff --git a/okhttp/src/test/java/okhttp3/DispatcherTest.java b/okhttp/src/test/java/okhttp3/DispatcherTest.java index 2a7c9a06b..edcd981c8 100644 --- a/okhttp/src/test/java/okhttp3/DispatcherTest.java +++ b/okhttp/src/test/java/okhttp3/DispatcherTest.java @@ -20,16 +20,15 @@ public final class DispatcherTest { RecordingWebSocketListener webSocketListener = new RecordingWebSocketListener(); Dispatcher dispatcher = new Dispatcher(executor); RecordingEventListener listener = new RecordingEventListener(); - OkHttpClient client; + OkHttpClient client = clientTestRule.newClientBuilder() + .dispatcher(dispatcher) + .eventListener(listener) + .build(); @Before public void setUp() throws Exception { dispatcher.setMaxRequests(20); dispatcher.setMaxRequestsPerHost(10); listener.forbidLock(dispatcher); - client = clientTestRule.newClientBuilder() - .dispatcher(dispatcher) - .eventListener(listener) - .build(); } @Test public void maxRequestsZero() throws Exception { diff --git a/okhttp/src/test/java/okhttp3/DuplexTest.java b/okhttp/src/test/java/okhttp3/DuplexTest.java index 7453035a9..d33b3e8b6 100644 --- a/okhttp/src/test/java/okhttp3/DuplexTest.java +++ b/okhttp/src/test/java/okhttp3/DuplexTest.java @@ -51,14 +51,13 @@ public final class DuplexTest { private RecordingEventListener listener = new RecordingEventListener(); private HandshakeCertificates handshakeCertificates = localhost(); - private OkHttpClient client; + private OkHttpClient client = clientTestRule.newClientBuilder() + .eventListener(listener) + .build(); @Before public void setUp() { platform.assumeNotOpenJSSE(); platform.assumeHttp2Support(); - client = clientTestRule.newClientBuilder() - .eventListener(listener) - .build(); } @Test public void http1DoesntSupportDuplex() throws IOException { diff --git a/okhttp/src/test/java/okhttp3/EventListenerTest.java b/okhttp/src/test/java/okhttp3/EventListenerTest.java index d5f23f603..f5a8fb3ae 100644 --- a/okhttp/src/test/java/okhttp3/EventListenerTest.java +++ b/okhttp/src/test/java/okhttp3/EventListenerTest.java @@ -82,16 +82,14 @@ public final class EventListenerTest { private final RecordingEventListener listener = new RecordingEventListener(); private final HandshakeCertificates handshakeCertificates = localhost(); - private OkHttpClient client; + private OkHttpClient client = clientTestRule.newClientBuilder() + .eventListener(listener) + .build(); private SocksProxy socksProxy; @Before public void setUp() { platform.assumeNotOpenJSSE(); - client = clientTestRule.newClientBuilder() - .eventListener(listener) - .build(); - listener.forbidLock(RealConnectionPool.Companion.get(client.connectionPool())); listener.forbidLock(client.dispatcher()); } diff --git a/okhttp/src/test/java/okhttp3/InterceptorTest.java b/okhttp/src/test/java/okhttp3/InterceptorTest.java index 3f0784d38..b85efb6bd 100644 --- a/okhttp/src/test/java/okhttp3/InterceptorTest.java +++ b/okhttp/src/test/java/okhttp3/InterceptorTest.java @@ -39,7 +39,6 @@ import okio.GzipSink; import okio.Okio; import okio.Sink; import okio.Source; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -50,13 +49,9 @@ public final class InterceptorTest { @Rule public MockWebServer server = new MockWebServer(); @Rule public final OkHttpClientTestRule clientTestRule = new OkHttpClientTestRule(); - private OkHttpClient client; + private OkHttpClient client = clientTestRule.newClient(); private RecordingCallback callback = new RecordingCallback(); - @Before public void setUp() { - client = clientTestRule.newClient(); - } - @Test public void applicationInterceptorsCanShortCircuitResponses() throws Exception { server.shutdown(); // Accept no connections. diff --git a/okhttp/src/test/java/okhttp3/OpenJSSETest.kt b/okhttp/src/test/java/okhttp3/OpenJSSETest.kt index a99243576..8b4ac7392 100644 --- a/okhttp/src/test/java/okhttp3/OpenJSSETest.kt +++ b/okhttp/src/test/java/okhttp3/OpenJSSETest.kt @@ -36,13 +36,11 @@ class OpenJSSETest { @JvmField @Rule var platform = PlatformRule() @JvmField @Rule val clientTestRule = OkHttpClientTestRule() @JvmField @Rule val server = MockWebServer() - lateinit var client: OkHttpClient + var client = clientTestRule.newClient() @Before fun setUp() { platform.assumeOpenJSSE() - - client = clientTestRule.newClient() } @Test @@ -109,4 +107,4 @@ class OpenJSSETest { .build() server.useHttps(handshakeCertificates.sslSocketFactory(), false) } -} \ No newline at end of file +} diff --git a/okhttp/src/test/java/okhttp3/URLConnectionTest.java b/okhttp/src/test/java/okhttp3/URLConnectionTest.java index 656122295..230bf4495 100644 --- a/okhttp/src/test/java/okhttp3/URLConnectionTest.java +++ b/okhttp/src/test/java/okhttp3/URLConnectionTest.java @@ -114,12 +114,11 @@ public final class URLConnectionTest { @Rule public final OkHttpClientTestRule clientTestRule = new OkHttpClientTestRule(); private HandshakeCertificates handshakeCertificates = localhost(); - private OkHttpClient client; + private OkHttpClient client = clientTestRule.newClient(); private @Nullable Cache cache; @Before public void setUp() { server.setProtocolNegotiationEnabled(false); - client = clientTestRule.newClient(); } @After public void tearDown() throws Exception { diff --git a/okhttp/src/test/java/okhttp3/WholeOperationTimeoutTest.java b/okhttp/src/test/java/okhttp3/WholeOperationTimeoutTest.java index bf8b54343..376642db1 100644 --- a/okhttp/src/test/java/okhttp3/WholeOperationTimeoutTest.java +++ b/okhttp/src/test/java/okhttp3/WholeOperationTimeoutTest.java @@ -25,7 +25,6 @@ import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.testing.Flaky; import okio.BufferedSink; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -39,11 +38,7 @@ public final class WholeOperationTimeoutTest { @Rule public final MockWebServer server = new MockWebServer(); @Rule public final OkHttpClientTestRule clientTestRule = new OkHttpClientTestRule(); - private OkHttpClient client; - - @Before public void setUp() { - client = clientTestRule.newClient(); - } + private final OkHttpClient client = clientTestRule.newClient(); @Test public void defaultConfigIsNoTimeout() throws Exception { Request request = new Request.Builder() diff --git a/okhttp/src/test/java/okhttp3/internal/ws/WebSocketHttpTest.java b/okhttp/src/test/java/okhttp3/internal/ws/WebSocketHttpTest.java index 2006f235e..8d7d81634 100644 --- a/okhttp/src/test/java/okhttp3/internal/ws/WebSocketHttpTest.java +++ b/okhttp/src/test/java/okhttp3/internal/ws/WebSocketHttpTest.java @@ -72,28 +72,23 @@ public final class WebSocketHttpTest { private final WebSocketRecorder clientListener = new WebSocketRecorder("client"); private final WebSocketRecorder serverListener = new WebSocketRecorder("server"); private final Random random = new Random(0); - private OkHttpClient client; + private OkHttpClient client = clientTestRule.newClientBuilder() + .writeTimeout(500, TimeUnit.MILLISECONDS) + .readTimeout(500, TimeUnit.MILLISECONDS) + .addInterceptor(chain -> { + Response response = chain.proceed(chain.request()); + // Ensure application interceptors never see a null body. + assertThat(response.body()).isNotNull(); + return response; + }) + .build(); @Before public void setUp() { platform.assumeNotOpenJSSE(); - - client = clientTestRule.newClientBuilder() - .writeTimeout(500, TimeUnit.MILLISECONDS) - .readTimeout(500, TimeUnit.MILLISECONDS) - .addInterceptor(chain -> { - Response response = chain.proceed(chain.request()); - // Ensure application interceptors never see a null body. - assertThat(response.body()).isNotNull(); - return response; - }) - .build(); } @After public void tearDown() { clientListener.assertExhausted(); - - // TODO: assert all connections are released once leaks are fixed - clientTestRule.abandonClient(); } @Test public void textMessage() {