Merge pull request square#5507 from square/jwilson.0926.okhttpclient_…

…is_lightweight

Create OkHttpClient instances eagerly in tests

okhttp-sse/src/test/java/okhttp3/internal/sse/EventSourceHttpTest.java

@@ -24,7 +24,6 @@
 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,39 +22,44 @@ 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) {
     clientEventsList.add(it)
   }
 
   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.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 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.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
@@ -109,4 +107,4 @@ class OpenJSSETest {
         .build()
     server.useHttps(handshakeCertificates.sslSocketFactory(), false)
   }
-}
+}

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