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

Refactor ClientLoader.clientTests #4548

Merged
merged 6 commits into from
Dec 17, 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
4 changes: 2 additions & 2 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ apply(from = "gradle/verifier.gradle")
extra["skipPublish"] = mutableListOf(
"ktor-server-test-suites",
"ktor-server-tests",
"ktor-junit",
"ktor-test-base",
)

// Point old artifact to new location
Expand All @@ -48,7 +48,7 @@ val disabledExplicitApiModeProjects = listOf(
"ktor-server-test-suites",
"ktor-server-tests",
"ktor-client-content-negotiation-tests",
"ktor-junit"
"ktor-test-base"
)

apply(from = "gradle/compatibility.gradle")
Expand Down
17 changes: 8 additions & 9 deletions buildSrc/src/main/kotlin/JsConfig.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@
* Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
*/

import internal.*
import org.gradle.api.*
import org.gradle.internal.extensions.stdlib.*
import org.gradle.kotlin.dsl.*
import org.jetbrains.kotlin.gradle.targets.js.dsl.*
import java.io.*
import kotlin.toString
import internal.capitalized
import internal.libs
import org.gradle.api.Project
import org.gradle.kotlin.dsl.invoke
import org.jetbrains.kotlin.gradle.targets.js.dsl.KotlinJsSubTargetDsl

fun Project.configureJs() {
kotlin {
Expand All @@ -34,7 +32,8 @@ fun Project.configureJs() {
internal fun KotlinJsSubTargetDsl.useMochaForTests() {
testTask {
useMocha {
timeout = "10000"
// Disable timeout as we use individual timeouts for tests
timeout = "0"
}
}
}
Expand All @@ -43,7 +42,7 @@ internal fun KotlinJsSubTargetDsl.useKarmaForTests() {
testTask {
useKarma {
useChromeHeadless()
useConfigDirectory(File(project.rootProject.projectDir, "karma"))
useConfigDirectory(project.rootProject.file("karma"))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion gradle/compatibility.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ apiValidation {
'ktor-client-plugins',
'ktor-server-test-suites',
"ktor-server-test-base",
'ktor-junit',
'ktor-test-base',
].toSet()

def projects = [].toSet()
Expand Down
3 changes: 2 additions & 1 deletion karma/chrome_bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ config.set({
"client": {
captureConsole: true,
"mocha": {
timeout: 10000
// Disable timeout as we use individual timeouts for tests
timeout: 0
}
}
});
Expand Down
2 changes: 1 addition & 1 deletion ktor-client/ktor-client-cio/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ kotlin {
jvmTest {
dependencies {
api(project(":ktor-network:ktor-network-tls:ktor-network-tls-certificates"))
api(project(":ktor-shared:ktor-junit"))
api(project(":ktor-shared:ktor-test-base"))
implementation(libs.mockk)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import io.ktor.client.statement.*
import io.ktor.client.tests.utils.*
import io.ktor.http.*
import io.ktor.http.content.*
import io.ktor.junit.coroutines.*
import io.ktor.server.engine.*
import io.ktor.server.netty.*
import io.ktor.server.response.*
import io.ktor.server.routing.*
import io.ktor.test.junit.coroutines.*
import io.mockk.mockkStatic
import io.mockk.verify
import kotlinx.coroutines.delay
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import io.ktor.client.plugins.*
import io.ktor.client.request.*
import io.ktor.client.statement.*
import io.ktor.http.*
import io.ktor.junit.coroutines.*
import io.ktor.test.junit.coroutines.*
import io.ktor.network.tls.certificates.*
import io.ktor.server.application.*
import io.ktor.server.engine.*
Expand Down
5 changes: 2 additions & 3 deletions ktor-client/ktor-client-tests/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ kotlin.sourceSets {
dependencies {
api(project(":ktor-client:ktor-client-mock"))
api(project(":ktor-test-dispatcher"))
api(libs.kotlin.test)
api(libs.kotlinx.coroutines.test)
api(project(":ktor-shared:ktor-test-base"))
}
}
commonTest {
Expand Down Expand Up @@ -46,7 +45,7 @@ kotlin.sourceSets {
api(project(":ktor-server:ktor-server-plugins:ktor-server-auth"))
api(project(":ktor-server:ktor-server-plugins:ktor-server-websockets"))
api(project(":ktor-shared:ktor-serialization:ktor-serialization-kotlinx"))
api(project(":ktor-shared:ktor-junit"))
api(project(":ktor-shared:ktor-test-base"))
api(libs.logback.classic)
implementation(libs.kotlinx.coroutines.debug)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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.
*/
Expand All @@ -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(
Expand Down Expand Up @@ -132,5 +133,3 @@ private data class SkipEnginePattern(
}
}
}

private class TestFailure(val engineName: String, val cause: Throwable)
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ 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.Companion.milliseconds
import kotlin.time.Duration
import kotlin.time.Duration.Companion.minutes

/**
* Web url for tests.
Expand All @@ -31,29 +32,27 @@ const val TCP_SERVER: String = "http://127.0.0.1:8082"
*/
fun testWithEngine(
engine: HttpClientEngine,
timeoutMillis: Long = 60 * 1000L,
timeout: Duration = 1.minutes,
retries: Int = 1,
block: suspend TestClientBuilder<*>.() -> Unit
) = testWithClient(HttpClient(engine), timeoutMillis, retries, block)
) = testWithClient(HttpClient(engine), timeout, retries, block)

/**
* Perform test with selected [client].
*/
private fun testWithClient(
client: HttpClient,
timeout: Long,
timeout: Duration,
retries: Int,
block: suspend TestClientBuilder<HttpClientEngineConfig>.() -> Unit
) = runTest(timeout = timeout.milliseconds) {
) = 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) }
}
}

Expand All @@ -66,18 +65,17 @@ private fun testWithClient(
fun <T : HttpClientEngineConfig> testWithEngine(
factory: HttpClientEngineFactory<T>,
loader: ClientLoader? = null,
timeoutMillis: Long = 60L * 1000L,
timeout: Duration = 1.minutes,
retries: Int = 1,
block: suspend TestClientBuilder<T>.() -> Unit
) = runTest(timeout = timeoutMillis.milliseconds) {
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() }
Expand All @@ -89,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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ val testArrays = testSize.map {
makeArray(it)
}

class ContentTest : ClientLoader(timeout = 5.minutes) {
class ContentTest : ClientLoader() {

@Test
fun testGetFormData() = clientTests {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2014-2023 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.plugins
Expand All @@ -25,9 +25,8 @@ import kotlin.coroutines.CoroutineContext
import kotlin.coroutines.resume
import kotlin.coroutines.suspendCoroutine
import kotlin.test.*
import kotlin.time.Duration.Companion.minutes

class ServerSentEventsTest : ClientLoader(timeout = 2.minutes) {
class ServerSentEventsTest : ClientLoader() {

@Test
fun testExceptionIfSseIsNotInstalled() = testSuspend {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ package io.ktor.client.tests.utils

import ch.qos.logback.classic.Level
import ch.qos.logback.classic.Logger
import io.ktor.junit.coroutines.*
import io.ktor.server.engine.*
import io.ktor.test.junit.coroutines.*
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.slf4j.LoggerFactory
Expand Down
Loading
Loading