Add coercion rationale, improve test coverage

Author: qwwdfsad

integration/kotlinx-coroutines-jdk8/src/time/Time.kt

@@ -3,48 +3,55 @@
  */
 package kotlinx.coroutines.time
 
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.selects.SelectBuilder
-import java.time.Duration
-import java.time.temporal.ChronoUnit
+import kotlinx.coroutines.*
+import kotlinx.coroutines.selects.*
+import java.time.*
+import java.time.temporal.*
 
 /**
- * "java.time" adapter method for [kotlinx.coroutines.delay]
+ * "java.time" adapter method for [kotlinx.coroutines.delay].
  */
 public suspend fun delay(duration: Duration) =
-        kotlinx.coroutines.delay(duration.toMillisDelay())
+        kotlinx.coroutines.delay(duration.coerceToMillis())
 
 /**
- * "java.time" adapter method for [SelectBuilder.onTimeout]
+ * "java.time" adapter method for [SelectBuilder.onTimeout].
  */
 public fun <R> SelectBuilder<R>.onTimeout(duration: Duration, block: suspend () -> R) =
-        onTimeout(duration.toMillisDelay(), block)
+        onTimeout(duration.coerceToMillis(), block)
 
 /**
- * "java.time" adapter method for [kotlinx.coroutines.withTimeout]
+ * "java.time" adapter method for [kotlinx.coroutines.withTimeout].
  */
 public suspend fun <T> withTimeout(duration: Duration, block: suspend CoroutineScope.() -> T): T =
-        kotlinx.coroutines.withTimeout(duration.toMillisDelay(), block)
+        kotlinx.coroutines.withTimeout(duration.coerceToMillis(), block)
 
 /**
- * "java.time" adapter method for [kotlinx.coroutines.withTimeoutOrNull]
+ * "java.time" adapter method for [kotlinx.coroutines.withTimeoutOrNull].
  */
 public suspend fun <T> withTimeoutOrNull(duration: Duration, block: suspend CoroutineScope.() -> T): T? =
-        kotlinx.coroutines.withTimeoutOrNull(duration.toMillisDelay(), block)
+        kotlinx.coroutines.withTimeoutOrNull(duration.coerceToMillis(), block)
 
 /**
- * Convert the [Duration] to millisecond delay.
+ * Coerces the given [Duration] to a millisecond delay.
+ * Negative values are coerced to zero, values that cannot
+ * be represented in milliseconds as long ("infinite" duration) are coerced to [Long.MAX_VALUE]
+ * and durations lesser than a millisecond are coerced to 1 millisecond.
  *
- * @return strictly positive duration is coerced to 1..[Long.MAX_VALUE] ms, `0` otherwise.
+ * The rationale of coercion:
+ * 1) Too large durations typically indicate infinity and Long.MAX_VALUE is the
+ *    best approximation of infinity we can provide.
+ * 2) Coercing too small durations to 1 instead of 0 is crucial for two patterns:
+ *    - Programming with deadlines and delays
+ *    - Non-suspending fast-paths (e.g. `withTimeout(1 nanosecond) { 42 }` should not throw)
  */
-private fun Duration.toMillisDelay(): Long =
-        if (this <= ChronoUnit.MILLIS.duration) {
-            if (this <= Duration.ZERO) 0
-            else 1
-        } else {
-            // values of Duration.ofMillis(Long.MAX_VALUE)
-            val maxSeconds = 9223372036854775
-            val maxNanos = 807000000
-            if (seconds < maxSeconds || seconds == maxSeconds && nano < maxNanos) toMillis()
-            else Long.MAX_VALUE
-        }
+private fun Duration.coerceToMillis(): Long {
+    if (isNegative) return 0
+    if (this <= ChronoUnit.MILLIS.duration) return 1
+
+    // Maximum scalar values of Duration.ofMillis(Long.MAX_VALUE)
+    val maxSeconds = 9223372036854775
+    val maxNanos = 807000000
+    return if (seconds < maxSeconds || seconds == maxSeconds && nano < maxNanos) toMillis()
+    else Long.MAX_VALUE
+}

integration/kotlinx-coroutines-jdk8/test/time/DurationOverflowTest.kt

@@ -0,0 +1,68 @@
+/*
+ * Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+
+package kotlinx.coroutines.time
+
+import kotlinx.coroutines.*
+import kotlinx.coroutines.selects.*
+import org.junit.Test
+import java.time.*
+import java.time.temporal.*
+import kotlin.test.*
+
+class DurationOverflowTest : TestBase() {
+
+    private val durations = ChronoUnit.values().map { it.duration }
+
+    @Test
+    fun testDelay() = runTest {
+        var counter = 0
+        for (duration in durations) {
+            expect(++counter)
+            delay(duration.negated()) // Instant bail out from negative values
+            launch(start = CoroutineStart.UNDISPATCHED) {
+                expect(++counter)
+                delay(duration)
+            }.cancelAndJoin()
+            expect(++counter)
+        }
+
+        finish(++counter)
+    }
+
+    @Test
+    fun testOnTimeout() = runTest {
+        for (duration in durations) {
+            // Does not crash on overflows
+            select<Unit> {
+                onTimeout(duration) {}
+                onTimeout(duration.negated()) {}
+            }
+        }
+    }
+
+    @Test
+    fun testWithTimeout() = runTest {
+        for (duration in durations) {
+            withTimeout(duration) {}
+        }
+    }
+
+    @Test
+    fun testWithTimeoutOrNull() = runTest {
+        for (duration in durations) {
+            withTimeoutOrNull(duration) {}
+        }
+    }
+
+    @Test
+    fun testWithTimeoutOrNullNegativeDuration() = runTest {
+        val result = withTimeoutOrNull(Duration.ofSeconds(1).negated()) {
+            1
+        }
+
+        assertNull(result)
+    }
+
+}

integration/kotlinx-coroutines-jdk8/test/time/TimeTest.kt

@@ -0,0 +1,68 @@
+/*
+ * Copyright 2016-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+
+package kotlinx.coroutines.time
+
+import kotlinx.coroutines.*
+import org.junit.Test
+import java.time.*
+import kotlin.test.*
+
+class WithTimeoutTest : TestBase() {
+
+    @Test
+    fun testWithTimeout() = runTest {
+        expect(1)
+        val result = withTimeout(Duration.ofMillis(10_000)) {
+            expect(2)
+            delay(Duration.ofNanos(1))
+            expect(3)
+            42
+        }
+
+        assertEquals(42, result)
+        finish(4)
+    }
+
+    @Test
+    fun testWithTimeoutOrNull() = runTest {
+        expect(1)
+        val result = withTimeoutOrNull(Duration.ofMillis(10_000)) {
+            expect(2)
+            delay(Duration.ofNanos(1))
+            expect(3)
+            42
+        }
+
+        assertEquals(42, result)
+        finish(4)
+    }
+
+    @Test
+    fun testWithTimeoutOrNullExceeded() = runTest {
+        expect(1)
+        val result = withTimeoutOrNull(Duration.ofMillis(3)) {
+            expect(2)
+            delay(Duration.ofSeconds(Long.MAX_VALUE))
+            expectUnreached()
+        }
+
+        assertNull(result)
+        finish(3)
+    }
+
+    @Test
+    fun testWithTimeoutExceeded() = runTest {
+        expect(1)
+        try {
+            withTimeout(Duration.ofMillis(3)) {
+                expect(2)
+                delay(Duration.ofSeconds(Long.MAX_VALUE))
+                expectUnreached()
+            }
+        } catch (e: TimeoutCancellationException) {
+            finish(3)
+        }
+    }
+}