Use runTestWithData in ClientLoader

ktorio · Dec 17, 2024 · 289b821 · 289b821
1 parent 5cbbda5
commit 289b821
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 73 deletions.
diff --git a/ktor-client/ktor-client-tests/common/src/io/ktor/client/tests/utils/ClientLoader.kt b/ktor-client/ktor-client-tests/common/src/io/ktor/client/tests/utils/ClientLoader.kt
@@ -1,12 +1,12 @@
 /*
- * Copyright 2014-2019 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
+ * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
  */
 
 package io.ktor.client.tests.utils
 
 import io.ktor.client.engine.*
+import io.ktor.test.*
 import kotlinx.coroutines.test.TestResult
-import kotlinx.coroutines.test.runTest
 import kotlin.time.Duration
 import kotlin.time.Duration.Companion.minutes
 
@@ -15,6 +15,8 @@ internal expect val platformName: String
 internal expect fun platformDumpCoroutines()
 internal expect fun platformWaitForAllCoroutines()
 
+private typealias ClientTestFailure = TestFailure<HttpClientEngineFactory<*>>
+
 /**
  * Helper interface to test client.
  */
@@ -26,39 +28,38 @@ abstract class ClientLoader(private val timeout: Duration = 1.minutes) {
         skipEngines: List<String> = emptyList(),
         onlyWithEngine: String? = null,
         retries: Int = 1,
+        timeout: Duration = this.timeout,
         block: suspend TestClientBuilder<HttpClientEngineConfig>.() -> Unit
-    ): TestResult = runTest(timeout = timeout) {
+    ): TestResult {
         val skipPatterns = skipEngines.map(SkipEnginePattern::parse)
-
-        val failures: List<TestFailure> = enginesToTest.mapNotNull { engineFactory ->
-            val engineName = engineFactory.engineName
-
-            if (shouldRun(engineName, skipPatterns, onlyWithEngine)) {
-                try {
-                    println("Run test with engine $engineName")
-                    // run test here
-                    performTestWithEngine(engineFactory, this@ClientLoader, retries, block)
-                    null // engine test passed
-                } catch (cause: Throwable) {
-                    // engine test failed, save failure to report after run for every engine.
-                    TestFailure(engineName, cause)
-                }
-            } else {
-                println("Skipping test with engine $engineName")
-                null // engine skipped
-            }
+        val (selectedEngines, skippedEngines) = enginesToTest
+            .partition { shouldRun(it.engineName, skipPatterns, onlyWithEngine) }
+        if (skippedEngines.isNotEmpty()) println("Skipped engines: ${skippedEngines.joinToString { it.engineName }}")
+
+        return runTestWithData(
+            selectedEngines,
+            timeout = timeout,
+            retries = retries,
+            handleFailures = ::aggregatedAssertionError,
+        ) { (engine, retry) ->
+            val retrySuffix = if (retry > 0) " [$retry]" else ""
+            println("Run test with engine ${engine.engineName}$retrySuffix")
+            performTestWithEngine(engine, this@ClientLoader, block)
         }
+    }
 
-        if (failures.isNotEmpty()) {
-            val message = buildString {
-                appendLine("Test failed for engines: ${failures.map { it.engineName }}")
-                failures.forEach {
-                    appendLine("Test failed for engine '$platformName:${it.engineName}' with:")
-                    appendLine(it.cause.stackTraceToString().prependIndent("  "))
-                }
+    private fun aggregatedAssertionError(failures: List<ClientTestFailure>): Nothing {
+        val message = buildString {
+            val engineNames = failures.map { it.data.engineName }
+            if (failures.size > 1) {
+                appendLine("Test failed for engines: ${engineNames.joinToString()}")
+            }
+            failures.forEachIndexed { index, (cause, _) ->
+                appendLine("Test failed for engine '$platformName:${engineNames[index]}' with:")
+                appendLine(cause.stackTraceToString().prependIndent("  "))
             }
-            throw AssertionError(message)
         }
+        throw AssertionError(message)
     }
 
     private fun shouldRun(
@@ -132,5 +133,3 @@ private data class SkipEnginePattern(
         }
     }
 }
-
-private class TestFailure(val engineName: String, val cause: Throwable)
diff --git a/ktor-client/ktor-client-tests/common/src/io/ktor/client/tests/utils/CommonClientTestUtils.kt b/ktor-client/ktor-client-tests/common/src/io/ktor/client/tests/utils/CommonClientTestUtils.kt
@@ -6,9 +6,9 @@ package io.ktor.client.tests.utils
 
 import io.ktor.client.*
 import io.ktor.client.engine.*
+import io.ktor.test.*
 import io.ktor.utils.io.core.*
 import kotlinx.coroutines.*
-import kotlinx.coroutines.test.runTest
 import kotlin.time.Duration
 import kotlin.time.Duration.Companion.minutes
 
@@ -45,16 +45,14 @@ private fun testWithClient(
     timeout: Duration,
     retries: Int,
     block: suspend TestClientBuilder<HttpClientEngineConfig>.() -> Unit
-) = runTest(timeout = timeout) {
+) = runTestWithData(listOf(client), timeout = timeout, retries = retries) {
     val builder = TestClientBuilder<HttpClientEngineConfig>().also { it.block() }
 
-    retryTest(retries) {
-        concurrency(builder.concurrency) { threadId ->
-            repeat(builder.repeatCount) { attempt ->
-                @Suppress("UNCHECKED_CAST")
-                client.config { builder.config(this as HttpClientConfig<HttpClientEngineConfig>) }
-                    .use { client -> builder.test(TestInfo(threadId, attempt), client) }
-            }
+    concurrency(builder.concurrency) { threadId ->
+        repeat(builder.repeatCount) { attempt ->
+            @Suppress("UNCHECKED_CAST")
+            client.config { builder.config(this as HttpClientConfig<HttpClientEngineConfig>) }
+                .use { client -> builder.test(TestInfo(threadId, attempt), client) }
         }
     }
 
@@ -70,15 +68,14 @@ fun <T : HttpClientEngineConfig> testWithEngine(
     timeout: Duration = 1.minutes,
     retries: Int = 1,
     block: suspend TestClientBuilder<T>.() -> Unit
-) = runTest(timeout = timeout) {
-    performTestWithEngine(factory, loader, retries, block)
+) = runTestWithData(listOf(factory), timeout = timeout, retries = retries) {
+    performTestWithEngine(factory, loader, block)
 }
 
 @OptIn(DelicateCoroutinesApi::class)
 suspend fun <T : HttpClientEngineConfig> performTestWithEngine(
     factory: HttpClientEngineFactory<T>,
     loader: ClientLoader? = null,
-    retries: Int = 1,
     block: suspend TestClientBuilder<T>.() -> Unit
 ) {
     val builder = TestClientBuilder<T>().apply { block() }
@@ -90,45 +87,31 @@ suspend fun <T : HttpClientEngineConfig> performTestWithEngine(
         }
     }
 
-    retryTest(retries) {
-        withContext(Dispatchers.Default.limitedParallelism(1)) {
-            concurrency(builder.concurrency) { threadId ->
-                repeat(builder.repeatCount) { attempt ->
-                    val client = HttpClient(factory, block = builder.config)
+    withContext(Dispatchers.Default.limitedParallelism(1)) {
+        concurrency(builder.concurrency) { threadId ->
+            repeat(builder.repeatCount) { attempt ->
+                val client = HttpClient(factory, block = builder.config)
 
-                    client.use {
-                        builder.test(TestInfo(threadId, attempt), it)
-                    }
+                client.use {
+                    builder.test(TestInfo(threadId, attempt), it)
+                }
 
-                    try {
-                        val job = client.coroutineContext[Job]!!
-                        while (job.isActive) {
-                            yield()
-                        }
-                    } catch (cause: Throwable) {
-                        client.cancel("Test failed", cause)
-                        throw cause
-                    } finally {
-                        builder.after(client)
+                try {
+                    val job = client.coroutineContext[Job]!!
+                    while (job.isActive) {
+                        yield()
                     }
+                } catch (cause: Throwable) {
+                    client.cancel("Test failed", cause)
+                    throw cause
+                } finally {
+                    builder.after(client)
                 }
             }
         }
     }
 }
 
-internal suspend fun <T> retryTest(attempts: Int, block: suspend () -> T): T {
-    var currentAttempt = 0
-    while (true) {
-        try {
-            return block()
-        } catch (cause: Throwable) {
-            if (currentAttempt >= attempts) throw cause
-            currentAttempt++
-        }
-    }
-}
-
 private suspend fun concurrency(level: Int, block: suspend (Int) -> Unit) {
     coroutineScope {
         List(level) {