Use plain old thread instead of single-thread executor in order to be able to safely interrupt it in the end of a test

Replace delay(1) with yield in tests to make them timing-independent

Author: qwwdfsad

kotlinx-coroutines-debug/README.md

@@ -25,9 +25,9 @@ dependencies {
 ### Using in unit tests
 
 For JUnit4 debug module provides special test rule, [CoroutinesTimeout], for installing debug probes
-and dump coroutines on timeout to simplify tests debugging.
+and to dump coroutines on timeout to simplify tests debugging.
 
-Its usage is better to demonstrate by the example (runnable code is [here](test/TestRuleExample.kt)):
+Its usage is better demonstrated by the example (runnable code is [here](test/TestRuleExample.kt)):
 
 ```kotlin
 class TestRuleExample {
@@ -37,21 +37,16 @@ class TestRuleExample {
 
     private suspend fun someFunctionDeepInTheStack() {
         withContext(Dispatchers.IO) {
-            delay(Long.MAX_VALUE)
-            println("This line is never executed")
-        }
-
-        println("This line is never executed as well")
+            delay(Long.MAX_VALUE) // Hang method
+        }  
     }
 
     @Test
     fun hangingTest() = runBlocking {
         val job = launch {
             someFunctionDeepInTheStack()
         }
-
-        println("Doing some work...")
-        job.join()
+        job.join() // Join will hang
     }
 }
 ```

kotlinx-coroutines-debug/src/junit4/CoroutinesTimeout.kt

@@ -48,16 +48,14 @@ public class CoroutinesTimeout(
         /**
          * Creates [CoroutinesTimeout] rule with the given timeout in seconds.
          */
-        @JvmStatic
-        public fun seconds(seconds: Int, cancelOnTimeout: Boolean = false): CoroutinesTimeout {
-            return CoroutinesTimeout(TimeUnit.SECONDS.toMillis(seconds.toLong()), cancelOnTimeout)
-        }
+        public fun seconds(seconds: Int, cancelOnTimeout: Boolean = false): CoroutinesTimeout =
+            CoroutinesTimeout(TimeUnit.SECONDS.toMillis(seconds.toLong()), cancelOnTimeout)
+
     }
 
     /**
      * @suppress suppress from Dokka
      */
-    override fun apply(base: Statement, description: Description): Statement {
-        return CoroutinesTimeoutStatement(base, description, testTimeoutMs, cancelOnTimeout)
-    }
+    override fun apply(base: Statement, description: Description): Statement =
+        CoroutinesTimeoutStatement(base, description, testTimeoutMs, cancelOnTimeout)
 }

kotlinx-coroutines-debug/src/junit4/CoroutinesTimeoutStatement.kt

@@ -4,54 +4,51 @@
 
 package kotlinx.coroutines.debug.junit4
 
-import kotlinx.coroutines.*
 import kotlinx.coroutines.debug.*
 import org.junit.runner.*
 import org.junit.runners.model.*
 import java.util.concurrent.*
 
 internal class CoroutinesTimeoutStatement(
-    private val testStatement: Statement, private val testDescription: Description,
+    testStatement: Statement,
+    private val testDescription: Description,
     private val testTimeoutMs: Long,
     private val cancelOnTimeout: Boolean = false
 ) : Statement() {
 
-    private val testExecutor = Executors.newSingleThreadExecutor {
-        Thread(it).apply {
-            name = "Timeout test executor"
-            isDaemon = true
-        }
+    private val testStartedLatch = CountDownLatch(1)
+
+    private val testResult = FutureTask<Unit> {
+        testStartedLatch.countDown()
+        testStatement.evaluate()
     }
 
-    // Thread to dump stack from, captured by testExecutor
-    private lateinit var testThread: Thread
+    /*
+     * 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() {
-        DebugProbes.install() // Fail-fast if probes are unavailable
-        val latch = CountDownLatch(1)
-        val testFuture = CompletableFuture.runAsync(Runnable {
-            testThread = Thread.currentThread()
-            latch.countDown()
-            testStatement.evaluate()
-        }, testExecutor)
-
-        latch.await() // Await until test is started
+        DebugProbes.install()
+        testThread.start()
+        // Await until test is started to take only test execution time into account
+        testStartedLatch.await()
         try {
-            testFuture.get(testTimeoutMs, TimeUnit.MILLISECONDS)
+            testResult.get(testTimeoutMs, TimeUnit.MILLISECONDS)
             return
         } catch (e: TimeoutException) {
             handleTimeout(testDescription)
         } catch (e: ExecutionException) {
             throw e.cause ?: e
         } finally {
             DebugProbes.uninstall()
-            testExecutor.shutdown()
         }
     }
 
     private fun handleTimeout(description: Description) {
         val units =
-            if (testTimeoutMs % 1000L == 0L)
+            if (testTimeoutMs % 1000 == 0L)
                 "${testTimeoutMs / 1000} seconds"
             else "$testTimeoutMs milliseconds"
 

kotlinx-coroutines-debug/test/CoroutinesDumpTest.kt

@@ -81,6 +81,7 @@ class CoroutinesDumpTest : TestBase() {
     fun testRunningCoroutineWithSuspensionPoint() = synchronized(monitor) {
         val deferred = GlobalScope.async {
             activeMethod(shouldSuspend = true)
+            yield() // tail-call
         }
 
         awaitCoroutineStarted()
@@ -143,11 +144,11 @@ class CoroutinesDumpTest : TestBase() {
 
     private suspend fun activeMethod(shouldSuspend: Boolean) {
         nestedActiveMethod(shouldSuspend)
-        delay(1)
+        assertTrue(true) // tail-call
     }
 
     private suspend fun nestedActiveMethod(shouldSuspend: Boolean) {
-        if (shouldSuspend) delay(1)
+        if (shouldSuspend) yield()
         notifyTest()
         while (coroutineContext[Job]!!.isActive) {
             Thread.sleep(100)
@@ -156,11 +157,11 @@ class CoroutinesDumpTest : TestBase() {
 
     private suspend fun sleepingOuterMethod() {
         sleepingNestedMethod()
-        delay(1)
+        yield()
     }
 
     private suspend fun sleepingNestedMethod() {
-        delay(1)
+        yield()
         notifyTest()
         delay(Long.MAX_VALUE)
     }