From a20c8f2635346892f853aceaaa1212c536b397d4 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 5 Oct 2020 11:20:06 +0300 Subject: [PATCH 01/18] WIP on CoroutinesTimeout for JUnit5 --- gradle.properties | 1 + .../api/kotlinx-coroutines-debug.api | 17 ++ kotlinx-coroutines-debug/build.gradle | 8 + .../src/junit5/CoroutinesTimeout.kt | 20 +++ .../src/junit5/CoroutinesTimeoutExtension.kt | 163 ++++++++++++++++++ .../test/junit5/CoroutinesTimeoutTest.kt | 22 +++ 6 files changed, 231 insertions(+) create mode 100644 kotlinx-coroutines-debug/src/junit5/CoroutinesTimeout.kt create mode 100644 kotlinx-coroutines-debug/src/junit5/CoroutinesTimeoutExtension.kt create mode 100644 kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt diff --git a/gradle.properties b/gradle.properties index 97c6af986f..46ec77cab9 100644 --- a/gradle.properties +++ b/gradle.properties @@ -9,6 +9,7 @@ kotlin_version=1.5.0-RC # Dependencies junit_version=4.12 +junit5_version=5.5.0 atomicfu_version=0.15.2 knit_version=0.2.3 html_version=0.7.2 diff --git a/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api b/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api index b6056c410c..8120454c33 100644 --- a/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api +++ b/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api @@ -61,3 +61,20 @@ public final class kotlinx/coroutines/debug/junit4/CoroutinesTimeout$Companion { public static synthetic fun seconds$default (Lkotlinx/coroutines/debug/junit4/CoroutinesTimeout$Companion;JZZILjava/lang/Object;)Lkotlinx/coroutines/debug/junit4/CoroutinesTimeout; } +public abstract interface annotation class kotlinx/coroutines/debug/junit5/CoroutinesTimeout : java/lang/annotation/Annotation { + public abstract fun cancelOnTimeout ()Z + public abstract fun enableCoroutineCreationStackTraces ()Z + public abstract fun testTimeoutMs ()J +} + +public final class kotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension : org/junit/jupiter/api/extension/InvocationInterceptor { + public fun ()V + public fun interceptAfterAllMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V + public fun interceptAfterEachMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V + public fun interceptBeforeAllMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V + public fun interceptBeforeEachMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V + public fun interceptTestFactoryMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)Ljava/lang/Object; + public fun interceptTestMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V + public fun interceptTestTemplateMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V +} + diff --git a/kotlinx-coroutines-debug/build.gradle b/kotlinx-coroutines-debug/build.gradle index faaed91206..a9f8876511 100644 --- a/kotlinx-coroutines-debug/build.gradle +++ b/kotlinx-coroutines-debug/build.gradle @@ -18,8 +18,16 @@ configurations { configureKotlinJvmPlatform(shadow) } +test { + useJUnitPlatform() +} + dependencies { compileOnly "junit:junit:$junit_version" + compileOnly "org.junit.jupiter:junit-jupiter-api:$junit5_version" + testCompile "org.junit.jupiter:junit-jupiter-api:$junit5_version" + testCompile "org.junit.jupiter:junit-jupiter-engine:$junit5_version" + testRuntimeOnly "org.junit.vintage:junit-vintage-engine:$junit5_version" shadowDeps "net.bytebuddy:byte-buddy:$byte_buddy_version" shadowDeps "net.bytebuddy:byte-buddy-agent:$byte_buddy_version" compileOnly "io.projectreactor.tools:blockhound:$blockhound_version" diff --git a/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeout.kt b/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeout.kt new file mode 100644 index 0000000000..876dc5e7e5 --- /dev/null +++ b/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeout.kt @@ -0,0 +1,20 @@ +/* + * 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 org.junit.jupiter.api.extension.* +import java.lang.annotation.* + +/** + */ +@MustBeDocumented +@Retention(value = AnnotationRetention.RUNTIME) +@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION) +@ExtendWith(CoroutinesTimeoutExtension::class) +@Inherited +public annotation class CoroutinesTimeout( + val testTimeoutMs: Long, + val cancelOnTimeout: Boolean = false, + val enableCoroutineCreationStackTraces: Boolean = true +) \ No newline at end of file diff --git a/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeoutExtension.kt new file mode 100644 index 0000000000..64626bcbce --- /dev/null +++ b/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeoutExtension.kt @@ -0,0 +1,163 @@ +/* + * 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.ExperimentalCoroutinesApi +import kotlinx.coroutines.debug.DebugProbes +import org.junit.jupiter.api.extension.BeforeAllCallback +import org.junit.jupiter.api.extension.ExtensionConfigurationException +import org.junit.jupiter.api.extension.ExtensionContext +import org.junit.jupiter.api.extension.InvocationInterceptor +import org.junit.jupiter.api.extension.ReflectiveInvocationContext +import org.junit.platform.commons.JUnitException +import org.junit.platform.commons.support.AnnotationSupport +import org.junit.runner.* +import org.junit.runners.model.* +import java.io.ByteArrayOutputStream +import java.io.PrintStream +import java.lang.reflect.Method +import java.time.Duration +import java.time.format.DateTimeParseException +import java.util.concurrent.CountDownLatch +import java.util.concurrent.ExecutionException +import java.util.concurrent.FutureTask +import java.util.concurrent.TimeUnit +import java.util.concurrent.TimeoutException + +public class CoroutinesTimeoutExtension: InvocationInterceptor { + + override fun interceptTestMethod( + invocation: InvocationInterceptor.Invocation, + invocationContext: ReflectiveInvocationContext, + extensionContext: ExtensionContext + ) { + val annotation = + AnnotationSupport.findAnnotation(invocationContext.executable, CoroutinesTimeout::class.java).or { + AnnotationSupport.findAnnotation(extensionContext.testClass, CoroutinesTimeout::class.java) + }.orElseGet { + throw UnsupportedOperationException("CoroutinesTimeoutExtension should not be used directly; annotate the test class or method with CoroutinesTimeout instead.") + } + invocation.proceed() + } + + override fun interceptAfterAllMethod( + invocation: InvocationInterceptor.Invocation, + invocationContext: ReflectiveInvocationContext, + extensionContext: ExtensionContext + ) { + interceptLifecycleMethod(invocation, invocationContext, extensionContext) + } + + override fun interceptAfterEachMethod( + invocation: InvocationInterceptor.Invocation, + invocationContext: ReflectiveInvocationContext, + extensionContext: ExtensionContext + ) { + interceptLifecycleMethod(invocation, invocationContext, extensionContext) + } + + override fun interceptBeforeAllMethod( + invocation: InvocationInterceptor.Invocation, + invocationContext: ReflectiveInvocationContext, + extensionContext: ExtensionContext + ) { + interceptLifecycleMethod(invocation, invocationContext, extensionContext) + } + + override fun interceptBeforeEachMethod( + invocation: InvocationInterceptor.Invocation, + invocationContext: ReflectiveInvocationContext, + extensionContext: ExtensionContext + ) { + interceptLifecycleMethod(invocation, invocationContext, extensionContext) + } + + private fun interceptLifecycleMethod( + invocation: InvocationInterceptor.Invocation, + invocationContext: ReflectiveInvocationContext, + extensionContext: ExtensionContext + ) { + invocation.proceed() + } + + override fun interceptTestFactoryMethod( + invocation: InvocationInterceptor.Invocation, + invocationContext: ReflectiveInvocationContext, + extensionContext: ExtensionContext + ): T = invocation.proceed() + + override fun interceptTestTemplateMethod( + invocation: InvocationInterceptor.Invocation, + invocationContext: ReflectiveInvocationContext, + extensionContext: ExtensionContext + ) { + invocation.proceed() + } + + private fun interceptInvocation( + invocation: InvocationInterceptor.Invocation, + methodName: String, + annotation: CoroutinesTimeout + ): T { + val testStartedLatch = CountDownLatch(1) + val testResult = FutureTask { + testStartedLatch.countDown() + invocation.proceed() + } + 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(annotation.testTimeoutMs, TimeUnit.MILLISECONDS) + } catch (e: TimeoutException) { + handleTimeout(methodName, testThread, annotation) + } catch (e: ExecutionException) { + throw e.cause ?: e + } finally { + DebugProbes.uninstall() + } + } + + private fun handleTimeout(methodName: String, testThread: Thread, annotation: CoroutinesTimeout): Nothing { + val units = + if (annotation.testTimeoutMs % 1000 == 0L) + "${annotation.testTimeoutMs / 1000} seconds" + else "$annotation.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 + */ + val exception = createTimeoutException(testThread, annotation.testTimeoutMs) + cancelIfNecessary(annotation.cancelOnTimeout) + // If timed out test throws an exception, we can't do much except ignoring it + throw exception + } + + private fun cancelIfNecessary(cancelOnTimeout: Boolean) { + if (cancelOnTimeout) { + DebugProbes.dumpCoroutinesInfo().forEach { + it.job?.cancel() + } + } + } + + private fun createTimeoutException(thread: Thread, testTimeoutMs: Long): Exception { + val stackTrace = thread.stackTrace + val exception = TestTimedOutException(testTimeoutMs, TimeUnit.MILLISECONDS) + exception.stackTrace = stackTrace + thread.interrupt() + return exception + } +} \ No newline at end of file diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt new file mode 100644 index 0000000000..6f2b725c6f --- /dev/null +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt @@ -0,0 +1,22 @@ +/* + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package junit5 +import kotlinx.coroutines.debug.junit5.* +import org.junit.jupiter.api.* + +@CoroutinesTimeout(6) +class CoroutinesTimeoutTest { + + @CoroutinesTimeout(5) + @Test + fun test() { + + } + + @Test + fun test2() { + + } +} From cd7afaf216993addc325cc38b47e27c8174705e6 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 16 Oct 2020 10:42:50 +0300 Subject: [PATCH 02/18] Fix debug probes not being installed --- .../src/junit5/CoroutinesTimeoutExtension.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeoutExtension.kt index 64626bcbce..6abd79d0f0 100644 --- a/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeoutExtension.kt @@ -39,7 +39,7 @@ public class CoroutinesTimeoutExtension: InvocationInterceptor { }.orElseGet { throw UnsupportedOperationException("CoroutinesTimeoutExtension should not be used directly; annotate the test class or method with CoroutinesTimeout instead.") } - invocation.proceed() + interceptInvocation(invocation, invocationContext.executable.name, annotation) } override fun interceptAfterAllMethod( @@ -107,6 +107,7 @@ public class CoroutinesTimeoutExtension: InvocationInterceptor { invocation.proceed() } val testThread = Thread(testResult, "Timeout test thread").apply { isDaemon = true } + DebugProbes.install() try { testThread.start() // Await until test is started to take only test execution time into account From 531d6a1269f50ecf837734afac6e8dd6051288fb Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 13 Nov 2020 16:59:10 +0300 Subject: [PATCH 03/18] Extract common code for JUnit4 and JUnit5 CoroutinesTimeout --- kotlinx-coroutines-debug/build.gradle | 10 +- .../src/junit/CoroutinesTimeoutImpl.kt | 82 ++++++++++++ .../{ => junit}/junit4/CoroutinesTimeout.kt | 0 .../junit4/CoroutinesTimeoutStatement.kt | 27 ++++ .../{ => junit}/junit5/CoroutinesTimeout.kt | 0 .../junit5/CoroutinesTimeoutExtension.kt | 121 ++++++------------ .../src/junit4/CoroutinesTimeoutStatement.kt | 87 ------------- .../junit5/CoroutinesTimeoutSimpleTest.kt | 61 +++++++++ .../test/junit5/CoroutinesTimeoutTest.kt | 43 +++++-- 9 files changed, 245 insertions(+), 186 deletions(-) create mode 100644 kotlinx-coroutines-debug/src/junit/CoroutinesTimeoutImpl.kt rename kotlinx-coroutines-debug/src/{ => junit}/junit4/CoroutinesTimeout.kt (100%) create mode 100644 kotlinx-coroutines-debug/src/junit/junit4/CoroutinesTimeoutStatement.kt rename kotlinx-coroutines-debug/src/{ => junit}/junit5/CoroutinesTimeout.kt (100%) rename kotlinx-coroutines-debug/src/{ => junit}/junit5/CoroutinesTimeoutExtension.kt (52%) delete mode 100644 kotlinx-coroutines-debug/src/junit4/CoroutinesTimeoutStatement.kt create mode 100644 kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutSimpleTest.kt diff --git a/kotlinx-coroutines-debug/build.gradle b/kotlinx-coroutines-debug/build.gradle index a9f8876511..3bc018036a 100644 --- a/kotlinx-coroutines-debug/build.gradle +++ b/kotlinx-coroutines-debug/build.gradle @@ -18,16 +18,12 @@ configurations { configureKotlinJvmPlatform(shadow) } -test { - useJUnitPlatform() -} - dependencies { compileOnly "junit:junit:$junit_version" compileOnly "org.junit.jupiter:junit-jupiter-api:$junit5_version" testCompile "org.junit.jupiter:junit-jupiter-api:$junit5_version" testCompile "org.junit.jupiter:junit-jupiter-engine:$junit5_version" - testRuntimeOnly "org.junit.vintage:junit-vintage-engine:$junit5_version" + testCompile "org.junit.platform:junit-platform-testkit:1.7.0" shadowDeps "net.bytebuddy:byte-buddy:$byte_buddy_version" shadowDeps "net.bytebuddy:byte-buddy-agent:$byte_buddy_version" compileOnly "io.projectreactor.tools:blockhound:$blockhound_version" @@ -46,6 +42,10 @@ if (rootProject.ext.jvm_ir_enabled) { } } +java { + disableAutoTargetJvm() +} + jar { manifest { attributes "Premain-Class": "kotlinx.coroutines.debug.AgentPremain" diff --git a/kotlinx-coroutines-debug/src/junit/CoroutinesTimeoutImpl.kt b/kotlinx-coroutines-debug/src/junit/CoroutinesTimeoutImpl.kt new file mode 100644 index 0000000000..55db88e3f5 --- /dev/null +++ b/kotlinx-coroutines-debug/src/junit/CoroutinesTimeoutImpl.kt @@ -0,0 +1,82 @@ +/* + * 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 org.junit.runners.model.* +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 runWithTimeoutDumpingCoroutines( + methodName: String, + testTimeoutMs: Long, + cancelOnTimeout: Boolean, + 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) + } catch (e: ExecutionException) { + throw e.cause ?: e + } +} + +private fun handleTimeout(testThread: Thread, methodName: String, testTimeoutMs: Long, cancelOnTimeout: Boolean): 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 + */ + val exception = createTimeoutException(testThread, testTimeoutMs) + cancelIfNecessary(cancelOnTimeout) + // If timed out test throws an exception, we can't do much except ignoring it + throw exception +} + +private fun cancelIfNecessary(cancelOnTimeout: Boolean) { + if (cancelOnTimeout) { + DebugProbes.dumpCoroutinesInfo().forEach { + it.job?.cancel() + } + } +} + +private fun createTimeoutException(thread: Thread, testTimeoutMs: Long): Exception { + val stackTrace = thread.stackTrace + val exception = TestTimedOutException(testTimeoutMs, TimeUnit.MILLISECONDS) + exception.stackTrace = stackTrace + thread.interrupt() + return exception +} diff --git a/kotlinx-coroutines-debug/src/junit4/CoroutinesTimeout.kt b/kotlinx-coroutines-debug/src/junit/junit4/CoroutinesTimeout.kt similarity index 100% rename from kotlinx-coroutines-debug/src/junit4/CoroutinesTimeout.kt rename to kotlinx-coroutines-debug/src/junit/junit4/CoroutinesTimeout.kt diff --git a/kotlinx-coroutines-debug/src/junit/junit4/CoroutinesTimeoutStatement.kt b/kotlinx-coroutines-debug/src/junit/junit4/CoroutinesTimeoutStatement.kt new file mode 100644 index 0000000000..5ba72a4301 --- /dev/null +++ b/kotlinx-coroutines-debug/src/junit/junit4/CoroutinesTimeoutStatement.kt @@ -0,0 +1,27 @@ +/* + * 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.* + +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) { + testStatement.evaluate() + } + } finally { + DebugProbes.uninstall() + } + } +} diff --git a/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeout.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt similarity index 100% rename from kotlinx-coroutines-debug/src/junit5/CoroutinesTimeout.kt rename to kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt diff --git a/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt similarity index 52% rename from kotlinx-coroutines-debug/src/junit5/CoroutinesTimeoutExtension.kt rename to kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index 6abd79d0f0..396b3c79cc 100644 --- a/kotlinx-coroutines-debug/src/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -4,22 +4,14 @@ package kotlinx.coroutines.debug.junit5 -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.debug.DebugProbes -import org.junit.jupiter.api.extension.BeforeAllCallback -import org.junit.jupiter.api.extension.ExtensionConfigurationException +import kotlinx.coroutines.debug.* +import kotlinx.coroutines.debug.runWithTimeoutDumpingCoroutines import org.junit.jupiter.api.extension.ExtensionContext import org.junit.jupiter.api.extension.InvocationInterceptor import org.junit.jupiter.api.extension.ReflectiveInvocationContext -import org.junit.platform.commons.JUnitException import org.junit.platform.commons.support.AnnotationSupport -import org.junit.runner.* import org.junit.runners.model.* -import java.io.ByteArrayOutputStream -import java.io.PrintStream import java.lang.reflect.Method -import java.time.Duration -import java.time.format.DateTimeParseException import java.util.concurrent.CountDownLatch import java.util.concurrent.ExecutionException import java.util.concurrent.FutureTask @@ -33,13 +25,7 @@ public class CoroutinesTimeoutExtension: InvocationInterceptor { invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ) { - val annotation = - AnnotationSupport.findAnnotation(invocationContext.executable, CoroutinesTimeout::class.java).or { - AnnotationSupport.findAnnotation(extensionContext.testClass, CoroutinesTimeout::class.java) - }.orElseGet { - throw UnsupportedOperationException("CoroutinesTimeoutExtension should not be used directly; annotate the test class or method with CoroutinesTimeout instead.") - } - interceptInvocation(invocation, invocationContext.executable.name, annotation) + interceptNormalMethod(invocation, invocationContext, extensionContext) } override fun interceptAfterAllMethod( @@ -47,7 +33,7 @@ public class CoroutinesTimeoutExtension: InvocationInterceptor { invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ) { - interceptLifecycleMethod(invocation, invocationContext, extensionContext) + interceptLifecycleMethod(invocation, invocationContext) } override fun interceptAfterEachMethod( @@ -55,7 +41,7 @@ public class CoroutinesTimeoutExtension: InvocationInterceptor { invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ) { - interceptLifecycleMethod(invocation, invocationContext, extensionContext) + interceptLifecycleMethod(invocation, invocationContext) } override fun interceptBeforeAllMethod( @@ -63,7 +49,7 @@ public class CoroutinesTimeoutExtension: InvocationInterceptor { invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ) { - interceptLifecycleMethod(invocation, invocationContext, extensionContext) + interceptLifecycleMethod(invocation, invocationContext) } override fun interceptBeforeEachMethod( @@ -71,29 +57,46 @@ public class CoroutinesTimeoutExtension: InvocationInterceptor { invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ) { - interceptLifecycleMethod(invocation, invocationContext, extensionContext) + interceptLifecycleMethod(invocation, invocationContext) } - private fun interceptLifecycleMethod( + override fun interceptTestFactoryMethod( + invocation: InvocationInterceptor.Invocation, + invocationContext: ReflectiveInvocationContext, + extensionContext: ExtensionContext + ): T = interceptNormalMethod(invocation, invocationContext, extensionContext) + + override fun interceptTestTemplateMethod( invocation: InvocationInterceptor.Invocation, invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ) { - invocation.proceed() + interceptNormalMethod(invocation, invocationContext, extensionContext) } - override fun interceptTestFactoryMethod( + private fun interceptNormalMethod( invocation: InvocationInterceptor.Invocation, invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext - ): T = invocation.proceed() + ): T { + val annotation = + AnnotationSupport.findAnnotation(invocationContext.executable, CoroutinesTimeout::class.java).or { + AnnotationSupport.findAnnotation(extensionContext.testClass, CoroutinesTimeout::class.java) + }.orElseGet { + throw UnsupportedOperationException("CoroutinesTimeoutExtension should not be used directly; annotate the test class or method with CoroutinesTimeout instead.") + } + return interceptInvocation(invocation, invocationContext.executable.name, annotation) + } - override fun interceptTestTemplateMethod( + private fun interceptLifecycleMethod( invocation: InvocationInterceptor.Invocation, - invocationContext: ReflectiveInvocationContext, - extensionContext: ExtensionContext + invocationContext: ReflectiveInvocationContext ) { - invocation.proceed() + val annotation = + AnnotationSupport.findAnnotation(invocationContext.executable, CoroutinesTimeout::class.java).orElseGet { + throw UnsupportedOperationException("CoroutinesTimeoutExtension should not be used directly; annotate the test class or method with CoroutinesTimeout instead.") + } + interceptInvocation(invocation, invocationContext.executable.name, annotation) } private fun interceptInvocation( @@ -101,64 +104,14 @@ public class CoroutinesTimeoutExtension: InvocationInterceptor { methodName: String, annotation: CoroutinesTimeout ): T { - val testStartedLatch = CountDownLatch(1) - val testResult = FutureTask { - testStartedLatch.countDown() - invocation.proceed() - } - val testThread = Thread(testResult, "Timeout test thread").apply { isDaemon = true } + DebugProbes.enableCreationStackTraces = annotation.enableCoroutineCreationStackTraces DebugProbes.install() - try { - testThread.start() - // Await until test is started to take only test execution time into account - testStartedLatch.await() - return testResult.get(annotation.testTimeoutMs, TimeUnit.MILLISECONDS) - } catch (e: TimeoutException) { - handleTimeout(methodName, testThread, annotation) - } catch (e: ExecutionException) { - throw e.cause ?: e + return try { + runWithTimeoutDumpingCoroutines(methodName, annotation.testTimeoutMs, annotation.cancelOnTimeout) { + invocation.proceed() + } } finally { DebugProbes.uninstall() } } - - private fun handleTimeout(methodName: String, testThread: Thread, annotation: CoroutinesTimeout): Nothing { - val units = - if (annotation.testTimeoutMs % 1000 == 0L) - "${annotation.testTimeoutMs / 1000} seconds" - else "$annotation.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 - */ - val exception = createTimeoutException(testThread, annotation.testTimeoutMs) - cancelIfNecessary(annotation.cancelOnTimeout) - // If timed out test throws an exception, we can't do much except ignoring it - throw exception - } - - private fun cancelIfNecessary(cancelOnTimeout: Boolean) { - if (cancelOnTimeout) { - DebugProbes.dumpCoroutinesInfo().forEach { - it.job?.cancel() - } - } - } - - private fun createTimeoutException(thread: Thread, testTimeoutMs: Long): Exception { - val stackTrace = thread.stackTrace - val exception = TestTimedOutException(testTimeoutMs, TimeUnit.MILLISECONDS) - exception.stackTrace = stackTrace - thread.interrupt() - return exception - } } \ No newline at end of file diff --git a/kotlinx-coroutines-debug/src/junit4/CoroutinesTimeoutStatement.kt b/kotlinx-coroutines-debug/src/junit4/CoroutinesTimeoutStatement.kt deleted file mode 100644 index 4baf409de8..0000000000 --- a/kotlinx-coroutines-debug/src/junit4/CoroutinesTimeoutStatement.kt +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Copyright 2016-2021 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( - testStatement: Statement, - private val testDescription: Description, - private val testTimeoutMs: Long, - private val cancelOnTimeout: Boolean = false -) : Statement() { - - private val testStartedLatch = CountDownLatch(1) - - private val testResult = FutureTask { - testStartedLatch.countDown() - testStatement.evaluate() - } - - /* - * 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 - */ - private val testThread = Thread(testResult, "Timeout test thread").apply { isDaemon = true } - - override fun evaluate() { - try { - testThread.start() - // Await until test is started to take only test execution time into account - testStartedLatch.await() - testResult.get(testTimeoutMs, TimeUnit.MILLISECONDS) - return - } catch (e: TimeoutException) { - handleTimeout(testDescription) - } catch (e: ExecutionException) { - throw e.cause ?: e - } finally { - DebugProbes.uninstall() - } - } - - private fun handleTimeout(description: Description) { - val units = - if (testTimeoutMs % 1000 == 0L) - "${testTimeoutMs / 1000} seconds" - else "$testTimeoutMs milliseconds" - - System.err.println("\nTest ${description.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 - */ - val exception = createTimeoutException(testThread) - cancelIfNecessary() - // If timed out test throws an exception, we can't do much except ignoring it - throw exception - } - - private fun cancelIfNecessary() { - if (cancelOnTimeout) { - DebugProbes.dumpCoroutinesInfo().forEach { - it.job?.cancel() - } - } - } - - private fun createTimeoutException(thread: Thread): Exception { - val stackTrace = thread.stackTrace - val exception = TestTimedOutException(testTimeoutMs, TimeUnit.MILLISECONDS) - exception.stackTrace = stackTrace - thread.interrupt() - return exception - } -} diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutSimpleTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutSimpleTest.kt new file mode 100644 index 0000000000..1b95875716 --- /dev/null +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutSimpleTest.kt @@ -0,0 +1,61 @@ +/* + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package junit5 +import kotlinx.coroutines.* +import kotlinx.coroutines.debug.junit5.* +import org.junit.jupiter.api.* + +/** + * Tests the basic usage of [CoroutinesTimeout] on classes and test methods. + * + * This test class is not intended to be run manually. Instead, use [CoroutinesTimeoutTest] as the entry point. + */ +@TestMethodOrder(MethodOrderer.OrderAnnotation::class) +@CoroutinesTimeout(100) +class CoroutinesTimeoutSimpleTest { + + @Test + @Order(1) + fun usesClassTimeout1() { + runBlocking { + delay(150) + } + } + + @CoroutinesTimeout(1000) + @Test + @Order(2) + fun ignoresClassTimeout() { + runBlocking { + delay(150) + } + } + + @CoroutinesTimeout(200) + @Test + @Order(3) + fun usesMethodTimeout() { + runBlocking { + delay(300) + } + } + + @Test + @Order(4) + fun fitsInClassTimeout() { + runBlocking { + delay(50) + } + } + + @Test + @Order(5) + fun usesClassTimeout2() { + runBlocking { + delay(150) + } + } + +} diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt index 6f2b725c6f..0cfef30671 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt @@ -3,20 +3,43 @@ */ package junit5 -import kotlinx.coroutines.debug.junit5.* -import org.junit.jupiter.api.* -@CoroutinesTimeout(6) +import org.assertj.core.api.* +import org.junit.* +import org.junit.platform.engine.discovery.DiscoverySelectors.* +import org.junit.platform.testkit.engine.* +import org.junit.platform.testkit.engine.EventConditions.* +import org.junit.platform.testkit.engine.TestExecutionResultConditions.* +import org.junit.runners.model.* + class CoroutinesTimeoutTest { - @CoroutinesTimeout(5) + // note that this test is run using JUnit4 in order not to mix the testing systems. @Test - fun test() { - + fun testCoroutinesTimeout() { + EngineTestKit.engine("junit-jupiter") + .selectors(selectClass(CoroutinesTimeoutSimpleTest::class.java)) + .execute() + .testEvents() + .assertThatEvents() + .testFinishedSuccessfully("ignoresClassTimeout") + .testFinishedSuccessfully("fitsInClassTimeout") + .testTimedOut("usesClassTimeout1", 100) + .testTimedOut("usesClassTimeout2", 100) + .testTimedOut("usesMethodTimeout", 200) } +} - @Test - fun test2() { +private fun ListAssert.testFinishedSuccessfully(testName: String): ListAssert = + haveExactly(1, event( + test(testName), + finishedSuccessfully() + )) - } -} +private fun ListAssert.testTimedOut(testName: String, after: Int): ListAssert = + haveExactly(1, event( + test(testName), + finishedWithFailure(instanceOf(TestTimedOutException::class.java), message { + it.contains(Regex("\\b$after\\b")) + }) + )) From 77e646f0831187f421154359126b54fa57890132 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 13 Nov 2020 17:25:09 +0300 Subject: [PATCH 04/18] Generalize exceptions thrown from 'runWithTimeoutDumpingCoroutines' --- .../src/junit/CoroutinesTimeoutImpl.kt | 19 +++++++++---------- .../junit4/CoroutinesTimeoutStatement.kt | 5 ++++- .../junit5/CoroutinesTimeoutExtension.kt | 12 +++++------- .../test/junit5/CoroutinesTimeoutTest.kt | 8 ++++---- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/kotlinx-coroutines-debug/src/junit/CoroutinesTimeoutImpl.kt b/kotlinx-coroutines-debug/src/junit/CoroutinesTimeoutImpl.kt index 55db88e3f5..06a84a5bf9 100644 --- a/kotlinx-coroutines-debug/src/junit/CoroutinesTimeoutImpl.kt +++ b/kotlinx-coroutines-debug/src/junit/CoroutinesTimeoutImpl.kt @@ -4,7 +4,6 @@ package kotlinx.coroutines.debug -import org.junit.runners.model.* import java.util.concurrent.* /** @@ -17,6 +16,7 @@ internal inline fun runWithTimeoutDumpingCoroutines( methodName: String, testTimeoutMs: Long, cancelOnTimeout: Boolean, + initCancellationException: () -> Throwable, crossinline invocation: () -> T ): T { val testStartedLatch = CountDownLatch(1) @@ -35,13 +35,14 @@ internal inline fun runWithTimeoutDumpingCoroutines( testStartedLatch.await() return testResult.get(testTimeoutMs, TimeUnit.MILLISECONDS) } catch (e: TimeoutException) { - handleTimeout(testThread, methodName, testTimeoutMs, cancelOnTimeout) + handleTimeout(testThread, methodName, testTimeoutMs, cancelOnTimeout, initCancellationException()) } catch (e: ExecutionException) { throw e.cause ?: e } } -private fun handleTimeout(testThread: Thread, methodName: String, testTimeoutMs: Long, cancelOnTimeout: Boolean): Nothing { +private fun handleTimeout(testThread: Thread, methodName: String, testTimeoutMs: Long, cancelOnTimeout: Boolean, + cancellationException: Throwable): Nothing { val units = if (testTimeoutMs % 1000 == 0L) "${testTimeoutMs / 1000} seconds" @@ -59,10 +60,11 @@ private fun handleTimeout(testThread: Thread, methodName: String, testTimeoutMs: * 2) Cancel all coroutines via debug agent API (changing system state!) * 3) Throw created exception */ - val exception = createTimeoutException(testThread, testTimeoutMs) + cancellationException.attachStacktraceFrom(testThread) + testThread.interrupt() cancelIfNecessary(cancelOnTimeout) // If timed out test throws an exception, we can't do much except ignoring it - throw exception + throw cancellationException } private fun cancelIfNecessary(cancelOnTimeout: Boolean) { @@ -73,10 +75,7 @@ private fun cancelIfNecessary(cancelOnTimeout: Boolean) { } } -private fun createTimeoutException(thread: Thread, testTimeoutMs: Long): Exception { +private fun Throwable.attachStacktraceFrom(thread: Thread) { val stackTrace = thread.stackTrace - val exception = TestTimedOutException(testTimeoutMs, TimeUnit.MILLISECONDS) - exception.stackTrace = stackTrace - thread.interrupt() - return exception + this.stackTrace = stackTrace } diff --git a/kotlinx-coroutines-debug/src/junit/junit4/CoroutinesTimeoutStatement.kt b/kotlinx-coroutines-debug/src/junit/junit4/CoroutinesTimeoutStatement.kt index 5ba72a4301..aa6b8df243 100644 --- a/kotlinx-coroutines-debug/src/junit/junit4/CoroutinesTimeoutStatement.kt +++ b/kotlinx-coroutines-debug/src/junit/junit4/CoroutinesTimeoutStatement.kt @@ -7,6 +7,7 @@ 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, @@ -17,7 +18,9 @@ internal class CoroutinesTimeoutStatement( override fun evaluate() { try { - runWithTimeoutDumpingCoroutines(testDescription.methodName, testTimeoutMs, cancelOnTimeout) { + runWithTimeoutDumpingCoroutines(testDescription.methodName, testTimeoutMs, cancelOnTimeout, + { TestTimedOutException(testTimeoutMs, TimeUnit.MILLISECONDS) }) + { testStatement.evaluate() } } finally { diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index 396b3c79cc..a372614a0e 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -10,13 +10,9 @@ import org.junit.jupiter.api.extension.ExtensionContext import org.junit.jupiter.api.extension.InvocationInterceptor import org.junit.jupiter.api.extension.ReflectiveInvocationContext import org.junit.platform.commons.support.AnnotationSupport -import org.junit.runners.model.* import java.lang.reflect.Method -import java.util.concurrent.CountDownLatch -import java.util.concurrent.ExecutionException -import java.util.concurrent.FutureTask -import java.util.concurrent.TimeUnit -import java.util.concurrent.TimeoutException + +public class CoroutinesTimeoutException(public val timeoutMs: Long): Exception("test timed out ofter $timeoutMs ms") public class CoroutinesTimeoutExtension: InvocationInterceptor { @@ -107,7 +103,9 @@ public class CoroutinesTimeoutExtension: InvocationInterceptor { DebugProbes.enableCreationStackTraces = annotation.enableCoroutineCreationStackTraces DebugProbes.install() return try { - runWithTimeoutDumpingCoroutines(methodName, annotation.testTimeoutMs, annotation.cancelOnTimeout) { + runWithTimeoutDumpingCoroutines(methodName, annotation.testTimeoutMs, annotation.cancelOnTimeout, + { CoroutinesTimeoutException(annotation.testTimeoutMs) } + ) { invocation.proceed() } } finally { diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt index 0cfef30671..534473d423 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt @@ -4,6 +4,7 @@ package junit5 +import kotlinx.coroutines.debug.junit5.* import org.assertj.core.api.* import org.junit.* import org.junit.platform.engine.discovery.DiscoverySelectors.* @@ -36,10 +37,9 @@ private fun ListAssert.testFinishedSuccessfully(testName: String): ListAs finishedSuccessfully() )) -private fun ListAssert.testTimedOut(testName: String, after: Int): ListAssert = +private fun ListAssert.testTimedOut(testName: String, after: Long): ListAssert = haveExactly(1, event( test(testName), - finishedWithFailure(instanceOf(TestTimedOutException::class.java), message { - it.contains(Regex("\\b$after\\b")) - }) + finishedWithFailure(Condition({ it is CoroutinesTimeoutException && it.timeoutMs == after }, + "is CoroutinesTimeoutException($after)")) )) From 1120924c902765c579e1e753bc55f2c731c50714 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 13 Nov 2020 17:52:27 +0300 Subject: [PATCH 05/18] Fix API check --- kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api b/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api index 8120454c33..dcc6c1c3b5 100644 --- a/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api +++ b/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api @@ -67,6 +67,11 @@ public abstract interface annotation class kotlinx/coroutines/debug/junit5/Corou public abstract fun testTimeoutMs ()J } +public final class kotlinx/coroutines/debug/junit5/CoroutinesTimeoutException : java/lang/Exception { + public fun (J)V + public final fun getTimeoutMs ()J +} + public final class kotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension : org/junit/jupiter/api/extension/InvocationInterceptor { public fun ()V public fun interceptAfterAllMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V From da8edd44e06bc31d407d651870cbd908430ae6fd Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 16 Nov 2020 11:45:49 +0300 Subject: [PATCH 06/18] Test inheritance of CoroutinesTimeout for JUnit5 --- .../CoroutinesTimeoutInheritanceTest.kt | 61 +++++++++++++++ .../test/junit5/CoroutinesTimeoutTest.kt | 75 ++++++++++++++++--- 2 files changed, 126 insertions(+), 10 deletions(-) create mode 100644 kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutInheritanceTest.kt diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutInheritanceTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutInheritanceTest.kt new file mode 100644 index 0000000000..5647dd1ce3 --- /dev/null +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutInheritanceTest.kt @@ -0,0 +1,61 @@ +/* + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package junit5 + +import kotlinx.coroutines.* +import kotlinx.coroutines.debug.junit5.* +import org.junit.jupiter.api.* + +/** + * Tests that [CoroutinesTimeout] is inherited. + * + * This test class is not intended to be run manually. Instead, use [CoroutinesTimeoutTest] as the entry point. + */ +class CoroutinesTimeoutInheritanceTest { + + @CoroutinesTimeout(100) + open class Base + + @TestMethodOrder(MethodOrderer.OrderAnnotation::class) + class InheritedWithNoTimeout: Base() { + + @Test + @Order(1) + fun usesBaseClassTimeout() = runBlocking { + delay(1000) + } + + @CoroutinesTimeout(300) + @Test + @Order(2) + fun methodOverridesBaseClassTimeoutWithGreaterTimeout() = runBlocking { + delay(200) + } + + @CoroutinesTimeout(10) + @Test + @Order(3) + fun methodOverridesBaseClassTimeoutWithLesserTimeout() = runBlocking { + delay(50) + } + + } + + @CoroutinesTimeout(300) + class InheritedWithGreaterTimeout : TestBase() { + + @Test + fun classOverridesBaseClassTimeout1() = runBlocking { + delay(200) + } + + @Test + fun classOverridesBaseClassTimeout2() = runBlocking { + delay(400) + } + + } + +} \ No newline at end of file diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt index 534473d423..9a55de09e6 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt @@ -6,28 +6,66 @@ package junit5 import kotlinx.coroutines.debug.junit5.* import org.assertj.core.api.* -import org.junit.* +import org.junit.Assert.* +import org.junit.Test +import org.junit.platform.engine.* import org.junit.platform.engine.discovery.DiscoverySelectors.* import org.junit.platform.testkit.engine.* import org.junit.platform.testkit.engine.EventConditions.* -import org.junit.platform.testkit.engine.TestExecutionResultConditions.* -import org.junit.runners.model.* +import java.io.* +// note that these tests are run using JUnit4 in order not to mix the testing systems. class CoroutinesTimeoutTest { - // note that this test is run using JUnit4 in order not to mix the testing systems. @Test fun testCoroutinesTimeout() { - EngineTestKit.engine("junit-jupiter") - .selectors(selectClass(CoroutinesTimeoutSimpleTest::class.java)) - .execute() - .testEvents() - .assertThatEvents() + val capturedOut = ByteArrayOutputStream() + eventsForSelector(selectClass(CoroutinesTimeoutSimpleTest::class.java), capturedOut) .testFinishedSuccessfully("ignoresClassTimeout") .testFinishedSuccessfully("fitsInClassTimeout") .testTimedOut("usesClassTimeout1", 100) - .testTimedOut("usesClassTimeout2", 100) .testTimedOut("usesMethodTimeout", 200) + .testTimedOut("usesClassTimeout2", 100) + assertEquals(capturedOut.toString(), 3, countDumps(capturedOut)) + } + + @Test + fun testCoroutinesTimeoutInheritanceWithNoTimeoutInDerived() { + val capturedOut = ByteArrayOutputStream() + eventsForSelector(selectClass(CoroutinesTimeoutInheritanceTest.InheritedWithNoTimeout::class.java), capturedOut) + .testFinishedSuccessfully("methodOverridesBaseClassTimeoutWithGreaterTimeout") + .testTimedOut("usesBaseClassTimeout", 100) + .testTimedOut("methodOverridesBaseClassTimeoutWithLesserTimeout", 10) + assertEquals(capturedOut.toString(), 2, countDumps(capturedOut)) + } + + @Test + fun testCoroutinesTimeoutInheritanceWithGreaterTimeoutInDerived() { + val capturedOut = ByteArrayOutputStream() + eventsForSelector( + selectClass(CoroutinesTimeoutInheritanceTest.InheritedWithGreaterTimeout::class.java), + capturedOut + ) + .testFinishedSuccessfully("classOverridesBaseClassTimeout1") + .testTimedOut("classOverridesBaseClassTimeout2", 300) + assertEquals(capturedOut.toString(), 1, countDumps(capturedOut)) + } +} + +private fun eventsForSelector(selector: DiscoverySelector, capturedOut: OutputStream): ListAssert { + val systemOut: PrintStream = System.out + val systemErr: PrintStream = System.err + return try { + System.setOut(PrintStream(capturedOut)) + System.setErr(PrintStream(capturedOut)) + EngineTestKit.engine("junit-jupiter") + .selectors(selector) + .execute() + .testEvents() + .assertThatEvents() + } finally { + System.setOut(systemOut) + System.setErr(systemErr) } } @@ -43,3 +81,20 @@ private fun ListAssert.testTimedOut(testName: String, after: Long): ListA finishedWithFailure(Condition({ it is CoroutinesTimeoutException && it.timeoutMs == after }, "is CoroutinesTimeoutException($after)")) )) + +/** Counts the number of occurrences of "Coroutines dump" in [capturedOut] */ +private fun countDumps(capturedOut: ByteArrayOutputStream): Int { + var result = 0 + val outStr = capturedOut.toString() + val header = "Coroutines dump" + var i = 0 + while (i < outStr.length - header.length) { + if (outStr.substring(i, i + header.length) == header) { + result += 1 + i += header.length + } else { + i += 1 + } + } + return result +} \ No newline at end of file From 140ddfbb236addba68a586f423a07689f76b4702 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Tue, 17 Nov 2020 17:50:03 +0300 Subject: [PATCH 07/18] Fix DebugProbes installation in CoroutinesTimeout for JUnit 5 As implemented, now DebugProbes will be installed before the first constructor of a test class that uses CoroutinesTimeout is invoked, and uninstalled during the cleanup of the extension. --- gradle.properties | 2 +- kotlinx-coroutines-debug/build.gradle | 2 ++ .../junit5/CoroutinesTimeoutExtension.kt | 31 ++++++++++++++----- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/gradle.properties b/gradle.properties index 46ec77cab9..15935ab114 100644 --- a/gradle.properties +++ b/gradle.properties @@ -9,7 +9,7 @@ kotlin_version=1.5.0-RC # Dependencies junit_version=4.12 -junit5_version=5.5.0 +junit5_version=5.7.0 atomicfu_version=0.15.2 knit_version=0.2.3 html_version=0.7.2 diff --git a/kotlinx-coroutines-debug/build.gradle b/kotlinx-coroutines-debug/build.gradle index 3bc018036a..17b378e62b 100644 --- a/kotlinx-coroutines-debug/build.gradle +++ b/kotlinx-coroutines-debug/build.gradle @@ -43,6 +43,8 @@ if (rootProject.ext.jvm_ir_enabled) { } java { + /* This is needed to be able to run JUnit5 tests. Otherwise, Gradle complains that it can't find the + JVM1.6-compatible version of the `junit-platform-testkit` artifact. */ disableAutoTargetJvm() } diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index a372614a0e..69130ad0a3 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -6,15 +6,33 @@ package kotlinx.coroutines.debug.junit5 import kotlinx.coroutines.debug.* import kotlinx.coroutines.debug.runWithTimeoutDumpingCoroutines -import org.junit.jupiter.api.extension.ExtensionContext -import org.junit.jupiter.api.extension.InvocationInterceptor -import org.junit.jupiter.api.extension.ReflectiveInvocationContext +import org.junit.jupiter.api.extension.* import org.junit.platform.commons.support.AnnotationSupport -import java.lang.reflect.Method +import java.lang.reflect.* public class CoroutinesTimeoutException(public val timeoutMs: Long): Exception("test timed out ofter $timeoutMs ms") -public class CoroutinesTimeoutExtension: InvocationInterceptor { +public class CoroutinesTimeoutExtension internal constructor(): InvocationInterceptor { + + private companion object { + val NAMESPACE = ExtensionContext.Namespace.create("kotlinx", "coroutines", "debug", "junit5", + "CoroutinesTimeout") + } + + override fun interceptTestClassConstructor( + invocation: InvocationInterceptor.Invocation, + invocationContext: ReflectiveInvocationContext>, + extensionContext: ExtensionContext + ): T { + if (extensionContext.getStore(NAMESPACE)["debugProbes"] == null) { + DebugProbes.install() + val uninstall: ExtensionContext.Store.CloseableResource = ExtensionContext.Store.CloseableResource { + DebugProbes.uninstall() + } + extensionContext.getStore(NAMESPACE).put("debugProbes", uninstall) + } + return invocation.proceed() + } override fun interceptTestMethod( invocation: InvocationInterceptor.Invocation, @@ -101,15 +119,12 @@ public class CoroutinesTimeoutExtension: InvocationInterceptor { annotation: CoroutinesTimeout ): T { DebugProbes.enableCreationStackTraces = annotation.enableCoroutineCreationStackTraces - DebugProbes.install() return try { runWithTimeoutDumpingCoroutines(methodName, annotation.testTimeoutMs, annotation.cancelOnTimeout, { CoroutinesTimeoutException(annotation.testTimeoutMs) } ) { invocation.proceed() } - } finally { - DebugProbes.uninstall() } } } \ No newline at end of file From 1a4ad3bfd4b7d39ff7841ee27ced841613390a03 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Thu, 19 Nov 2020 11:10:41 +0300 Subject: [PATCH 08/18] Disable configuring enableCoroutineCreationStacktrace for annotations Instead, another interface is implemented that resembles the one provided for JUnit4 and does allow such configuration. --- .../src/junit/junit5/CoroutinesTimeout.kt | 3 +- .../junit5/CoroutinesTimeoutExtension.kt | 48 +++++++++++-------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt index 876dc5e7e5..8c908eaa6c 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt @@ -15,6 +15,5 @@ import java.lang.annotation.* @Inherited public annotation class CoroutinesTimeout( val testTimeoutMs: Long, - val cancelOnTimeout: Boolean = false, - val enableCoroutineCreationStackTraces: Boolean = true + val cancelOnTimeout: Boolean = false ) \ No newline at end of file diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index 69130ad0a3..1d2cce0c58 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -12,11 +12,23 @@ import java.lang.reflect.* public class CoroutinesTimeoutException(public val timeoutMs: Long): Exception("test timed out ofter $timeoutMs ms") -public class CoroutinesTimeoutExtension internal constructor(): InvocationInterceptor { - - private companion object { - val NAMESPACE = ExtensionContext.Namespace.create("kotlinx", "coroutines", "debug", "junit5", - "CoroutinesTimeout") +public class CoroutinesTimeoutExtension internal constructor( + private val enableCoroutineCreationStackTraces: Boolean = true, + private val timeoutMs: Long? = null, + private val cancelOnTimeout: Boolean? = null): InvocationInterceptor +{ + public constructor(timeoutMs: Long, cancelOnTimeout: Boolean = false, + enableCoroutineCreationStackTraces: Boolean = true): + this(enableCoroutineCreationStackTraces, timeoutMs, cancelOnTimeout) + + public companion object { + private val NAMESPACE: ExtensionContext.Namespace = + ExtensionContext.Namespace.create("kotlinx", "coroutines", "debug", "junit5", "CoroutinesTimeout") + + @JvmOverloads + public fun seconds(timeout: Int, cancelOnTimeout: Boolean = false, + enableCoroutineCreationStackTraces: Boolean = true): CoroutinesTimeoutExtension = + CoroutinesTimeoutExtension(enableCoroutineCreationStackTraces, timeout.toLong() * 1000, cancelOnTimeout) } override fun interceptTestClassConstructor( @@ -24,12 +36,15 @@ public class CoroutinesTimeoutExtension internal constructor(): InvocationInterc invocationContext: ReflectiveInvocationContext>, extensionContext: ExtensionContext ): T { - if (extensionContext.getStore(NAMESPACE)["debugProbes"] == null) { + val store: ExtensionContext.Store = extensionContext.getStore(NAMESPACE) + if (store["debugProbes"] == null) { + /** no [DebugProbes] uninstaller is present, so this must be the first test that this instance of + * [CoroutinesTimeoutExtension] runs. Install the [DebugProbes]. */ + DebugProbes.enableCreationStackTraces = enableCoroutineCreationStackTraces DebugProbes.install() - val uninstall: ExtensionContext.Store.CloseableResource = ExtensionContext.Store.CloseableResource { - DebugProbes.uninstall() - } - extensionContext.getStore(NAMESPACE).put("debugProbes", uninstall) + /** put a fake resource into this extensions's store so that JUnit cleans it up, uninstalling the + * [DebugProbes] after this extension instance is no longer needed. **/ + store.put("debugProbes", ExtensionContext.Store.CloseableResource { DebugProbes.uninstall() }) } return invocation.proceed() } @@ -117,14 +132,7 @@ public class CoroutinesTimeoutExtension internal constructor(): InvocationInterc invocation: InvocationInterceptor.Invocation, methodName: String, annotation: CoroutinesTimeout - ): T { - DebugProbes.enableCreationStackTraces = annotation.enableCoroutineCreationStackTraces - return try { - runWithTimeoutDumpingCoroutines(methodName, annotation.testTimeoutMs, annotation.cancelOnTimeout, - { CoroutinesTimeoutException(annotation.testTimeoutMs) } - ) { - invocation.proceed() - } - } - } + ): T = + runWithTimeoutDumpingCoroutines(methodName, annotation.testTimeoutMs, annotation.cancelOnTimeout, + { CoroutinesTimeoutException(annotation.testTimeoutMs) }, { invocation.proceed()}) } \ No newline at end of file From a152ac6e40fc9352a8177a23c1de0558ebb49658 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Thu, 19 Nov 2020 11:31:25 +0300 Subject: [PATCH 09/18] Handle annotation on outher classes for JUnit5 CoroutinesTimeout --- .../src/junit/junit5/CoroutinesTimeoutExtension.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index 1d2cce0c58..d4cedfbfa0 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -9,6 +9,7 @@ import kotlinx.coroutines.debug.runWithTimeoutDumpingCoroutines import org.junit.jupiter.api.extension.* import org.junit.platform.commons.support.AnnotationSupport import java.lang.reflect.* +import java.util.* public class CoroutinesTimeoutException(public val timeoutMs: Long): Exception("test timed out ofter $timeoutMs ms") @@ -103,6 +104,11 @@ public class CoroutinesTimeoutExtension internal constructor( interceptNormalMethod(invocation, invocationContext, extensionContext) } + private fun Class.coroutinesTimeoutAnnotation(): Optional = + AnnotationSupport.findAnnotation(this, CoroutinesTimeout::class.java).or { + enclosingClass?.coroutinesTimeoutAnnotation() ?: Optional.empty() + } + private fun interceptNormalMethod( invocation: InvocationInterceptor.Invocation, invocationContext: ReflectiveInvocationContext, @@ -110,7 +116,7 @@ public class CoroutinesTimeoutExtension internal constructor( ): T { val annotation = AnnotationSupport.findAnnotation(invocationContext.executable, CoroutinesTimeout::class.java).or { - AnnotationSupport.findAnnotation(extensionContext.testClass, CoroutinesTimeout::class.java) + extensionContext.testClass.flatMap { it.coroutinesTimeoutAnnotation() } }.orElseGet { throw UnsupportedOperationException("CoroutinesTimeoutExtension should not be used directly; annotate the test class or method with CoroutinesTimeout instead.") } From d3117b4f83e8e9e72908e1cf0d428ccc656caf1a Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 20 Nov 2020 16:36:30 +0300 Subject: [PATCH 10/18] Implement registering CoroutinesTimeoutExtension via RegisterExtension --- .../api/kotlinx-coroutines-debug.api | 12 ++- .../junit5/CoroutinesTimeoutExtension.kt | 73 +++++++++++++------ 2 files changed, 62 insertions(+), 23 deletions(-) diff --git a/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api b/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api index dcc6c1c3b5..5677548293 100644 --- a/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api +++ b/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api @@ -63,7 +63,6 @@ public final class kotlinx/coroutines/debug/junit4/CoroutinesTimeout$Companion { public abstract interface annotation class kotlinx/coroutines/debug/junit5/CoroutinesTimeout : java/lang/annotation/Annotation { public abstract fun cancelOnTimeout ()Z - public abstract fun enableCoroutineCreationStackTraces ()Z public abstract fun testTimeoutMs ()J } @@ -73,13 +72,24 @@ public final class kotlinx/coroutines/debug/junit5/CoroutinesTimeoutException : } public final class kotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension : org/junit/jupiter/api/extension/InvocationInterceptor { + public static final field Companion Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension$Companion; public fun ()V + public fun (JZZ)V + public synthetic fun (JZZILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun interceptAfterAllMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V public fun interceptAfterEachMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V public fun interceptBeforeAllMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V public fun interceptBeforeEachMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V + public fun interceptTestClassConstructor (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)Ljava/lang/Object; public fun interceptTestFactoryMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)Ljava/lang/Object; public fun interceptTestMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V public fun interceptTestTemplateMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V } +public final class kotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension$Companion { + public final fun seconds (I)Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension; + public final fun seconds (IZ)Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension; + public final fun seconds (IZZ)Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension; + public static synthetic fun seconds$default (Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension$Companion;IZZILjava/lang/Object;)Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension; +} + diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index d4cedfbfa0..be6ddc4f67 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -63,7 +63,7 @@ public class CoroutinesTimeoutExtension internal constructor( invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ) { - interceptLifecycleMethod(invocation, invocationContext) + interceptLifecycleMethod(invocation, invocationContext, extensionContext) } override fun interceptAfterEachMethod( @@ -71,7 +71,7 @@ public class CoroutinesTimeoutExtension internal constructor( invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ) { - interceptLifecycleMethod(invocation, invocationContext) + interceptLifecycleMethod(invocation, invocationContext, extensionContext) } override fun interceptBeforeAllMethod( @@ -79,7 +79,7 @@ public class CoroutinesTimeoutExtension internal constructor( invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ) { - interceptLifecycleMethod(invocation, invocationContext) + interceptLifecycleMethod(invocation, invocationContext, extensionContext) } override fun interceptBeforeEachMethod( @@ -87,7 +87,7 @@ public class CoroutinesTimeoutExtension internal constructor( invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ) { - interceptLifecycleMethod(invocation, invocationContext) + interceptLifecycleMethod(invocation, invocationContext, extensionContext) } override fun interceptTestFactoryMethod( @@ -109,36 +109,65 @@ public class CoroutinesTimeoutExtension internal constructor( enclosingClass?.coroutinesTimeoutAnnotation() ?: Optional.empty() } - private fun interceptNormalMethod( + private fun interceptMethod( + useClassAnnotation: Boolean, invocation: InvocationInterceptor.Invocation, invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ): T { - val annotation = - AnnotationSupport.findAnnotation(invocationContext.executable, CoroutinesTimeout::class.java).or { - extensionContext.testClass.flatMap { it.coroutinesTimeoutAnnotation() } - }.orElseGet { - throw UnsupportedOperationException("CoroutinesTimeoutExtension should not be used directly; annotate the test class or method with CoroutinesTimeout instead.") + val testAnnotationOptional = + AnnotationSupport.findAnnotation(invocationContext.executable, CoroutinesTimeout::class.java) + val classAnnotationOptional = extensionContext.testClass.flatMap { it.coroutinesTimeoutAnnotation() } + if (timeoutMs != null && cancelOnTimeout != null) { + // this means we @RegisterExtension was used in order to register this extension. + if (testAnnotationOptional.isPresent || classAnnotationOptional.isPresent) { + /* Using annotations creates a separate instance of the extension, which composes in a strange way: both + timeouts are applied. This is at odds with the concept that method-level annotations override the outer + rules and may lead to unexpected outcomes, so we prohibit this. */ + throw UnsupportedOperationException("Using CoroutinesTimeout along with instance field-registered CoroutinesTimeout is prohibited; please use either @RegisterExtension or @CoroutinesTimeout, but not both") } - return interceptInvocation(invocation, invocationContext.executable.name, annotation) + return interceptInvocation(invocation, invocationContext.executable.name, timeoutMs, cancelOnTimeout) + } + /* The extension was registered via an annotation; check that we succeeded in finding the annotation that led to + the extension being registered and taking its parameters. */ + if (testAnnotationOptional.isEmpty && classAnnotationOptional.isEmpty) { + throw UnsupportedOperationException("Timeout was registered with a CoroutinesTimeout annotation, but we were unable to find it. Please report this.") + } + return when { + testAnnotationOptional.isPresent -> { + val annotation = testAnnotationOptional.get() + interceptInvocation(invocation, invocationContext.executable.name, annotation.testTimeoutMs, + annotation.cancelOnTimeout) + } + useClassAnnotation && classAnnotationOptional.isPresent -> { + val annotation = classAnnotationOptional.get() + interceptInvocation(invocation, invocationContext.executable.name, annotation.testTimeoutMs, + annotation.cancelOnTimeout) + } + else -> { + invocation.proceed() + } + } } + private fun interceptNormalMethod( + invocation: InvocationInterceptor.Invocation, + invocationContext: ReflectiveInvocationContext, + extensionContext: ExtensionContext + ): T = interceptMethod(true, invocation, invocationContext, extensionContext) + private fun interceptLifecycleMethod( invocation: InvocationInterceptor.Invocation, - invocationContext: ReflectiveInvocationContext - ) { - val annotation = - AnnotationSupport.findAnnotation(invocationContext.executable, CoroutinesTimeout::class.java).orElseGet { - throw UnsupportedOperationException("CoroutinesTimeoutExtension should not be used directly; annotate the test class or method with CoroutinesTimeout instead.") - } - interceptInvocation(invocation, invocationContext.executable.name, annotation) - } + invocationContext: ReflectiveInvocationContext, + extensionContext: ExtensionContext + ) = interceptMethod(false, invocation, invocationContext, extensionContext) private fun interceptInvocation( invocation: InvocationInterceptor.Invocation, methodName: String, - annotation: CoroutinesTimeout + testTimeoutMs: Long, + cancelOnTimeout: Boolean ): T = - runWithTimeoutDumpingCoroutines(methodName, annotation.testTimeoutMs, annotation.cancelOnTimeout, - { CoroutinesTimeoutException(annotation.testTimeoutMs) }, { invocation.proceed()}) + runWithTimeoutDumpingCoroutines(methodName, testTimeoutMs, cancelOnTimeout, + { CoroutinesTimeoutException(testTimeoutMs) }, { invocation.proceed() }) } \ No newline at end of file From 8e414fdbe7456b12f3608cad0c060e028f37d833 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 20 Nov 2020 18:59:03 +0300 Subject: [PATCH 11/18] Test CoroutinesTimeoutExtension thoroughly --- .../src/junit/junit5/CoroutinesTimeout.kt | 1 + .../junit5/CoroutinesTimeoutExtension.kt | 32 ++++- .../junit5/CoroutinesTimeoutExtensionTest.kt | 122 ++++++++++++++++++ .../junit5/CoroutinesTimeoutMethodTest.kt | 44 +++++++ .../junit5/CoroutinesTimeoutNestedTest.kt | 30 +++++ .../test/junit5/CoroutinesTimeoutTest.kt | 77 ++++++++++- 6 files changed, 300 insertions(+), 6 deletions(-) create mode 100644 kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt create mode 100644 kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutMethodTest.kt create mode 100644 kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutNestedTest.kt diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt index 8c908eaa6c..9e5b00de1b 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt @@ -4,6 +4,7 @@ package kotlinx.coroutines.debug.junit5 import org.junit.jupiter.api.extension.* +import org.junit.jupiter.api.parallel.* import java.lang.annotation.* /** diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index be6ddc4f67..e943396dd6 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -10,9 +10,11 @@ import org.junit.jupiter.api.extension.* import org.junit.platform.commons.support.AnnotationSupport import java.lang.reflect.* import java.util.* +import java.util.concurrent.atomic.* public class CoroutinesTimeoutException(public val timeoutMs: Long): Exception("test timed out ofter $timeoutMs ms") +// NB: the constructor is not private so that JUnit is able to call it via reflection. public class CoroutinesTimeoutExtension internal constructor( private val enableCoroutineCreationStackTraces: Boolean = true, private val timeoutMs: Long? = null, @@ -32,22 +34,41 @@ public class CoroutinesTimeoutExtension internal constructor( CoroutinesTimeoutExtension(enableCoroutineCreationStackTraces, timeout.toLong() * 1000, cancelOnTimeout) } + private val debugProbesOwnershipPassed = AtomicBoolean(false) + + /* We install the debug probes early so that the coroutines launched from the test constructor are captured as well. + However, this is not enough as the same extension instance may be reused several times, even cleaning up its + resources from the store. */ + init { + DebugProbes.enableCreationStackTraces = enableCoroutineCreationStackTraces + DebugProbes.install() + } + + // This is needed so that a class with no tests still successfully passes the ownership of DebugProbes to JUnit5. override fun interceptTestClassConstructor( invocation: InvocationInterceptor.Invocation, invocationContext: ReflectiveInvocationContext>, extensionContext: ExtensionContext ): T { + initialize(extensionContext) + return invocation.proceed() + } + + private fun initialize(extensionContext: ExtensionContext) { val store: ExtensionContext.Store = extensionContext.getStore(NAMESPACE) if (store["debugProbes"] == null) { - /** no [DebugProbes] uninstaller is present, so this must be the first test that this instance of - * [CoroutinesTimeoutExtension] runs. Install the [DebugProbes]. */ - DebugProbes.enableCreationStackTraces = enableCoroutineCreationStackTraces - DebugProbes.install() + if (!debugProbesOwnershipPassed.compareAndSet(false, true)) { + /** This means that the [DebugProbes.install] call from the constructor of this extensions has already + * been matched with a corresponding cleanup procedure for JUnit5, but then JUnit5 cleaned everything up + * and later reused the same extension instance for other tests. Therefore, we need to install the + * [DebugProbes] anew. */ + DebugProbes.enableCreationStackTraces = enableCoroutineCreationStackTraces + DebugProbes.install() + } /** put a fake resource into this extensions's store so that JUnit cleans it up, uninstalling the * [DebugProbes] after this extension instance is no longer needed. **/ store.put("debugProbes", ExtensionContext.Store.CloseableResource { DebugProbes.uninstall() }) } - return invocation.proceed() } override fun interceptTestMethod( @@ -115,6 +136,7 @@ public class CoroutinesTimeoutExtension internal constructor( invocationContext: ReflectiveInvocationContext, extensionContext: ExtensionContext ): T { + initialize(extensionContext) val testAnnotationOptional = AnnotationSupport.findAnnotation(invocationContext.executable, CoroutinesTimeout::class.java) val classAnnotationOptional = extensionContext.testClass.flatMap { it.coroutinesTimeoutAnnotation() } diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt new file mode 100644 index 0000000000..7b87d60749 --- /dev/null +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt @@ -0,0 +1,122 @@ +/* + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package junit5 + +import kotlinx.coroutines.* +import kotlinx.coroutines.debug.junit5.* +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.* +import org.junit.jupiter.api.parallel.* + +class CoroutinesTimeoutExtensionTest { + + /** + * Tests that disabling coroutine creation stacktraces in [CoroutinesTimeoutExtension] does lead to them not being + * created. + * + * Adapted from [CoroutinesTimeoutDisabledTracesTest], an identical test for the JUnit4 rule. + * + * This test class is not intended to be run manually. Instead, use [CoroutinesTimeoutTest] as the entry point. + */ + class DisabledStackTracesTest { + @JvmField + @RegisterExtension + val timeout = CoroutinesTimeoutExtension(500, true, false) + + private val job = GlobalScope.launch(Dispatchers.Unconfined) { hangForever() } + + private suspend fun hangForever() { + suspendCancellableCoroutine { } + expectUnreached() + } + + @Test + fun hangingTest() = runBlocking { + waitForHangJob() + expectUnreached() + } + + private suspend fun waitForHangJob() { + job.join() + expectUnreached() + } + } + + /** + * Tests that [CoroutinesTimeoutExtension] is installed eagerly and detects the coroutines that were launched before + * any test events start happening. + * + * Adapted from [CoroutinesTimeoutEagerTest], an identical test for the JUnit4 rule. + * + * This test class is not intended to be run manually. Instead, use [CoroutinesTimeoutTest] as the entry point. + */ + class EagerTest { + + @JvmField + @RegisterExtension + val timeout = CoroutinesTimeoutExtension(500) + + private val job = GlobalScope.launch(Dispatchers.Unconfined) { hangForever() } + + private suspend fun hangForever() { + suspendCancellableCoroutine { } + expectUnreached() + } + + @Test + fun hangingTest() = runBlocking { + waitForHangJob() + expectUnreached() + } + + private suspend fun waitForHangJob() { + job.join() + expectUnreached() + } + } + + /** + * Tests that [CoroutinesTimeoutExtension] performs sensibly in some simple scenarios. + * + * Adapted from [CoroutinesTimeoutTest], an identical test for the JUnit4 rule. + * + * This test class is not intended to be run manually. Instead, use [CoroutinesTimeoutTest] as the entry point. + */ + class SimpleTest { + + @JvmField + @RegisterExtension + public val timeout = CoroutinesTimeoutExtension(1000, false, true) + + @Test + fun hangingTest() = runBlocking { + suspendForever() + expectUnreached() + } + + private suspend fun suspendForever() { + delay(Long.MAX_VALUE) + expectUnreached() + } + + @Test + fun throwingTest() = runBlocking { + throw RuntimeException() + } + + @Test + fun successfulTest() = runBlocking { + val job = launch { + yield() + } + + job.join() + } + } +} + +private fun expectUnreached(): Nothing { + error("Should not be reached") +} \ No newline at end of file diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutMethodTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutMethodTest.kt new file mode 100644 index 0000000000..ef30d47d25 --- /dev/null +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutMethodTest.kt @@ -0,0 +1,44 @@ +/* + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package junit5 +import kotlinx.coroutines.* +import kotlinx.coroutines.debug.junit5.* +import org.junit.jupiter.api.* + +/** + * Tests usage of [CoroutinesTimeout] on classes and test methods when only methods are annotated. + * + * This test class is not intended to be run manually. Instead, use [CoroutinesTimeoutTest] as the entry point. + */ +@TestMethodOrder(MethodOrderer.OrderAnnotation::class) +class CoroutinesTimeoutMethodTest { + + @Test + @Order(1) + fun noClassTimeout() { + runBlocking { + delay(150) + } + } + + @CoroutinesTimeout(100) + @Test + @Order(2) + fun usesMethodTimeoutWithNoClassTimeout() { + runBlocking { + delay(1000) + } + } + + @CoroutinesTimeout(1000) + @Test + @Order(3) + fun fitsInMethodTimeout() { + runBlocking { + delay(10) + } + } + +} diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutNestedTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutNestedTest.kt new file mode 100644 index 0000000000..275e3024e0 --- /dev/null +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutNestedTest.kt @@ -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 junit5 + +import kotlinx.coroutines.* +import kotlinx.coroutines.debug.junit5.* +import org.junit.jupiter.api.* + +/** + * This test checks that nested classes correctly recognize the [CoroutinesTimeout] annotation. + * + * This test class is not intended to be run manually. Instead, use [CoroutinesTimeoutTest] as the entry point. + */ +@CoroutinesTimeout(200) +class CoroutinesTimeoutNestedTest { + @Nested + inner class NestedInInherited { + @Test + fun usesOuterClassTimeout() = runBlocking { + delay(1000) + } + + @Test + fun fitsInOuterClassTimeout() = runBlocking { + delay(10) + } + } +} diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt index 9a55de09e6..5da49c3d46 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt @@ -6,6 +6,7 @@ package junit5 import kotlinx.coroutines.debug.junit5.* import org.assertj.core.api.* +import org.junit.Ignore import org.junit.Assert.* import org.junit.Test import org.junit.platform.engine.* @@ -18,7 +19,7 @@ import java.io.* class CoroutinesTimeoutTest { @Test - fun testCoroutinesTimeout() { + fun testCoroutinesTimeoutSimple() { val capturedOut = ByteArrayOutputStream() eventsForSelector(selectClass(CoroutinesTimeoutSimpleTest::class.java), capturedOut) .testFinishedSuccessfully("ignoresClassTimeout") @@ -29,6 +30,25 @@ class CoroutinesTimeoutTest { assertEquals(capturedOut.toString(), 3, countDumps(capturedOut)) } + @Test + fun testCoroutinesTimeoutMethod() { + val capturedOut = ByteArrayOutputStream() + eventsForSelector(selectClass(CoroutinesTimeoutMethodTest::class.java), capturedOut) + .testFinishedSuccessfully("fitsInMethodTimeout") + .testFinishedSuccessfully("noClassTimeout") + .testTimedOut("usesMethodTimeoutWithNoClassTimeout", 100) + assertEquals(capturedOut.toString(), 1, countDumps(capturedOut)) + } + + @Test + fun testCoroutinesTimeoutNested() { + val capturedOut = ByteArrayOutputStream() + eventsForSelector(selectClass(CoroutinesTimeoutNestedTest::class.java), capturedOut) + .testFinishedSuccessfully("fitsInOuterClassTimeout") + .testTimedOut("usesOuterClassTimeout", 200) + assertEquals(capturedOut.toString(), 1, countDumps(capturedOut)) + } + @Test fun testCoroutinesTimeoutInheritanceWithNoTimeoutInDerived() { val capturedOut = ByteArrayOutputStream() @@ -50,6 +70,61 @@ class CoroutinesTimeoutTest { .testTimedOut("classOverridesBaseClassTimeout2", 300) assertEquals(capturedOut.toString(), 1, countDumps(capturedOut)) } + + /* Currently there's no ability to replicate [TestFailureValidation] as is for JUnit5: + https://github.com/junit-team/junit5/issues/506. So, the test mechanism is more ad-hoc. */ + + /** + * This test is disabled due to a race condition between JUnit4 executors that run the given tests + * in parallel. This led to [DebugProbes.enableCreationStackTraces] being changed while this test is running. + * Because of this, there is no way for this test to consistently pass when there is more than one executor. + * + * In JUnit5, this is less of a problem since tests can be marked with [ResourceLock] to denote that they should not + * be run in parallel, so this issue is not reason enough to forbid configuring whether to enable the coroutine + * creation stacktraces. Yet this is enough of a problem to prevent normal testing of the extension in JUnit4. + * + * Making all the tests run sequentially (for example, by putting all their code into a single tests) works to show + * that the issue is solely due to parallelism. + */ + @Ignore + @Test + fun testCoroutinesTimeoutExtensionDisabledTraces() { + val capturedOut = ByteArrayOutputStream() + eventsForSelector(selectClass(CoroutinesTimeoutExtensionTest.DisabledStackTracesTest::class.java), capturedOut) + .testTimedOut("hangingTest", 500) + assertEquals(false, capturedOut.toString().contains("Coroutine creation stacktrace")) + assertEquals(capturedOut.toString(), 1, countDumps(capturedOut)) + } + + @Test + fun testCoroutinesTimeoutExtensionEager() { + val capturedOut = ByteArrayOutputStream() + eventsForSelector(selectClass(CoroutinesTimeoutExtensionTest.EagerTest::class.java), capturedOut) + .testTimedOut("hangingTest", 500) + for (expectedPart in listOf("hangForever", "waitForHangJob", "BlockingCoroutine{Active}")) { + assertEquals(expectedPart, true, capturedOut.toString().contains(expectedPart)) + } + assertEquals(capturedOut.toString(), 1, countDumps(capturedOut)) + } + + @Test + fun testCoroutinesTimeoutExtensionSimple() { + val capturedOut = ByteArrayOutputStream() + eventsForSelector(selectClass(CoroutinesTimeoutExtensionTest.SimpleTest::class.java), capturedOut) + .testFinishedSuccessfully("successfulTest") + .testTimedOut("hangingTest", 1000) + .haveExactly(1, event( + test("throwingTest"), + finishedWithFailure(Condition({ it is RuntimeException}, "is RuntimeException")) + )) + for (expectedPart in listOf("suspendForever", "invokeSuspend", "BlockingCoroutine{Active}")) { + assertEquals(expectedPart, true, capturedOut.toString().contains(expectedPart)) + } + for (nonExpectedPart in listOf("delay", "throwingTest")) { + assertEquals(nonExpectedPart, false, capturedOut.toString().contains(nonExpectedPart)) + } + assertEquals(capturedOut.toString(), 1, countDumps(capturedOut)) + } } private fun eventsForSelector(selector: DiscoverySelector, capturedOut: OutputStream): ListAssert { From 6e51638119c4d150d6a5a9750458ad80868c66d8 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 20 Nov 2020 19:46:55 +0300 Subject: [PATCH 12/18] Add documentation for JUnit5 CoroutinesTimeout --- .../src/junit/junit5/CoroutinesTimeout.kt | 52 ++++++++++++++++++- .../junit5/CoroutinesTimeoutExtension.kt | 45 ++++++++++++++++ .../test/junit5/CoroutinesTimeoutTest.kt | 9 ++++ .../test/junit5/RegisterExtensionExample.kt | 21 ++++++++ 4 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt index 9e5b00de1b..cb3d6fe66e 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt @@ -3,18 +3,68 @@ */ package kotlinx.coroutines.debug.junit5 +import kotlinx.coroutines.* +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 dumps contain the coroutine creation stack traces. If + * there is need to disable the creation stack traces in order to speed tests up, consider directly using + * [CoroutinesTimeoutExtension], which allows such configuration. + * + * This annotation registers [CoroutinesTimeoutExtension] 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. + * + * Beware that registering the extension via this annotation conflicts with manually registering the extension on the + * same tests via other methods (most notably, [RegisterExtension]) and is prohibited. + * + * Example usage: + * ``` + * @CoroutinesTimeout(100) + * class CoroutinesTimeoutSimpleTest { + * // times out in 150 ms + * @CoroutinesTimeout(1000) + * @Test + * fun classTimeoutIsOverridden() { + * runBlocking { + * delay(150) + * } + * } + * + * // times out in 100 ms + * @Test + * fun classTimeoutIsUsed() { + * runBlocking { + * delay(150) + * } + * } + * } + * ``` + * + * @see Timeout + * @see CoroutinesTimeoutExtension */ @MustBeDocumented @Retention(value = AnnotationRetention.RUNTIME) @Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION) @ExtendWith(CoroutinesTimeoutExtension::class) +@ResourceLock("coroutines timeout") @Inherited public annotation class CoroutinesTimeout( val testTimeoutMs: Long, val cancelOnTimeout: Boolean = false -) \ No newline at end of file +) diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index e943396dd6..db243c4d22 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -14,12 +14,54 @@ import java.util.concurrent.atomic.* public class CoroutinesTimeoutException(public val timeoutMs: Long): Exception("test timed out ofter $timeoutMs ms") +/** + * This JUnit5 extension allows running test, test factory, test template, and lifecycle 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`. + * [enableCoroutineCreationStackTraces] controls the corresponding [DebugProbes.enableCreationStackTraces] property + * and can be optionally disabled to speed-up tests if creation stack traces are not needed. + * + * Beware that if several tests that use this extension set [enableCoroutineCreationStackTraces] to different values and + * execute in parallel, the behavior is ill-defined. In order to avoid conflicts between different instances of this + * extension when using JUnit5 in parallel, use [ResourceLock] with resource name `coroutines timeout` on tests that use + * it. Note that the tests annotated with [CoroutinesTimeout] already use this [ResourceLock], so there is no need to + * annotate them additionally. + * + * Note that while calls to test factories are verified to finish in the specified time, but the methods that they + * produce are not affected by this extension. + * + * Beware that registering the extension via [CoroutinesTimeout] annotation conflicts with manually registering it on + * the same tests via other methods (most notably, [RegisterExtension]) and is prohibited. + * + * Example of usage: + * ``` + * class HangingTest { + * @JvmField + * @RegisterExtension + * val timeout = CoroutinesTimeoutExtension.seconds(5) + * + * @Test + * fun testThatHangs() = runBlocking { + * ... + * delay(Long.MAX_VALUE) // somewhere deep in the stack + * ... + * } + * } + * ``` + * + * @see [CoroutinesTimeout] + * */ // NB: the constructor is not private so that JUnit is able to call it via reflection. public class CoroutinesTimeoutExtension internal constructor( private val enableCoroutineCreationStackTraces: Boolean = true, private val timeoutMs: Long? = null, private val cancelOnTimeout: Boolean? = null): InvocationInterceptor { + /** + * Creates the [CoroutinesTimeoutExtension] extension with the given timeout in milliseconds. + */ public constructor(timeoutMs: Long, cancelOnTimeout: Boolean = false, enableCoroutineCreationStackTraces: Boolean = true): this(enableCoroutineCreationStackTraces, timeoutMs, cancelOnTimeout) @@ -28,6 +70,9 @@ public class CoroutinesTimeoutExtension internal constructor( private val NAMESPACE: ExtensionContext.Namespace = ExtensionContext.Namespace.create("kotlinx", "coroutines", "debug", "junit5", "CoroutinesTimeout") + /** + * Creates the [CoroutinesTimeoutExtension] extension with the given timeout in seconds. + */ @JvmOverloads public fun seconds(timeout: Int, cancelOnTimeout: Boolean = false, enableCoroutineCreationStackTraces: Boolean = true): CoroutinesTimeoutExtension = diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt index 5da49c3d46..ed1a0ab2df 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt @@ -18,6 +18,15 @@ import java.io.* // note that these tests are run using JUnit4 in order not to mix the testing systems. class CoroutinesTimeoutTest { + // This test is ignored because it just checks an example. + @Test + @Ignore + fun testRegisterExtensionExample() { + val capturedOut = ByteArrayOutputStream() + eventsForSelector(selectClass(RegisterExtensionExample::class.java), capturedOut) + .testTimedOut("testThatHangs", 5000) + } + @Test fun testCoroutinesTimeoutSimple() { val capturedOut = ByteArrayOutputStream() diff --git a/kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt b/kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt new file mode 100644 index 0000000000..ea4de4ceb2 --- /dev/null +++ b/kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt @@ -0,0 +1,21 @@ +/* + * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package junit5 + +import kotlinx.coroutines.* +import kotlinx.coroutines.debug.junit5.* +import org.junit.jupiter.api.* +import org.junit.jupiter.api.extension.* + +class RegisterExtensionExample { + @JvmField + @RegisterExtension + val timeout = CoroutinesTimeoutExtension.seconds(5) + + @Test + fun testThatHangs() = runBlocking { + delay(Long.MAX_VALUE) // somewhere deep in the stack + } +} \ No newline at end of file From 79bd28c6f4b377906bc4ab51ef8e247ecaa490ed Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 20 Nov 2020 20:09:07 +0300 Subject: [PATCH 13/18] Fix the debug probes not being always uninstalled --- .../src/junit/junit5/CoroutinesTimeout.kt | 2 ++ .../src/junit/junit5/CoroutinesTimeoutExtension.kt | 7 +++++++ .../test/junit5/CoroutinesTimeoutTest.kt | 13 ------------- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt index cb3d6fe66e..c221054d94 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt @@ -68,3 +68,5 @@ public annotation class CoroutinesTimeout( val testTimeoutMs: Long, val cancelOnTimeout: Boolean = false ) + +internal val x = 0 diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index db243c4d22..d0edf4241e 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -113,6 +113,13 @@ public class CoroutinesTimeoutExtension internal constructor( /** put a fake resource into this extensions's store so that JUnit cleans it up, uninstalling the * [DebugProbes] after this extension instance is no longer needed. **/ store.put("debugProbes", ExtensionContext.Store.CloseableResource { DebugProbes.uninstall() }) + } else if (!debugProbesOwnershipPassed.get()) { + /** This instance shares its store with other ones. Because of this, there was no need to install + * [DebugProbes], they are already installed, and this fact will outlive this instance of the extension. */ + if (debugProbesOwnershipPassed.compareAndSet(false, true)) { + // We successfully marked the ownership as passed, and now may uninstall the extraneous debug probes. + DebugProbes.uninstall() + } } } diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt index ed1a0ab2df..f609f6fd6a 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt @@ -83,19 +83,6 @@ class CoroutinesTimeoutTest { /* Currently there's no ability to replicate [TestFailureValidation] as is for JUnit5: https://github.com/junit-team/junit5/issues/506. So, the test mechanism is more ad-hoc. */ - /** - * This test is disabled due to a race condition between JUnit4 executors that run the given tests - * in parallel. This led to [DebugProbes.enableCreationStackTraces] being changed while this test is running. - * Because of this, there is no way for this test to consistently pass when there is more than one executor. - * - * In JUnit5, this is less of a problem since tests can be marked with [ResourceLock] to denote that they should not - * be run in parallel, so this issue is not reason enough to forbid configuring whether to enable the coroutine - * creation stacktraces. Yet this is enough of a problem to prevent normal testing of the extension in JUnit4. - * - * Making all the tests run sequentially (for example, by putting all their code into a single tests) works to show - * that the issue is solely due to parallelism. - */ - @Ignore @Test fun testCoroutinesTimeoutExtensionDisabledTraces() { val capturedOut = ByteArrayOutputStream() From 815e49862e4abe10972577477390aafb45c3c573 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 20 Nov 2020 20:15:01 +0300 Subject: [PATCH 14/18] Fix --- kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt index c221054d94..cb3d6fe66e 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt @@ -68,5 +68,3 @@ public annotation class CoroutinesTimeout( val testTimeoutMs: Long, val cancelOnTimeout: Boolean = false ) - -internal val x = 0 From b7df0933768d6ca238d5045ce21f711f8f0f98f4 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Tue, 24 Nov 2020 15:01:55 +0300 Subject: [PATCH 15/18] Improve handling of potential concurrency problems 1. A case where extension instances run concurrently and use the same extension context or where an extension instance is run on several methods concurrently is handled, even though it's unclear whether such things could happen. 2. Using CoroutinesTimeout via annotation no longer needs write access to the resource lock, meaning that tests annotated with it don't have to run sequentially. 3. Better documentation for the now-complex logic. --- .../src/junit/junit5/CoroutinesTimeout.kt | 2 +- .../junit5/CoroutinesTimeoutExtension.kt | 79 +++++++++++++------ 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt index cb3d6fe66e..2b2bf944ce 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt @@ -62,7 +62,7 @@ import java.lang.annotation.* @Retention(value = AnnotationRetention.RUNTIME) @Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION) @ExtendWith(CoroutinesTimeoutExtension::class) -@ResourceLock("coroutines timeout") +@ResourceLock("coroutines timeout", mode = ResourceAccessMode.READ) @Inherited public annotation class CoroutinesTimeout( val testTimeoutMs: Long, diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index d0edf4241e..326c4144b5 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -67,9 +67,6 @@ public class CoroutinesTimeoutExtension internal constructor( this(enableCoroutineCreationStackTraces, timeoutMs, cancelOnTimeout) public companion object { - private val NAMESPACE: ExtensionContext.Namespace = - ExtensionContext.Namespace.create("kotlinx", "coroutines", "debug", "junit5", "CoroutinesTimeout") - /** * Creates the [CoroutinesTimeoutExtension] extension with the given timeout in seconds. */ @@ -79,8 +76,14 @@ public class CoroutinesTimeoutExtension internal constructor( CoroutinesTimeoutExtension(enableCoroutineCreationStackTraces, timeout.toLong() * 1000, cancelOnTimeout) } + /** @see [initialize] */ private val debugProbesOwnershipPassed = AtomicBoolean(false) + private fun tryPassDebugProbesOwnership() = debugProbesOwnershipPassed.compareAndSet(false, true) + + private val isDebugProbesOwnershipPassed + get() = debugProbesOwnershipPassed.get() + /* We install the debug probes early so that the coroutines launched from the test constructor are captured as well. However, this is not enough as the same extension instance may be reused several times, even cleaning up its resources from the store. */ @@ -99,26 +102,58 @@ public class CoroutinesTimeoutExtension internal constructor( return invocation.proceed() } + /** + * Initialize this extension instance and/or the extension value store. + * + * It seems that the only way to reliably have JUnit5 clean up after its extensions is to put an instance of + * [ExtensionContext.Store.CloseableResource] into the value store corresponding to the extension instance, which + * means that [DebugProbes.uninstall] must be placed into the value store. [debugProbesOwnershipPassed] is `true` + * if the call to [DebugProbes.install] performed in the constructor of the extension instance was matched with a + * placing of [DebugProbes.uninstall] into the value store. We call the process of placing the cleanup procedure + * "passing the ownership", as now JUnit5 (and not our code) has to worry about uninstalling the debug probes. + * + * However, extension instances can be reused with different value stores, and value stores can be reused across + * extension instances. This leads to a tricky scheme of performing [DebugProbes.uninstall]: + * + * * If neither the ownership of this instance's [DebugProbes] was yet passed nor there is any cleanup procedure + * stored, it means that we can just store our cleanup procedure, passing the ownership. + * * If the ownership was not yet passed, but a cleanup procedure is already stored, we can't just replace it with + * another one, as this would lead to imbalance between [DebugProbes.install] and [DebugProbes.uninstall]. + * Instead, we know that this extension context will at least outlive this use of this instance, so some debug + * probes other than the ones from our constructor are already installed and won't be uninstalled during our + * operation. We simply uninstall the debug probes that were installed in our constructor. + * * If the ownership was passed, but the store is empty, it means that this test instance is reused and, possibly, + * the debug probes installed in its constructor were already uninstalled. This means that we have to install them + * anew and store an uninstaller. + */ private fun initialize(extensionContext: ExtensionContext) { - val store: ExtensionContext.Store = extensionContext.getStore(NAMESPACE) - if (store["debugProbes"] == null) { - if (!debugProbesOwnershipPassed.compareAndSet(false, true)) { - /** This means that the [DebugProbes.install] call from the constructor of this extensions has already - * been matched with a corresponding cleanup procedure for JUnit5, but then JUnit5 cleaned everything up - * and later reused the same extension instance for other tests. Therefore, we need to install the - * [DebugProbes] anew. */ - DebugProbes.enableCreationStackTraces = enableCoroutineCreationStackTraces - DebugProbes.install() - } - /** put a fake resource into this extensions's store so that JUnit cleans it up, uninstalling the - * [DebugProbes] after this extension instance is no longer needed. **/ - store.put("debugProbes", ExtensionContext.Store.CloseableResource { DebugProbes.uninstall() }) - } else if (!debugProbesOwnershipPassed.get()) { - /** This instance shares its store with other ones. Because of this, there was no need to install - * [DebugProbes], they are already installed, and this fact will outlive this instance of the extension. */ - if (debugProbesOwnershipPassed.compareAndSet(false, true)) { - // We successfully marked the ownership as passed, and now may uninstall the extraneous debug probes. - DebugProbes.uninstall() + val store: ExtensionContext.Store = extensionContext.getStore( + ExtensionContext.Namespace.create(CoroutinesTimeoutExtension::class, extensionContext.uniqueId)) + /** It seems that the JUnit5 documentation does not specify the relationship between the extension instances and + * the corresponding [ExtensionContext] (in which the value stores are managed), so it is unclear whether it's + * theoretically possible for two extension instances that run concurrently to share an extension context. So, + * just in case this risk exists, we synchronize here. */ + synchronized(store) { + if (store["debugProbes"] == null) { + if (!tryPassDebugProbesOwnership()) { + /** This means that the [DebugProbes.install] call from the constructor of this extensions has + * already been matched with a corresponding cleanup procedure for JUnit5, but then JUnit5 cleaned + * everything up and later reused the same extension instance for other tests. Therefore, we need to + * install the [DebugProbes] anew. */ + DebugProbes.enableCreationStackTraces = enableCoroutineCreationStackTraces + DebugProbes.install() + } + /** put a fake resource into this extensions's store so that JUnit cleans it up, uninstalling the + * [DebugProbes] after this extension instance is no longer needed. **/ + store.put("debugProbes", ExtensionContext.Store.CloseableResource { DebugProbes.uninstall() }) + } else if (!debugProbesOwnershipPassed.get()) { + /** This instance shares its store with other ones. Because of this, there was no need to install + * [DebugProbes], they are already installed, and this fact will outlive this use of this instance of + * the extension. */ + if (tryPassDebugProbesOwnership()) { + // We successfully marked the ownership as passed and now may uninstall the extraneous debug probes. + DebugProbes.uninstall() + } } } } From 324a00129ce7bac70b138e79ffa38c8f10a70cf5 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 26 Mar 2021 13:31:35 +0300 Subject: [PATCH 16/18] Fixes --- kotlinx-coroutines-debug/build.gradle | 1 - .../src/junit/junit5/CoroutinesTimeout.kt | 13 ++++++------- .../src/junit/junit5/CoroutinesTimeoutExtension.kt | 2 +- .../test/junit5/CoroutinesTimeoutExtensionTest.kt | 3 +-- .../test/junit5/CoroutinesTimeoutInheritanceTest.kt | 3 +-- .../test/junit5/CoroutinesTimeoutMethodTest.kt | 4 ++-- .../test/junit5/CoroutinesTimeoutNestedTest.kt | 3 +-- .../test/junit5/CoroutinesTimeoutSimpleTest.kt | 4 ++-- .../test/junit5/CoroutinesTimeoutTest.kt | 3 +-- .../test/junit5/RegisterExtensionExample.kt | 3 +-- 10 files changed, 16 insertions(+), 23 deletions(-) diff --git a/kotlinx-coroutines-debug/build.gradle b/kotlinx-coroutines-debug/build.gradle index 17b378e62b..1939489628 100644 --- a/kotlinx-coroutines-debug/build.gradle +++ b/kotlinx-coroutines-debug/build.gradle @@ -21,7 +21,6 @@ configurations { dependencies { compileOnly "junit:junit:$junit_version" compileOnly "org.junit.jupiter:junit-jupiter-api:$junit5_version" - testCompile "org.junit.jupiter:junit-jupiter-api:$junit5_version" testCompile "org.junit.jupiter:junit-jupiter-engine:$junit5_version" testCompile "org.junit.platform:junit-platform-testkit:1.7.0" shadowDeps "net.bytebuddy:byte-buddy:$byte_buddy_version" diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt index 2b2bf944ce..3f4800ecdc 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt @@ -3,7 +3,6 @@ */ package kotlinx.coroutines.debug.junit5 -import kotlinx.coroutines.* import kotlinx.coroutines.debug.* import org.junit.jupiter.api.* import org.junit.jupiter.api.extension.* @@ -15,7 +14,7 @@ import java.lang.annotation.* * 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 dumps contain the coroutine creation stack traces. If + * coroutines on timeout if [cancelOnTimeout] set to `true`. The dump contains the coroutine creation stack traces. If * there is need to disable the creation stack traces in order to speed tests up, consider directly using * [CoroutinesTimeoutExtension], which allows such configuration. * @@ -36,7 +35,7 @@ import java.lang.annotation.* * ``` * @CoroutinesTimeout(100) * class CoroutinesTimeoutSimpleTest { - * // times out in 150 ms + * // does not time out, as the annotation on the method overrides the class-level one * @CoroutinesTimeout(1000) * @Test * fun classTimeoutIsOverridden() { @@ -45,7 +44,7 @@ import java.lang.annotation.* * } * } * - * // times out in 100 ms + * // times out in 100 ms, timeout value is taken from the class-level annotation * @Test * fun classTimeoutIsUsed() { * runBlocking { @@ -58,12 +57,12 @@ import java.lang.annotation.* * @see Timeout * @see CoroutinesTimeoutExtension */ +@ExtendWith(CoroutinesTimeoutExtension::class) +@Inherited @MustBeDocumented +@ResourceLock("coroutines timeout", mode = ResourceAccessMode.READ) @Retention(value = AnnotationRetention.RUNTIME) @Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION) -@ExtendWith(CoroutinesTimeoutExtension::class) -@ResourceLock("coroutines timeout", mode = ResourceAccessMode.READ) -@Inherited public annotation class CoroutinesTimeout( val testTimeoutMs: Long, val cancelOnTimeout: Boolean = false diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index 326c4144b5..8ee0096525 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -12,7 +12,7 @@ import java.lang.reflect.* import java.util.* import java.util.concurrent.atomic.* -public class CoroutinesTimeoutException(public val timeoutMs: Long): Exception("test timed out ofter $timeoutMs ms") +internal class CoroutinesTimeoutException(val timeoutMs: Long): Exception("test timed out ofter $timeoutMs ms") /** * This JUnit5 extension allows running test, test factory, test template, and lifecycle methods in a separate thread, diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt index 7b87d60749..e892aaf2c3 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt @@ -2,10 +2,9 @@ * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ -package junit5 +package kotlinx.coroutines.debug.junit5 import kotlinx.coroutines.* -import kotlinx.coroutines.debug.junit5.* import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.* import org.junit.jupiter.api.parallel.* diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutInheritanceTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutInheritanceTest.kt index 5647dd1ce3..7c8de53db7 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutInheritanceTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutInheritanceTest.kt @@ -2,10 +2,9 @@ * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ -package junit5 +package kotlinx.coroutines.debug.junit5 import kotlinx.coroutines.* -import kotlinx.coroutines.debug.junit5.* import org.junit.jupiter.api.* /** diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutMethodTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutMethodTest.kt index ef30d47d25..64611b31e4 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutMethodTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutMethodTest.kt @@ -2,9 +2,9 @@ * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ -package junit5 +package kotlinx.coroutines.debug.junit5 + import kotlinx.coroutines.* -import kotlinx.coroutines.debug.junit5.* import org.junit.jupiter.api.* /** diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutNestedTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutNestedTest.kt index 275e3024e0..04c933d043 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutNestedTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutNestedTest.kt @@ -2,10 +2,9 @@ * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ -package junit5 +package kotlinx.coroutines.debug.junit5 import kotlinx.coroutines.* -import kotlinx.coroutines.debug.junit5.* import org.junit.jupiter.api.* /** diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutSimpleTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutSimpleTest.kt index 1b95875716..513a884601 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutSimpleTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutSimpleTest.kt @@ -2,9 +2,9 @@ * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ -package junit5 +package kotlinx.coroutines.debug.junit5 + import kotlinx.coroutines.* -import kotlinx.coroutines.debug.junit5.* import org.junit.jupiter.api.* /** diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt index f609f6fd6a..1f7b2080cb 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutTest.kt @@ -2,9 +2,8 @@ * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ -package junit5 +package kotlinx.coroutines.debug.junit5 -import kotlinx.coroutines.debug.junit5.* import org.assertj.core.api.* import org.junit.Ignore import org.junit.Assert.* diff --git a/kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt b/kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt index ea4de4ceb2..31033613dc 100644 --- a/kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt +++ b/kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt @@ -2,10 +2,9 @@ * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. */ -package junit5 +package kotlinx.coroutines.debug.junit5 import kotlinx.coroutines.* -import kotlinx.coroutines.debug.junit5.* import org.junit.jupiter.api.* import org.junit.jupiter.api.extension.* From 6c1d6e7b92498b5ae0a917a7b03441fea8a6238e Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 26 Mar 2021 13:47:49 +0300 Subject: [PATCH 17/18] apiDump --- kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api | 5 ----- 1 file changed, 5 deletions(-) diff --git a/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api b/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api index 5677548293..b8449df72b 100644 --- a/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api +++ b/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api @@ -66,11 +66,6 @@ public abstract interface annotation class kotlinx/coroutines/debug/junit5/Corou public abstract fun testTimeoutMs ()J } -public final class kotlinx/coroutines/debug/junit5/CoroutinesTimeoutException : java/lang/Exception { - public fun (J)V - public final fun getTimeoutMs ()J -} - public final class kotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension : org/junit/jupiter/api/extension/InvocationInterceptor { public static final field Companion Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension$Companion; public fun ()V From bb5216c205f5e174da7257fa6d3dcf24dd51d2bc Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 23 Apr 2021 11:45:58 +0300 Subject: [PATCH 18/18] Fixes --- .../api/kotlinx-coroutines-debug.api | 22 ------------------- kotlinx-coroutines-debug/build.gradle | 2 +- .../src/junit/junit5/CoroutinesTimeout.kt | 12 +++------- .../junit5/CoroutinesTimeoutExtension.kt | 5 +---- .../junit5/CoroutinesTimeoutExtensionTest.kt | 6 ++--- .../test/junit5/RegisterExtensionExample.kt | 2 +- 6 files changed, 9 insertions(+), 40 deletions(-) diff --git a/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api b/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api index b8449df72b..5bf70626a4 100644 --- a/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api +++ b/kotlinx-coroutines-debug/api/kotlinx-coroutines-debug.api @@ -66,25 +66,3 @@ public abstract interface annotation class kotlinx/coroutines/debug/junit5/Corou public abstract fun testTimeoutMs ()J } -public final class kotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension : org/junit/jupiter/api/extension/InvocationInterceptor { - public static final field Companion Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension$Companion; - public fun ()V - public fun (JZZ)V - public synthetic fun (JZZILkotlin/jvm/internal/DefaultConstructorMarker;)V - public fun interceptAfterAllMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V - public fun interceptAfterEachMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V - public fun interceptBeforeAllMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V - public fun interceptBeforeEachMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V - public fun interceptTestClassConstructor (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)Ljava/lang/Object; - public fun interceptTestFactoryMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)Ljava/lang/Object; - public fun interceptTestMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V - public fun interceptTestTemplateMethod (Lorg/junit/jupiter/api/extension/InvocationInterceptor$Invocation;Lorg/junit/jupiter/api/extension/ReflectiveInvocationContext;Lorg/junit/jupiter/api/extension/ExtensionContext;)V -} - -public final class kotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension$Companion { - public final fun seconds (I)Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension; - public final fun seconds (IZ)Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension; - public final fun seconds (IZZ)Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension; - public static synthetic fun seconds$default (Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension$Companion;IZZILjava/lang/Object;)Lkotlinx/coroutines/debug/junit5/CoroutinesTimeoutExtension; -} - diff --git a/kotlinx-coroutines-debug/build.gradle b/kotlinx-coroutines-debug/build.gradle index 1939489628..b2e3f2cf53 100644 --- a/kotlinx-coroutines-debug/build.gradle +++ b/kotlinx-coroutines-debug/build.gradle @@ -43,7 +43,7 @@ if (rootProject.ext.jvm_ir_enabled) { java { /* This is needed to be able to run JUnit5 tests. Otherwise, Gradle complains that it can't find the - JVM1.6-compatible version of the `junit-platform-testkit` artifact. */ + JVM1.6-compatible version of the `junit-jupiter-api` artifact. */ disableAutoTargetJvm() } diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt index 3f4800ecdc..9a8263fe5e 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeout.kt @@ -14,12 +14,10 @@ import java.lang.annotation.* * 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. If - * there is need to disable the creation stack traces in order to speed tests up, consider directly using - * [CoroutinesTimeoutExtension], which allows such configuration. + * coroutines on timeout if [cancelOnTimeout] set to `true`. The dump contains the coroutine creation stack traces. * - * This annotation registers [CoroutinesTimeoutExtension] on test, test factory, test template, and lifecycle methods - * and test classes that are annotated with it. + * 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 @@ -28,9 +26,6 @@ import java.lang.annotation.* * 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. * - * Beware that registering the extension via this annotation conflicts with manually registering the extension on the - * same tests via other methods (most notably, [RegisterExtension]) and is prohibited. - * * Example usage: * ``` * @CoroutinesTimeout(100) @@ -55,7 +50,6 @@ import java.lang.annotation.* * ``` * * @see Timeout - * @see CoroutinesTimeoutExtension */ @ExtendWith(CoroutinesTimeoutExtension::class) @Inherited diff --git a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt index 8ee0096525..a3e7713a5c 100644 --- a/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt +++ b/kotlinx-coroutines-debug/src/junit/junit5/CoroutinesTimeoutExtension.kt @@ -54,7 +54,7 @@ internal class CoroutinesTimeoutException(val timeoutMs: Long): Exception("test * @see [CoroutinesTimeout] * */ // NB: the constructor is not private so that JUnit is able to call it via reflection. -public class CoroutinesTimeoutExtension internal constructor( +internal class CoroutinesTimeoutExtension internal constructor( private val enableCoroutineCreationStackTraces: Boolean = true, private val timeoutMs: Long? = null, private val cancelOnTimeout: Boolean? = null): InvocationInterceptor @@ -81,9 +81,6 @@ public class CoroutinesTimeoutExtension internal constructor( private fun tryPassDebugProbesOwnership() = debugProbesOwnershipPassed.compareAndSet(false, true) - private val isDebugProbesOwnershipPassed - get() = debugProbesOwnershipPassed.get() - /* We install the debug probes early so that the coroutines launched from the test constructor are captured as well. However, this is not enough as the same extension instance may be reused several times, even cleaning up its resources from the store. */ diff --git a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt index e892aaf2c3..752c6c35cd 100644 --- a/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt +++ b/kotlinx-coroutines-debug/test/junit5/CoroutinesTimeoutExtensionTest.kt @@ -22,7 +22,7 @@ class CoroutinesTimeoutExtensionTest { class DisabledStackTracesTest { @JvmField @RegisterExtension - val timeout = CoroutinesTimeoutExtension(500, true, false) + internal val timeout = CoroutinesTimeoutExtension(500, true, false) private val job = GlobalScope.launch(Dispatchers.Unconfined) { hangForever() } @@ -55,7 +55,7 @@ class CoroutinesTimeoutExtensionTest { @JvmField @RegisterExtension - val timeout = CoroutinesTimeoutExtension(500) + internal val timeout = CoroutinesTimeoutExtension(500) private val job = GlobalScope.launch(Dispatchers.Unconfined) { hangForever() } @@ -87,7 +87,7 @@ class CoroutinesTimeoutExtensionTest { @JvmField @RegisterExtension - public val timeout = CoroutinesTimeoutExtension(1000, false, true) + internal val timeout = CoroutinesTimeoutExtension(1000, false, true) @Test fun hangingTest() = runBlocking { diff --git a/kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt b/kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt index 31033613dc..2de6b5b289 100644 --- a/kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt +++ b/kotlinx-coroutines-debug/test/junit5/RegisterExtensionExample.kt @@ -11,7 +11,7 @@ import org.junit.jupiter.api.extension.* class RegisterExtensionExample { @JvmField @RegisterExtension - val timeout = CoroutinesTimeoutExtension.seconds(5) + internal val timeout = CoroutinesTimeoutExtension.seconds(5) @Test fun testThatHangs() = runBlocking {