[testing] Simplify our implementation of runTest (#5935)

* remove our own implementation of runTest * make sure to close the connection at the end of the test
apollographql · Jun 4, 2024 · 5b6ad56 · 5b6ad56
1 parent 9f630e5
commit 5b6ad56
Show file tree

Hide file tree

Showing 21 changed files with 385 additions and 565 deletions.
diff --git a/...ries/apollo-runtime/src/androidInstrumentedTest/kotlin/instrumented/NetworkMonitorTest.kt b/...ries/apollo-runtime/src/androidInstrumentedTest/kotlin/instrumented/NetworkMonitorTest.kt
@@ -36,7 +36,6 @@ class NetworkMonitorTest {
 
   @Test
   fun test() = mockServerTest(
-      skipDelays = false,
       clientBuilder = {
         networkMonitor(NetworkMonitor(InstrumentationRegistry.getInstrumentation().context))
         retryOnError { true }

diff --git a/libraries/apollo-runtime/src/commonTest/kotlin/test/network/NetworkMonitorTest.kt b/libraries/apollo-runtime/src/commonTest/kotlin/test/network/NetworkMonitorTest.kt
@@ -13,7 +13,6 @@ import com.apollographql.apollo3.mockserver.assertNoRequest
 import com.apollographql.apollo3.mockserver.enqueueString
 import com.apollographql.apollo3.network.NetworkMonitor
 import com.apollographql.apollo3.testing.FooQuery
-import com.apollographql.apollo3.testing.internal.ApolloTestResult
 import com.apollographql.apollo3.testing.internal.runTest
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.Flow
@@ -24,6 +23,7 @@ import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.mapNotNull
 import kotlinx.coroutines.flow.takeWhile
+import kotlinx.coroutines.test.TestResult
 import okio.use
 import kotlin.test.Test
 import kotlin.test.assertEquals
@@ -33,7 +33,7 @@ import kotlin.test.assertNull
 
 class NetworkMonitorTest {
   @Test
-  fun failFastIfOfflineTest(): ApolloTestResult {
+  fun failFastIfOfflineTest(): TestResult {
     val fakeNetworkMonitor = FakeNetworkMonitor()
 
     return mockServerTest(clientBuilder = {
@@ -57,7 +57,7 @@ class NetworkMonitorTest {
   }
 
   @Test
-  fun networkMonitorInterceptorTest(): ApolloTestResult {
+  fun networkMonitorInterceptorTest(): TestResult {
     val fakeNetworkMonitor = FakeNetworkMonitor()
 
     return mockServerTest(clientBuilder = {
@@ -116,7 +116,7 @@ class MockServerTest(val mockServer: MockServer, val apolloClient: ApolloClient,
 fun mockServerTest(
     clientBuilder: ApolloClient.Builder.() -> Unit = {},
     block: suspend MockServerTest.() -> Unit
-) = runTest(true) {
+) = runTest() {
   MockServer().use { mockServer ->
     ApolloClient.Builder()
         .serverUrl(mockServer.url())

diff --git a/libraries/apollo-runtime/src/commonTest/kotlin/test/network/WebSocketNetworkTransportTest.kt b/libraries/apollo-runtime/src/commonTest/kotlin/test/network/WebSocketNetworkTransportTest.kt
@@ -192,7 +192,7 @@ class WebSocketNetworkTransportTest {
   }
 
   @Test
-  fun socketClosedEmitsException() = runTest(false) {
+  fun socketClosedEmitsException() = runTest() {
     val mockServer = MockServer()
     ApolloClient.Builder()
         .serverUrl(mockServer.url())
@@ -390,7 +390,7 @@ class MockServerWebSocketTest(
   }
 }
 
-fun mockServerWebSocketTest(customizeTransport: WebSocketNetworkTransport.Builder.() -> Unit = {}, block: suspend MockServerWebSocketTest.() -> Unit) = runTest(false) {
+fun mockServerWebSocketTest(customizeTransport: WebSocketNetworkTransport.Builder.() -> Unit = {}, block: suspend MockServerWebSocketTest.() -> Unit) = runTest() {
   MockServer().use { mockServer ->
 
     ApolloClient.Builder()

diff --git a/libraries/apollo-runtime/src/jvmTest/kotlin/RetryWebSocketsTest.kt b/libraries/apollo-runtime/src/jvmTest/kotlin/RetryWebSocketsTest.kt
@@ -34,7 +34,7 @@ import kotlin.time.Duration.Companion.seconds
 
 class RetryWebSocketsTest {
   @Test
-  fun retryIsWorking() = runTest(skipDelays = false) {
+  fun retryIsWorking() = runTest {
     MockServer().use { mockServer ->
 
     ApolloClient.Builder()
@@ -91,7 +91,7 @@ class RetryWebSocketsTest {
   }
 
   @Test
-  fun socketReopensAfterAnError() = runTest(false) {
+  fun socketReopensAfterAnError() = runTest {
     var mockServer = MockServer()
 
     ApolloClient.Builder()
@@ -152,7 +152,7 @@ class RetryWebSocketsTest {
   }
 
   @Test
-  fun retryCanBeDisabled() = runTest(skipDelays = false) {
+  fun retryCanBeDisabled() = runTest {
     val mockServer = MockServer()
     ApolloClient.Builder()
         .serverUrl(mockServer.url())
@@ -191,7 +191,7 @@ class RetryWebSocketsTest {
   }
 
   @Test
-  fun subscriptionsAreNotRetriedByDefault() = runTest(skipDelays = false) {
+  fun subscriptionsAreNotRetriedByDefault() = runTest {
     val mockServer = MockServer()
     ApolloClient.Builder()
         .serverUrl(mockServer.url())

diff --git a/libraries/apollo-testing-support/build.gradle.kts b/libraries/apollo-testing-support/build.gradle.kts
@@ -18,7 +18,7 @@ kotlin {
         implementation(libs.atomicfu.library.get().toString()) {
           because("We need locks in TestNetworkTransportHandler (we don't use the gradle plugin rewrite)")
         }
-        implementation(libs.kotlinx.coroutines.test)
+        api(libs.kotlinx.coroutines.test)
       }
     }
 

diff --git a/...-support/src/appleMain/kotlin/com/apollographql/apollo3/testing/internal/runTest.apple.kt b/...-support/src/appleMain/kotlin/com/apollographql/apollo3/testing/internal/runTest.apple.kt
diff --git a/...sting-support/src/commonMain/kotlin/com/apollographql/apollo3/testing/internal/runTest.kt b/...sting-support/src/commonMain/kotlin/com/apollographql/apollo3/testing/internal/runTest.kt
@@ -2,22 +2,40 @@ package com.apollographql.apollo3.testing.internal
 
 import com.apollographql.apollo3.annotations.ApolloInternal
 import kotlinx.coroutines.CoroutineScope
-import kotlin.coroutines.CoroutineContext
-import kotlin.coroutines.EmptyCoroutineContext
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.TestResult
+import kotlinx.coroutines.withContext
 
 /**
- * Utility method that executes the given [block] with optional [before] and [after] blocks.
- *
- * When [skipDelays] is `true`, the block is executed in [kotlinx.coroutines.test.runTest], otherwise in `runBlocking`.
+ * Utility method that executes the given [block] with optional [before] and [after] blocks and disables
+ * the skipDelay behaviour from the coroutines `runTest`. Our tests use delay() in some situations.
  */
 @ApolloInternal
-expect fun runTest(
-    skipDelays: Boolean = false,
-    context: CoroutineContext = EmptyCoroutineContext,
-    before: suspend CoroutineScope.() -> Unit = {},
-    after: suspend CoroutineScope.() -> Unit = {},
+fun runTest(
     block: suspend CoroutineScope.() -> Unit,
-): ApolloTestResult
+) = runTest(before = {}, after = {}, block)
 
+/**
+ * We should probably deprecate this overload and remove the before/after state and lateinit
+ * variables in various tests.There are > 150 instances of it though, so I'm not pushing the
+ * deprecation button just yet.
+ */
+@OptIn(ExperimentalCoroutinesApi::class)
 @ApolloInternal
-expect class ApolloTestResult
+fun runTest(
+    before: suspend CoroutineScope.() -> Unit = {},
+    after: suspend CoroutineScope.() -> Unit = {},
+    block: suspend CoroutineScope.() -> Unit,
+): TestResult {
+  return kotlinx.coroutines.test.runTest {
+    /**
+     * See https://github.com/Kotlin/kotlinx.coroutines/issues/3179
+     */
+    withContext(Dispatchers.Default.limitedParallelism(1)) {
+      before()
+      block()
+      after()
+    }
+  }
+}
diff --git a/...apollo-testing-support/src/commonMain/kotlin/com/apollographql/apollo3/testing/runTest.kt b/...apollo-testing-support/src/commonMain/kotlin/com/apollographql/apollo3/testing/runTest.kt
@@ -21,10 +21,9 @@ class MockServerTest(val mockServer: MockServer, val apolloClient: ApolloClient,
 @ApolloDeprecatedSince(ApolloDeprecatedSince.Version.v4_0_0)
 @Suppress("DEPRECATION")
 fun mockServerTest(
-    skipDelays: Boolean = true,
     clientBuilder: ApolloClient.Builder.() -> Unit = {},
     block: suspend MockServerTest.() -> Unit
-) = runTest(skipDelays) {
+) = runTest {
   MockServer().use { mockServer ->
     ApolloClient.Builder()
         .serverUrl(mockServer.url())

diff --git a/...esting-support/src/jsMain/kotlin/com/apollographql/apollo3/testing/internal/runTest.js.kt b/...esting-support/src/jsMain/kotlin/com/apollographql/apollo3/testing/internal/runTest.js.kt
diff --git a/...ting-support/src/jvmMain/kotlin/com/apollographql/apollo3/testing/internal/runTest.jvm.kt b/...ting-support/src/jvmMain/kotlin/com/apollographql/apollo3/testing/internal/runTest.jvm.kt
diff --git a/...upport/src/wasmJsMain/kotlin/com/apollographql/apollo3/testing/internal/runTest.wasmJs.kt b/...upport/src/wasmJsMain/kotlin/com/apollographql/apollo3/testing/internal/runTest.wasmJs.kt
diff --git a/tests/engine/src/commonTest/kotlin/WebSocketEngineTest.kt b/tests/engine/src/commonTest/kotlin/WebSocketEngineTest.kt
@@ -1,9 +1,10 @@
+
 import com.apollographql.apollo3.api.http.HttpHeader
 import com.apollographql.apollo3.exception.ApolloException
 import com.apollographql.apollo3.exception.ApolloNetworkException
 import com.apollographql.apollo3.exception.ApolloWebSocketClosedException
-import com.apollographql.apollo3.mockserver.DataMessage
 import com.apollographql.apollo3.mockserver.CloseFrame
+import com.apollographql.apollo3.mockserver.DataMessage
 import com.apollographql.apollo3.mockserver.MockServer
 import com.apollographql.apollo3.mockserver.TextMessage
 import com.apollographql.apollo3.mockserver.awaitWebSocketRequest
@@ -111,6 +112,7 @@ class WebSocketEngineTest {
       assertEquals("Bye now", e.reason)
     }
 
+    connection.close()
     webSocketServer.close()
   }
 

diff --git a/tests/gzip/src/commonTest/kotlin/test/GzipTest.kt b/tests/gzip/src/commonTest/kotlin/test/GzipTest.kt
@@ -42,7 +42,7 @@ class MockServerTest(val mockServer: MockServer, val apolloClient: ApolloClient,
 fun mockServerTest(
     clientBuilder: ApolloClient.Builder.() -> Unit = {},
     block: suspend MockServerTest.() -> Unit
-) = runTest(true) {
+) = runTest() {
   MockServer().use { mockServer ->
     ApolloClient.Builder()
         .serverUrl(mockServer.url())