Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow constructor injection of MockWebServer #8191

Merged
merged 6 commits into from
Jan 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion android-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ android {
testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
testInstrumentationRunnerArguments += mapOf(
"runnerBuilder" to "de.mannodermaus.junit5.AndroidJUnit5Builder",
"notPackage" to "org.bouncycastle"
"notPackage" to "org.bouncycastle",
"configurationParameters" to "junit.jupiter.extensions.autodetection.enabled=true"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do our end-users need to do this? JUnit 5 is such a mess.

Copy link

@TWiStErRob TWiStErRob Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't "need to", users have the option of

  • using the extension directly by annotating test class.
  • using the extension directly by annotating an annotation to collect extensions
  • setting this property to magically apply extension to all tests

They must consume extensions somehow for sure, but this is not much different than adding a rule to a class in JUnit 4, right?

Copy link

@TWiStErRob TWiStErRob Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, just realized that this is in internal package, but it's public API, as users can choose how to use the extensions (you choose to use the atuodetection in this project). Could this be made actual API? @swankjesse

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change our MockWebServer JUnit 5 extension API to use an annotation on each annotated class. It’s two imports instead of JUnit 4’s single import, but it’s self-contained in the test class.

)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import javax.net.ssl.TrustManagerFactory
import javax.net.ssl.X509TrustManager
import mockwebserver3.MockResponse
import mockwebserver3.MockWebServer
import mockwebserver3.junit5.internal.MockWebServerExtension
import okhttp3.Cache
import okhttp3.Call
import okhttp3.CertificatePinner
Expand Down Expand Up @@ -83,14 +82,13 @@ import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.RegisterExtension
import org.opentest4j.TestAbortedException

/**
* Run with "./gradlew :android-test:connectedCheck -PandroidBuild=true" and make sure ANDROID_SDK_ROOT is set.
*/
@ExtendWith(MockWebServerExtension::class)

@Tag("Slow")
class OkHttpTest {
@Suppress("RedundantVisibilityModifier")
Expand Down
15 changes: 15 additions & 0 deletions mockwebserver-junit5/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,21 @@ class MyTest(
}
```

Multiple instances can be obtained by naming additional ones:

```
class MyTest(
private val server: MockWebServer,
@MockWebServerInstance("server2") private val server2: MockWebServer,
@MockWebServerInstance("server3") private val server3: MockWebServer
) {
@Test
fun test() {
...
}
}
```

Requirements
------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@
package mockwebserver3.junit5.internal

import java.io.IOException
import java.lang.reflect.Method
import java.util.logging.Level
import java.util.logging.Logger
import kotlin.jvm.optionals.getOrNull
import mockwebserver3.MockWebServer
import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement
import org.junit.jupiter.api.extension.AfterEachCallback
Expand All @@ -41,10 +43,25 @@ import org.junit.jupiter.api.extension.ParameterResolver
class MockWebServerExtension :
BeforeEachCallback, AfterEachCallback, ParameterResolver {
private val ExtensionContext.resource: ServersForTest
get() =
getStore(namespace).getOrComputeIfAbsent(this.uniqueId) {
get() {
// For Methods (before/after/test) grab the class store
val store =
if (this.element.getOrNull() is Method) {
val parent = parent.get()
parent.getStore(namespace)
} else {
getStore(namespace)
}

return store.serversForTest
}

private val ExtensionContext.Store.serversForTest: ServersForTest
get() {
return getOrComputeIfAbsent(storeKey) {
ServersForTest()
} as ServersForTest
}

private class ServersForTest {
private val servers = mutableMapOf<String, MockWebServer>()
Expand All @@ -67,10 +84,11 @@ class MockWebServerExtension :

fun shutdownAll() {
try {
for (server in servers.values) {
val toClear = servers.values.toList()
servers.clear()
for (server in toClear) {
server.shutdown()
}
servers.clear()
} catch (e: IOException) {
logger.log(Level.WARNING, "MockWebServer shutdown failed", e)
}
Expand All @@ -83,9 +101,7 @@ class MockWebServerExtension :
parameterContext: ParameterContext,
extensionContext: ExtensionContext,
): Boolean {
// Not supported on constructors, or static contexts
return parameterContext.parameter.type === MockWebServer::class.java &&
extensionContext.testMethod.isPresent
return parameterContext.parameter.type === MockWebServer::class.java
}

@Suppress("NewApi")
Expand All @@ -112,9 +128,10 @@ class MockWebServerExtension :
context.resource.shutdownAll()
}

companion object {
private companion object {
private val logger = Logger.getLogger(MockWebServerExtension::class.java.name)
private val namespace = ExtensionContext.Namespace.create(MockWebServerExtension::class.java)
private val defaultName = MockWebServerExtension::class.java.simpleName
private val storeKey = "ServersForTest"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ package mockwebserver3.junit5.internal

import assertk.assertThat
import assertk.assertions.isEqualTo
import assertk.assertions.isSameAs
import assertk.assertions.isSameInstanceAs
import assertk.assertions.isTrue
import mockwebserver3.MockResponse
import mockwebserver3.MockWebServer
Expand All @@ -26,33 +26,38 @@ import okhttp3.Request
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.RegisterExtension

@ExtendWith(MockWebServerExtension::class)
class ExtensionLifecycleTest {
class ExtensionLifecycleTest(private val constructorServer: MockWebServer) {
@RegisterExtension
val clientTestRule: OkHttpClientTestRule = OkHttpClientTestRule()

lateinit var setUpServer: MockWebServer

@BeforeEach
fun setup(server: MockWebServer) {
this.setUpServer = server
assertThat(constructorServer).isSameInstanceAs(server)
assertThat(server.started).isTrue()
server.enqueue(MockResponse())
}

@AfterEach
fun tearDown(server: MockWebServer) {
assertThat(setUpServer).isSameAs(server)
assertThat(constructorServer).isSameInstanceAs(server)
assertThat(server.started).isTrue()
server.enqueue(MockResponse())
}

@Test
fun testClient(server: MockWebServer) {
assertThat(setUpServer).isSameAs(server)
assertThat(constructorServer).isSameInstanceAs(server)
assertThat(server.started).isTrue()
clientTestRule.newClient().newCall(Request(server.url("/"))).execute().use {
assertThat(it.code).isEqualTo(200)
}
}

@Test
fun testClient2(server: MockWebServer) {
assertThat(constructorServer).isSameInstanceAs(server)
assertThat(server.started).isTrue()
clientTestRule.newClient().newCall(Request(server.url("/"))).execute().use {
assertThat(it.code).isEqualTo(200)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ import mockwebserver3.MockWebServer
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith

@ExtendWith(MockWebServerExtension::class)
class ExtensionMultipleInstancesTest {
var defaultInstancePort: Int = -1
var instanceAPort: Int = -1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ import assertk.assertThat
import assertk.assertions.isTrue
import mockwebserver3.MockWebServer
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith

@ExtendWith(MockWebServerExtension::class)
class ExtensionMultipleTestsTest() {
class ExtensionMultipleTestsTest {
@Test
fun testClient1(
defaultInstance: MockWebServer,
Expand Down
3 changes: 0 additions & 3 deletions okhttp-coroutines/src/test/kotlin/okhttp3/SuspendCallTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,11 @@ import kotlinx.coroutines.withTimeout
import mockwebserver3.MockResponse
import mockwebserver3.MockWebServer
import mockwebserver3.SocketPolicy.DisconnectAfterRequest
import mockwebserver3.junit5.internal.MockWebServerExtension
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.RegisterExtension
import org.junit.jupiter.api.fail

@ExtendWith(MockWebServerExtension::class)
class SuspendCallTest {
@RegisterExtension
val clientTestRule = OkHttpClientTestRule()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import java.net.UnknownHostException
import java.util.concurrent.TimeUnit
import mockwebserver3.MockResponse
import mockwebserver3.MockWebServer
import mockwebserver3.junit5.internal.MockWebServerExtension
import okhttp3.Cache
import okhttp3.Dns
import okhttp3.OkHttpClient
Expand All @@ -44,10 +43,8 @@ import org.junit.jupiter.api.Assertions.fail
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.RegisterExtension

@ExtendWith(MockWebServerExtension::class)
@Tag("Slowish")
class DnsOverHttpsTest {
@RegisterExtension
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import assertk.assertions.matches
import java.net.UnknownHostException
import mockwebserver3.MockResponse
import mockwebserver3.MockWebServer
import mockwebserver3.junit5.internal.MockWebServerExtension
import okhttp3.HttpUrl
import okhttp3.Interceptor
import okhttp3.MediaType
Expand All @@ -45,10 +44,8 @@ import org.junit.jupiter.api.Assertions.fail
import org.junit.jupiter.api.Assumptions
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.RegisterExtension

@ExtendWith(MockWebServerExtension::class)
class HttpLoggingInterceptorTest {
@RegisterExtension
val platform = PlatformRule()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import java.util.Arrays
import mockwebserver3.MockResponse
import mockwebserver3.MockWebServer
import mockwebserver3.SocketPolicy.FailHandshake
import mockwebserver3.junit5.internal.MockWebServerExtension
import okhttp3.HttpUrl
import okhttp3.MediaType.Companion.toMediaType
import okhttp3.OkHttpClient
Expand All @@ -37,10 +36,8 @@ import okhttp3.testing.PlatformRule
import org.junit.jupiter.api.Assertions.fail
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.RegisterExtension

@ExtendWith(MockWebServerExtension::class)
@Suppress("ktlint:standard:max-line-length")
class LoggingEventListenerTest {
@RegisterExtension
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import assertk.assertions.isEqualTo
import java.util.concurrent.TimeUnit
import mockwebserver3.MockResponse
import mockwebserver3.MockWebServer
import mockwebserver3.junit5.internal.MockWebServerExtension
import okhttp3.OkHttpClientTestRule
import okhttp3.RecordingEventListener
import okhttp3.Request
Expand All @@ -32,12 +31,10 @@ import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.RegisterExtension
import org.junitpioneer.jupiter.RetryingTest

@Tag("Slowish")
@ExtendWith(MockWebServerExtension::class)
class EventSourceHttpTest {
@RegisterExtension
val platform = PlatformRule()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package okhttp3.sse.internal

import mockwebserver3.MockResponse
import mockwebserver3.MockWebServer
import mockwebserver3.junit5.internal.MockWebServerExtension
import okhttp3.OkHttpClientTestRule
import okhttp3.Request
import okhttp3.sse.EventSources.processResponse
Expand All @@ -26,11 +25,9 @@ import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.RegisterExtension

@Tag("Slowish")
@ExtendWith(MockWebServerExtension::class)
class EventSourcesHttpTest {
@RegisterExtension
val platform = PlatformRule()
Expand Down
6 changes: 3 additions & 3 deletions okhttp/src/test/java/okhttp3/InterceptorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import assertk.assertions.isEqualTo
import assertk.assertions.isFalse
import assertk.assertions.isNotNull
import assertk.assertions.isNull
import assertk.assertions.isSameAs
import assertk.assertions.isSameInstanceAs
import assertk.assertions.isTrue
import java.io.IOException
import java.net.SocketTimeoutException
Expand Down Expand Up @@ -87,7 +87,7 @@ class InterceptorTest {
.addInterceptor(Interceptor { chain: Interceptor.Chain? -> interceptorResponse })
.build()
val response = client.newCall(request).execute()
assertThat(response).isSameAs(interceptorResponse)
assertThat(response).isSameInstanceAs(interceptorResponse)
}

@Test
Expand Down Expand Up @@ -820,7 +820,7 @@ class InterceptorTest {
assertFailsWith<IOException> {
call.execute()
}
assertThat(callRef.get()).isSameAs(call)
assertThat(callRef.get()).isSameInstanceAs(call)
}

private fun uppercase(original: RequestBody?): RequestBody {
Expand Down
3 changes: 0 additions & 3 deletions okhttp/src/test/java/okhttp3/internal/tls/ClientAuthTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import javax.security.auth.x500.X500Principal
import kotlin.test.assertFailsWith
import mockwebserver3.MockResponse
import mockwebserver3.MockWebServer
import mockwebserver3.junit5.internal.MockWebServerExtension
import okhttp3.OkHttpClient
import okhttp3.OkHttpClientTestRule
import okhttp3.RecordingEventListener
Expand All @@ -52,12 +51,10 @@ import okhttp3.tls.internal.TlsUtil.newTrustManager
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.RegisterExtension
import org.junitpioneer.jupiter.RetryingTest

@Tag("Slowish")
@ExtendWith(MockWebServerExtension::class)
class ClientAuthTest {
@RegisterExtension
val platform = PlatformRule()
Expand Down