Execute CallTest with JUnit 5 (square#6349)

* Execute CallTest with JUnit 5
* Avoid Optional.isEmpty which is JDK11+

build.gradle

@@ -41,7 +41,9 @@ buildscript {
       'jsr305': "com.google.code.findbugs:jsr305:${versions.findbugs}",
       'junit': "junit:junit:${versions.junit}",
       'junit5Api': "org.junit.jupiter:junit-jupiter-api:${versions.junit5}",
-      'junit5Jupiter': "org.junit.jupiter:junit-jupiter-engine:${versions.junit5}",
+      'junit5JupiterEngine': "org.junit.jupiter:junit-jupiter-engine:${versions.junit5}",
+      'junit5VintageEngine': "org.junit.vintage:junit-vintage-engine:${versions.junit5}",
+      'junitPlatformConsole': "org.junit.platform:junit-platform-console:1.7.0",
       'kotlinStdlib': "org.jetbrains.kotlin:kotlin-stdlib:${versions.kotlin}",
       'moshi': "com.squareup.moshi:moshi:${versions.moshi}",
       'moshiKotlin': "com.squareup.moshi:moshi-kotlin-codegen:${versions.moshi}",
@@ -201,7 +203,13 @@ subprojects { project ->
   def platform = System.getProperty("okhttp.platform", "jdk9")
   def testJavaVersion = Integer.getInteger("test.java.version", 11)
 
+  dependencies {
+    testRuntimeOnly(deps.junit5JupiterEngine)
+    testRuntimeOnly(deps.junit5VintageEngine)
+  }
+
   test {
+    useJUnitPlatform()
     jvmArgs += "-Dokhttp.platform=$platform"
 
     javaLauncher = javaToolchains.launcherFor {
@@ -214,6 +222,7 @@ subprojects { project ->
     }
 
     systemProperty 'okhttp.platform', platform
+    systemProperty 'junit.jupiter.extensions.autodetection.enabled', 'true'
   }
 
   if (platform == "jdk8alpn") {

mockwebserver-junit5/build.gradle

@@ -14,7 +14,7 @@ dependencies {
   api deps.junit5Api
   compileOnly deps.animalSniffer
 
-  testRuntimeOnly deps.junit5Jupiter
+  testRuntimeOnly deps.junit5JupiterEngine
   testImplementation deps.assertj
   testImplementation("org.jetbrains.kotlin:kotlin-test-junit5:1.4.10")
 }

mockwebserver-junit5/src/main/kotlin/mockwebserver3/junit5/internal/MockWebServerExtension.kt

@@ -15,23 +15,34 @@
  */
 package mockwebserver3.junit5.internal
 
+import java.io.IOException
+import java.util.logging.Level
+import java.util.logging.Logger
 import mockwebserver3.MockWebServer
 import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement
+import org.junit.jupiter.api.extension.AfterAllCallback
+import org.junit.jupiter.api.extension.AfterEachCallback
+import org.junit.jupiter.api.extension.BeforeAllCallback
 import org.junit.jupiter.api.extension.BeforeEachCallback
 import org.junit.jupiter.api.extension.ExtensionContext
 import org.junit.jupiter.api.extension.ParameterContext
 import org.junit.jupiter.api.extension.ParameterResolver
-import java.io.IOException
-import java.util.logging.Level
-import java.util.logging.Logger
 
 /** Runs MockWebServer for the duration of a single test method. */
-class MockWebServerExtension: BeforeEachCallback, ParameterResolver {
+class MockWebServerExtension
+  : BeforeAllCallback, BeforeEachCallback, AfterAllCallback, AfterEachCallback, ParameterResolver {
   private val ExtensionContext.resource: Resource
-    get() = getStore(ExtensionContext.Namespace.GLOBAL)
-      .getOrComputeIfAbsent(Resource::class.java)
+    get() {
+      val store = getStore(namespace)
+      var result = store.get(uniqueId) as Resource?
+      if (result == null) {
+        result = Resource()
+        store.put(uniqueId, result)
+      }
+      return result
+    }
 
-  private class Resource : ExtensionContext.Store.CloseableResource {
+  private class Resource {
     private val servers = mutableListOf<MockWebServer>()
     private var started = false
 
@@ -50,7 +61,7 @@ class MockWebServerExtension: BeforeEachCallback, ParameterResolver {
       }
     }
 
-    override fun close() {
+    fun shutdownAll() {
       try {
         for (server in servers) {
           server.shutdown()
@@ -72,11 +83,26 @@ class MockWebServerExtension: BeforeEachCallback, ParameterResolver {
     extensionContext: ExtensionContext
   ): Any = extensionContext.resource.newServer()
 
-  override fun beforeEach(extensionContext: ExtensionContext) {
-    extensionContext.resource.startAll()
+  /** Start the servers passed in as test class constructor parameters. */
+  override fun beforeAll(context: ExtensionContext) {
+    context.resource.startAll()
+  }
+
+  /** Start the servers passed in as test method parameters. */
+  override fun beforeEach(context: ExtensionContext) {
+    context.resource.startAll()
+  }
+
+  override fun afterEach(context: ExtensionContext) {
+    context.resource.shutdownAll()
+  }
+
+  override fun afterAll(context: ExtensionContext) {
+    context.resource.shutdownAll()
   }
 
   companion object {
     private val logger = Logger.getLogger(MockWebServerExtension::class.java.name)
+    private val namespace = ExtensionContext.Namespace.create(MockWebServerExtension::class.java)
   }
 }

native-image-tests/build.gradle

@@ -4,9 +4,9 @@ plugins {
 
 dependencies {
   implementation deps.assertj
-  implementation "org.junit.jupiter:junit-jupiter-api:5.7.0"
-  implementation "org.junit.jupiter:junit-jupiter-engine:5.7.0"
-  implementation "org.junit.platform:junit-platform-console:1.7.0"
+  implementation deps.junit5Api
+  implementation deps.junit5JupiterEngine
+  implementation deps.junitPlatformConsole
 
   compileOnly deps.jsr305
 }

okhttp-testing-support/build.gradle

@@ -1,6 +1,7 @@
 dependencies {
   api project(':okhttp')
   api deps.junit
+  api deps.junit5Api
   api deps.assertj
   api deps.bouncycastle
   implementation deps.bouncycastlepkix

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

@@ -27,6 +27,9 @@ import okhttp3.internal.http2.Http2
 import okhttp3.testing.Flaky
 import org.junit.Assert.assertEquals
 import org.junit.Assert.fail
+import org.junit.jupiter.api.extension.AfterEachCallback
+import org.junit.jupiter.api.extension.BeforeEachCallback
+import org.junit.jupiter.api.extension.ExtensionContext
 import org.junit.rules.TestRule
 import org.junit.runner.Description
 import org.junit.runners.model.Statement
@@ -38,12 +41,14 @@ import org.junit.runners.model.Statement
  * Use [newClient] as a factory for a OkHttpClient instances. These instances are specifically
  * configured for testing.
  */
-class OkHttpClientTestRule : TestRule {
+class OkHttpClientTestRule : TestRule, BeforeEachCallback, AfterEachCallback {
   private val clientEventsList = mutableListOf<String>()
   private var testClient: OkHttpClient? = null
   private var uncaughtException: Throwable? = null
-  var logger: Logger? = null
-  lateinit var testName: String
+  private var logger: Logger? = null
+  private lateinit var testName: String
+  private var defaultUncaughtExceptionHandler: Thread.UncaughtExceptionHandler? = null
+  private var taskQueuesWereIdle: Boolean = false
 
   var recordEvents = true
   var recordTaskRunner = false
@@ -171,86 +176,99 @@ class OkHttpClientTestRule : TestRule {
     }
   }
 
+  override fun beforeEach(context: ExtensionContext) {
+    testName = context.displayName
+
+    beforeEach()
+  }
+
+  private fun beforeEach() {
+    defaultUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler()
+    Thread.setDefaultUncaughtExceptionHandler { _, throwable ->
+      initUncaughtException(throwable)
+    }
+
+    taskQueuesWereIdle = TaskRunner.INSTANCE.activeQueues().isEmpty()
+
+    applyLogger {
+      addHandler(testLogHandler)
+      level = Level.FINEST
+      useParentHandlers = false
+    }
+  }
+
+  override fun afterEach(context: ExtensionContext) {
+    val failure = context.executionException.orElseGet { null }
+    val newFailure = afterEach(context.isFlaky(), failure)
+    if (failure == null && newFailure != null) throw newFailure
+  }
+
+  /** Returns the exception to fail with. */
+  private fun afterEach(isFlaky: Boolean, failure: Throwable?): Throwable? {
+    if (uncaughtException != null) {
+      return failure + AssertionError("uncaught exception thrown during test", uncaughtException)
+    }
+
+    if (isFlaky) {
+      logEvents()
+    }
+
+    LogManager.getLogManager().reset()
+
+    var result: Throwable? = failure
+    Thread.setDefaultUncaughtExceptionHandler(defaultUncaughtExceptionHandler)
+    try {
+      ensureAllConnectionsReleased()
+      releaseClient()
+    } catch (ae: AssertionError) {
+      result += ae
+    }
+
+    try {
+      if (taskQueuesWereIdle) {
+        ensureAllTaskQueuesIdle()
+      }
+    } catch (ae: AssertionError) {
+      result += ae
+    }
+
+    return result
+  }
+
+  private fun releaseClient() {
+    testClient?.dispatcher?.executorService?.shutdown()
+  }
+
   override fun apply(
     base: Statement,
     description: Description
   ): Statement {
     return object : Statement() {
       override fun evaluate() {
         testName = description.methodName
+        beforeEach()
 
-        val defaultUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler()
-        Thread.setDefaultUncaughtExceptionHandler { _, throwable ->
-          initUncaughtException(throwable)
-        }
-        val taskQueuesWereIdle = TaskRunner.INSTANCE.activeQueues().isEmpty()
         var failure: Throwable? = null
         try {
-          applyLogger {
-            addHandler(testLogHandler)
-            level = Level.FINEST
-            useParentHandlers = false
-          }
-
           base.evaluate()
-          if (uncaughtException != null) {
-            throw AssertionError("uncaught exception thrown during test", uncaughtException)
-          }
-          logEventsIfFlaky(description)
         } catch (t: Throwable) {
           failure = t
           logEvents()
-          throw t
-        } finally {
-          LogManager.getLogManager().reset()
-
-          Thread.setDefaultUncaughtExceptionHandler(defaultUncaughtExceptionHandler)
-          try {
-            ensureAllConnectionsReleased()
-            releaseClient()
-          } catch (ae: AssertionError) {
-            // Prefer keeping the inflight failure, but don't release this in-use client.
-            if (failure != null) {
-              failure.addSuppressed(ae)
-            } else {
-              failure = ae
-            }
-          }
-
-          try {
-            if (taskQueuesWereIdle) {
-              ensureAllTaskQueuesIdle()
-            }
-          } catch (ae: AssertionError) {
-            // Prefer keeping the inflight failure, but don't release this in-use client.
-            if (failure != null) {
-              failure.addSuppressed(ae)
-            } else {
-              failure = ae
-            }
-          }
-
-          if (failure != null) {
-            throw failure
-          }
         }
-      }
 
-      private fun releaseClient() {
-        testClient?.dispatcher?.executorService?.shutdown()
+        val newFailure = afterEach(description.isFlaky(), failure)
+        if (newFailure != null) throw newFailure
       }
     }
   }
 
-  private fun logEventsIfFlaky(description: Description) {
-    if (isTestFlaky(description)) {
-      logEvents()
-    }
-  }
+  private fun Description.isFlaky() =
+    annotations.any { it.annotationClass == Flaky::class } ||
+      testClass.annotations.any { it.annotationClass == Flaky::class }
 
-  private fun isTestFlaky(description: Description): Boolean {
-    return description.annotations.any { it.annotationClass == Flaky::class } ||
-        description.testClass.annotations.any { it.annotationClass == Flaky::class }
+  private fun ExtensionContext.isFlaky(): Boolean {
+    return (testMethod.orElseGet { null }?.isAnnotationPresent(Flaky::class.java) == true) ||
+      (testClass.orElseGet { null }?.isAnnotationPresent(Flaky::class.java) == true)
   }
 
   @Synchronized private fun logEvents() {
@@ -275,5 +293,13 @@ class OkHttpClientTestRule : TestRule {
         return listOf(addresses[0])
       }
     }
+
+    private operator fun Throwable?.plus(throwable: Throwable): Throwable {
+      if (this != null) {
+        addSuppressed(throwable)
+        return this
+      }
+      return throwable
+    }
   }
 }

okhttp-testing-support/src/main/kotlin/okhttp3/internal/io/InMemoryFileSystem.kt

@@ -25,16 +25,22 @@ import okio.ForwardingSink
 import okio.ForwardingSource
 import okio.Sink
 import okio.Source
+import org.junit.jupiter.api.extension.AfterEachCallback
+import org.junit.jupiter.api.extension.ExtensionContext
 import org.junit.rules.TestRule
 import org.junit.runner.Description
 import org.junit.runners.model.Statement
 
 /** A simple file system where all files are held in memory. Not safe for concurrent use.  */
-class InMemoryFileSystem : FileSystem, TestRule {
+class InMemoryFileSystem : FileSystem, TestRule, AfterEachCallback {
   val files = mutableMapOf<File, Buffer>()
   private val openSources = IdentityHashMap<Source, File>()
   private val openSinks = IdentityHashMap<Sink, File>()
 
+  override fun afterEach(context: ExtensionContext?) {
+    ensureResourcesClosed()
+  }
+
   override fun apply(base: Statement, description: Description): Statement {
     return object : Statement() {
       override fun evaluate() {