Skip to content

Commit

Permalink
Rollback runTest timeout to 60 seconds, configure it with getenv
Browse files Browse the repository at this point in the history
This commit attempts to fix #3800.
The first part of the fix, reverting the timeout to 60 seconds, is
successful.
The second part, allowing global configuration, not so much.

On JVM, Native, and Node JS, it's possible to use environment
variables to communicate data to the process, and it could in
theory be the solution, but it doesn't seem to interoperate well
with Gradle.

The best attempt so far was to use this:
```kotlin
tasks.withType(AbstractTestTask::class).all {
    if (this is ProcessForkOptions) {
        environment("kotlinx.coroutines.test.default_timeout", "1ms")
    }
}
```

Unfortunately, only `jvmTest` implements `ProcessForkOptions`.

Without a clear way to configure the `runTest` timeout in Gradle
builds, we can't claim that this is a proper solution.
  • Loading branch information
dkhalanskyjb committed Nov 20, 2023
1 parent ff95ab7 commit 2843d66
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 5 deletions.
33 changes: 28 additions & 5 deletions kotlinx-coroutines-test/common/src/TestBuilders.kt
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,13 @@ public expect class TestResult
*
* #### Timing out
*
* There's a built-in timeout of 10 seconds for the test body. If the test body doesn't complete within this time,
* then the test fails with an [AssertionError]. The timeout can be changed by setting the [timeout] parameter.
* There's a built-in timeout of 60 seconds for the test body. If the test body doesn't complete within this time,
* then the test fails with an [AssertionError]. The timeout can be changed for each test separately by setting the
* [timeout] parameter. Additionally, setting the `kotlinx.coroutines.test.default_timeout` environment variable to
* any string that can be parsed using [Duration.parse] (like `1m`, `30s` or `1500ms`) will change the default timeout
* to that value for all tests whose [timeout] is not set explicitly; setting it to anything else will throw an
* exception every time [runTest] is invoked.
* Note that environment variables and JVM system properties are separate concepts and are configured differently.
*
* On timeout, the test body is cancelled so that the test finishes. If the code inside the test body does not
* respond to cancellation, the timeout will not be able to make the test execution stop.
Expand Down Expand Up @@ -157,7 +162,7 @@ public expect class TestResult
*/
public fun runTest(
context: CoroutineContext = EmptyCoroutineContext,
timeout: Duration = DEFAULT_TIMEOUT,
timeout: Duration = DEFAULT_TIMEOUT.getOrThrow(),
testBody: suspend TestScope.() -> Unit
): TestResult {
check(context[RunningInRunTest] == null) {
Expand Down Expand Up @@ -301,7 +306,7 @@ public fun runTest(
* Performs [runTest] on an existing [TestScope]. See the documentation for [runTest] for details.
*/
public fun TestScope.runTest(
timeout: Duration = DEFAULT_TIMEOUT,
timeout: Duration = DEFAULT_TIMEOUT.getOrThrow(),
testBody: suspend TestScope.() -> Unit
): TestResult = asSpecificImplementation().let { scope ->
scope.enter()
Expand Down Expand Up @@ -421,8 +426,15 @@ internal const val DEFAULT_DISPATCH_TIMEOUT_MS = 60_000L

/**
* The default timeout to use when running a test.
*
* It's not just a [Duration] but a [Result] so that every access to [runTest]
* throws the same clear exception if parsing the environment variable failed.
* Otherwise, the parsing error would only be thrown in one tests, while the
* other ones would get an incomprehensible `NoClassDefFoundError`.
*/
internal val DEFAULT_TIMEOUT = 10.seconds
private val DEFAULT_TIMEOUT: Result<Duration> = runCatching {
environmentVariable("kotlinx.coroutines.test.default_timeout", Duration::parse, 60.seconds)
}

/**
* Run the [body][testBody] of the [test coroutine][coroutine], waiting for asynchronous completions for at most
Expand Down Expand Up @@ -571,6 +583,17 @@ internal fun throwAll(head: Throwable?, other: List<Throwable>) {

internal expect fun dumpCoroutines()

private fun <T: Any> environmentVariable(
name: String,
parse: (String) -> T,
default: T,
): T {
val value = environmentVariableImpl(name) ?: return default
return parse(value)
}

internal expect fun environmentVariableImpl(name: String): String?

@Deprecated(
"This is for binary compatibility with the `runTest` overload that existed at some point",
level = DeprecationLevel.HIDDEN
Expand Down
9 changes: 9 additions & 0 deletions kotlinx-coroutines-test/js/src/TestBuilders.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ import kotlin.js.*
@Suppress("ACTUAL_WITHOUT_EXPECT", "ACTUAL_TYPE_ALIAS_TO_CLASS_WITH_DECLARATION_SITE_VARIANCE")
public actual typealias TestResult = Promise<Unit>

private external val process: dynamic

internal actual fun environmentVariableImpl(name: String): String? {
if (jsTypeOf(process) == "undefined") return null
if (jsTypeOf(process.env) == "undefined") return null
return process.env[name] as? String
}


internal actual fun createTestResult(testProcedure: suspend CoroutineScope.() -> Unit): TestResult =
GlobalScope.promise {
testProcedure()
Expand Down
3 changes: 3 additions & 0 deletions kotlinx-coroutines-test/jvm/src/TestBuildersJvm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ internal actual fun createTestResult(testProcedure: suspend CoroutineScope.() ->
}
}

internal actual fun environmentVariableImpl(name: String): String? =
System.getenv(name)

internal actual fun dumpCoroutines() {
@Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER")
if (DebugProbesImpl.isInstalled) {
Expand Down
6 changes: 6 additions & 0 deletions kotlinx-coroutines-test/native/src/TestBuilders.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package kotlinx.coroutines.test
import kotlinx.coroutines.*
import kotlin.native.concurrent.*
import kotlinx.cinterop.*
import platform.posix.*

@Suppress("ACTUAL_WITHOUT_EXPECT")
public actual typealias TestResult = Unit
Expand All @@ -15,4 +17,8 @@ internal actual fun createTestResult(testProcedure: suspend CoroutineScope.() ->
}
}

@OptIn(kotlinx.cinterop.ExperimentalForeignApi::class)
internal actual fun environmentVariableImpl(name: String): String? =
getenv(name)?.toKString()

internal actual fun dumpCoroutines() { }

0 comments on commit 2843d66

Please sign in to comment.