-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
CoroutinesTimeout for JUnit5 #2402
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
a20c8f2
WIP on CoroutinesTimeout for JUnit5
dkhalanskyjb cd7afaf
Fix debug probes not being installed
dkhalanskyjb 531d6a1
Extract common code for JUnit4 and JUnit5 CoroutinesTimeout
dkhalanskyjb 77e646f
Generalize exceptions thrown from 'runWithTimeoutDumpingCoroutines'
dkhalanskyjb 1120924
Fix API check
dkhalanskyjb da8edd4
Test inheritance of CoroutinesTimeout for JUnit5
dkhalanskyjb 140ddfb
Fix DebugProbes installation in CoroutinesTimeout for JUnit 5
dkhalanskyjb 1a4ad3b
Disable configuring enableCoroutineCreationStacktrace for annotations
dkhalanskyjb a152ac6
Handle annotation on outher classes for JUnit5 CoroutinesTimeout
dkhalanskyjb d3117b4
Implement registering CoroutinesTimeoutExtension via RegisterExtension
dkhalanskyjb 8e414fd
Test CoroutinesTimeoutExtension thoroughly
dkhalanskyjb 6e51638
Add documentation for JUnit5 CoroutinesTimeout
dkhalanskyjb 79bd28c
Fix the debug probes not being always uninstalled
dkhalanskyjb 815e498
Fix
dkhalanskyjb b7df093
Improve handling of potential concurrency problems
dkhalanskyjb 324a001
Fixes
dkhalanskyjb 6c1d6e7
apiDump
dkhalanskyjb bb5216c
Fixes
dkhalanskyjb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
81 changes: 81 additions & 0 deletions
81
kotlinx-coroutines-debug/src/junit/CoroutinesTimeoutImpl.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
/* | ||
* Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. | ||
*/ | ||
|
||
package kotlinx.coroutines.debug | ||
|
||
import java.util.concurrent.* | ||
|
||
/** | ||
* Run [invocation] in a separate thread with the given timeout in ms, after which the coroutines info is dumped and, if | ||
* [cancelOnTimeout] is set, the execution is interrupted. | ||
* | ||
* Assumes that [DebugProbes] are installed. Does not deinstall them. | ||
*/ | ||
internal inline fun <T : Any?> runWithTimeoutDumpingCoroutines( | ||
methodName: String, | ||
testTimeoutMs: Long, | ||
cancelOnTimeout: Boolean, | ||
initCancellationException: () -> Throwable, | ||
crossinline invocation: () -> T | ||
): T { | ||
val testStartedLatch = CountDownLatch(1) | ||
val testResult = FutureTask { | ||
testStartedLatch.countDown() | ||
invocation() | ||
} | ||
/* | ||
* We are using hand-rolled thread instead of single thread executor | ||
* in order to be able to safely interrupt thread in the end of a test | ||
*/ | ||
val testThread = Thread(testResult, "Timeout test thread").apply { isDaemon = true } | ||
try { | ||
testThread.start() | ||
// Await until test is started to take only test execution time into account | ||
testStartedLatch.await() | ||
return testResult.get(testTimeoutMs, TimeUnit.MILLISECONDS) | ||
} catch (e: TimeoutException) { | ||
handleTimeout(testThread, methodName, testTimeoutMs, cancelOnTimeout, initCancellationException()) | ||
} catch (e: ExecutionException) { | ||
throw e.cause ?: e | ||
} | ||
} | ||
|
||
private fun handleTimeout(testThread: Thread, methodName: String, testTimeoutMs: Long, cancelOnTimeout: Boolean, | ||
cancellationException: Throwable): Nothing { | ||
val units = | ||
if (testTimeoutMs % 1000 == 0L) | ||
"${testTimeoutMs / 1000} seconds" | ||
else "$testTimeoutMs milliseconds" | ||
|
||
System.err.println("\nTest $methodName timed out after $units\n") | ||
System.err.flush() | ||
|
||
DebugProbes.dumpCoroutines() | ||
System.out.flush() // Synchronize serr/sout | ||
|
||
/* | ||
* Order is important: | ||
* 1) Create exception with a stacktrace of hang test | ||
* 2) Cancel all coroutines via debug agent API (changing system state!) | ||
* 3) Throw created exception | ||
*/ | ||
cancellationException.attachStacktraceFrom(testThread) | ||
testThread.interrupt() | ||
cancelIfNecessary(cancelOnTimeout) | ||
// If timed out test throws an exception, we can't do much except ignoring it | ||
throw cancellationException | ||
} | ||
|
||
private fun cancelIfNecessary(cancelOnTimeout: Boolean) { | ||
if (cancelOnTimeout) { | ||
DebugProbes.dumpCoroutinesInfo().forEach { | ||
it.job?.cancel() | ||
} | ||
} | ||
} | ||
|
||
private fun Throwable.attachStacktraceFrom(thread: Thread) { | ||
val stackTrace = thread.stackTrace | ||
this.stackTrace = stackTrace | ||
} |
File renamed without changes.
30 changes: 30 additions & 0 deletions
30
kotlinx-coroutines-debug/src/junit/junit4/CoroutinesTimeoutStatement.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
/* | ||
* Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. | ||
*/ | ||
|
||
package kotlinx.coroutines.debug.junit4 | ||
|
||
import kotlinx.coroutines.debug.* | ||
import org.junit.runner.* | ||
import org.junit.runners.model.* | ||
import java.util.concurrent.* | ||
|
||
internal class CoroutinesTimeoutStatement( | ||
private val testStatement: Statement, | ||
private val testDescription: Description, | ||
private val testTimeoutMs: Long, | ||
private val cancelOnTimeout: Boolean = false | ||
) : Statement() { | ||
|
||
override fun evaluate() { | ||
try { | ||
runWithTimeoutDumpingCoroutines(testDescription.methodName, testTimeoutMs, cancelOnTimeout, | ||
{ TestTimedOutException(testTimeoutMs, TimeUnit.MILLISECONDS) }) | ||
{ | ||
testStatement.evaluate() | ||
} | ||
} finally { | ||
DebugProbes.uninstall() | ||
} | ||
} | ||
} |
63 changes: 63 additions & 0 deletions
63
kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
/* | ||
* Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. | ||
*/ | ||
|
||
package kotlinx.coroutines.debug.junit5 | ||
import kotlinx.coroutines.debug.* | ||
import org.junit.jupiter.api.* | ||
import org.junit.jupiter.api.extension.* | ||
import org.junit.jupiter.api.parallel.* | ||
import java.lang.annotation.* | ||
|
||
/** | ||
* Coroutines timeout annotation that is similar to JUnit5's [Timeout] annotation. It allows running test methods in a | ||
* separate thread, failing them after the provided time limit and interrupting the thread. | ||
* | ||
* Additionally, it installs [DebugProbes] and dumps all coroutines at the moment of the timeout. It also cancels | ||
* coroutines on timeout if [cancelOnTimeout] set to `true`. The dump contains the coroutine creation stack traces. | ||
* | ||
* This annotation has an effect on test, test factory, test template, and lifecycle methods and test classes that are | ||
* annotated with it. | ||
* | ||
* Annotating a class is the same as annotating every test, test factory, and test template method (but not lifecycle | ||
* methods) of that class and its inner test classes, unless any of them is annotated with [CoroutinesTimeout], in which | ||
* case their annotation overrides the one on the containing class. | ||
* | ||
* Declaring [CoroutinesTimeout] on a test factory checks that it finishes in the specified time, but does not check | ||
* whether the methods that it produces obey the timeout as well. | ||
* | ||
* Example usage: | ||
* ``` | ||
* @CoroutinesTimeout(100) | ||
* class CoroutinesTimeoutSimpleTest { | ||
* // does not time out, as the annotation on the method overrides the class-level one | ||
* @CoroutinesTimeout(1000) | ||
* @Test | ||
* fun classTimeoutIsOverridden() { | ||
* runBlocking { | ||
* delay(150) | ||
* } | ||
* } | ||
* | ||
* // times out in 100 ms, timeout value is taken from the class-level annotation | ||
* @Test | ||
* fun classTimeoutIsUsed() { | ||
* runBlocking { | ||
* delay(150) | ||
* } | ||
* } | ||
* } | ||
* ``` | ||
* | ||
* @see Timeout | ||
*/ | ||
@ExtendWith(CoroutinesTimeoutExtension::class) | ||
@Inherited | ||
@MustBeDocumented | ||
@ResourceLock("coroutines timeout", mode = ResourceAccessMode.READ) | ||
@Retention(value = AnnotationRetention.RUNTIME) | ||
@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION) | ||
public annotation class CoroutinesTimeout( | ||
val testTimeoutMs: Long, | ||
val cancelOnTimeout: Boolean = false | ||
) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't seem to be necessary for the latest
develop
where we have 1.8 targetThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I attempted to remove this block, but then the build was failing. I managed to fix the build of this subproject by adding
However, then kotlinx-coroutines-test also required
targetCompatibility
set. At that point, I stopped. Do you think this should be pursued further?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's enough, thanks for checking it