Create OkHttpClient instances eagerly in tests

We used to do this lazily because OkHttpClient instances were
somewhat heavy: each standalone instance held its own
ExecutorService for the connection pool.

Now that we have TaskRunner each instance is much more
lightweight and the drawbacks of creating instances eagerly
is negligible.

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();

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<String>()
-  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<OkHttpClient>()
-
     /**
      * 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<InetAddress> {
         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()
-    }
   }
 }

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

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() {

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 {

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"));

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

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 {

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 {

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 {

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());
   }

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.
 

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

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 {

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()

okhttp/src/test/java/okhttp3/internal/ws/WebSocketHttpTest.java

@@ -72,12 +72,7 @@ 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;
-
-  @Before public void setUp() {
-    platform.assumeNotOpenJSSE();
-
-    client = clientTestRule.newClientBuilder()
+  private OkHttpClient client = clientTestRule.newClientBuilder()
       .writeTimeout(500, TimeUnit.MILLISECONDS)
       .readTimeout(500, TimeUnit.MILLISECONDS)
       .addInterceptor(chain -> {
@@ -87,13 +82,13 @@ public final class WebSocketHttpTest {
         return response;
       })
       .build();
+
+  @Before public void setUp() {
+    platform.assumeNotOpenJSSE();
   }
 
   @After public void tearDown() {
     clientListener.assertExhausted();
-
-    // TODO: assert all connections are released once leaks are fixed
-    clientTestRule.abandonClient();
   }
 
   @Test public void textMessage() {