Respect munitTimeout for non-Future tests (#435)

docs/tests.md

@@ -23,8 +23,8 @@ test("basic") {
 
 ## Declare async test
 
-Async tests are declared the same way as basic tests. Test bodies that return
-`Future[T]` will automatically be awaited upon with `Await.result()`.
+Async tests are declared the same way as basic tests, except their test bodies
+return a value that can be converted into `Future[T]`.
 
 ```scala mdoc:silent
 import scala.concurrent.Future
@@ -36,31 +36,6 @@ test("async") {
 }
 ```
 
-```scala mdoc:passthrough
-println(s"The default timeout for async tests is $munitTimeout.")
-```
-
-Override `munitTimeout` to customize the timeout for how long tests should
-await.
-
-```scala mdoc
-import scala.concurrent.duration.Duration
-class CustomTimeoutSuite extends munit.FunSuite {
-  // await one second instead of default
-  override val munitTimeout = Duration(1, "s")
-  test("slow-async") {
-    Future {
-      Thread.sleep(5000)
-      // Test times out before `println()` is evaluated.
-      println("pass")
-    }
-  }
-}
-```
-
-Note that `Await.result()` only works on the JVM. Scala.js and Scala Native
-tests that return uncompleted `Future[T]` values will fail.
-
 MUnit has special handling for `scala.concurrent.Future[T]` since it is
 available in the standard library. Override `munitValueTransforms` to add custom
 handling for other asynchronous types.
@@ -85,9 +60,9 @@ test("buggy-task") {
 }
 ```
 
-Since tasks are lazy, a test that returns `LazyFuture[T]` will always pass since
-you need to call `run()` to start the task execution. Override
-`munitValueTransforms` to make sure that `LazyFuture.run()` gets called.
+The `LazyFuture` class doesn't evaluate the body until the `run()` method is
+invoked. Override `munitValueTransforms` to make sure that `LazyFuture.run()`
+gets called.
 
 ```scala mdoc
 import scala.concurrent.ExecutionContext.Implicits.global
@@ -108,6 +83,42 @@ class TaskSuite extends munit.FunSuite {
 }
 ```
 
+## Customize test timeouts
+
+> This feature is only available for the JVM and Scala.js. It's not available
+> for Scala Native.
+
+```scala mdoc:passthrough
+println(s"The default timeout for async tests is $munitTimeout.")
+println(s"Tests that exceed this timeout fail with an error message.")
+```
+
+```
+==> X munit.TimeoutSuite.slow  0.106s java.util.concurrent.TimeoutException: test timed out after 100 milliseconds
+```
+
+Override `munitTimeout` to customize the timeout for how long tests should
+await.
+
+```scala mdoc
+import scala.concurrent.duration.Duration
+class CustomTimeoutSuite extends munit.FunSuite {
+  // await one second instead of default
+  override val munitTimeout = Duration(1, "s")
+  test("slow-async") {
+    Future {
+      Thread.sleep(5000)
+      // Test times out before `println()` is evaluated.
+      println("pass")
+    }
+  }
+}
+```
+
+Note that old version for MUnit (v0.x series) the timeout only applied to async
+tests. Since the release of MUnit v1.0, the timeout applies to all tests
+including non-async tests.
+
 ## Run tests in parallel
 
 MUnit does not support running individual test cases in parallel. However, sbt

munit/js/src/main/scala/munit/internal/PlatformCompat.scala

@@ -10,6 +10,9 @@ import sbt.testing.Logger
 import scala.concurrent.Promise
 import scala.concurrent.duration.Duration
 import scala.concurrent.ExecutionContext
+import scala.scalajs.js.timers.clearTimeout
+import scala.scalajs.js.timers.setTimeout
+import java.util.concurrent.TimeoutException
 
 object PlatformCompat {
   def executeAsync(
@@ -23,11 +26,25 @@ object PlatformCompat {
   }
 
   def waitAtMost[T](
-      future: Future[T],
+      startFuture: () => Future[T],
       duration: Duration,
       ec: ExecutionContext
   ): Future[T] = {
-    future
+    val onComplete = Promise[T]()
+    val timeoutHandle = setTimeout(duration.toMillis) {
+      onComplete.tryFailure(
+        new TimeoutException(s"test timed out after $duration")
+      )
+    }
+    ec.execute(new Runnable {
+      def run(): Unit = {
+        startFuture().onComplete { result =>
+          onComplete.tryComplete(result)
+          clearTimeout(timeoutHandle)
+        }(ec)
+      }
+    })
+    onComplete.future
   }
 
   // Scala.js does not support looking up annotations at runtime.

munit/jvm/src/main/scala/munit/internal/PlatformCompat.scala

@@ -26,33 +26,31 @@ object PlatformCompat {
       future: Future[T],
       duration: Duration
   ): Future[T] = {
-    waitAtMost(future, duration, ExecutionContext.global)
+    waitAtMost(() => future, duration, ExecutionContext.global)
   }
   def waitAtMost[T](
-      future: Future[T],
+      startFuture: () => Future[T],
       duration: Duration,
       ec: ExecutionContext
   ): Future[T] = {
-    if (future.value.isDefined) {
-      // Avoid heavy timeout overhead for non-async tests.
-      future
-    } else {
-      val onComplete = Promise[T]()
-      var onCancel: () => Unit = () => ()
-      future.onComplete { result =>
-        onComplete.tryComplete(result)
-      }(ec)
-      val timeout = sh.schedule[Unit](
-        () =>
-          onComplete.tryFailure(
-            new TimeoutException(s"test timed out after $duration")
-          ),
-        duration.toMillis,
-        TimeUnit.MILLISECONDS
-      )
-      onCancel = () => timeout.cancel(false)
-      onComplete.future
-    }
+    val onComplete = Promise[T]()
+    val timeout = sh.schedule[Unit](
+      () =>
+        onComplete.tryFailure(
+          new TimeoutException(s"test timed out after $duration")
+        ),
+      duration.toMillis,
+      TimeUnit.MILLISECONDS
+    )
+    ec.execute(new Runnable {
+      def run(): Unit = {
+        startFuture().onComplete { result =>
+          onComplete.tryComplete(result)
+          timeout.cancel(false)
+        }(ec)
+      }
+    })
+    onComplete.future
   }
 
   def isIgnoreSuite(cls: Class[_]): Boolean =

munit/native/src/main/scala/munit/internal/PlatformCompat.scala

@@ -20,11 +20,11 @@ object PlatformCompat {
     Future.successful(())
   }
   def waitAtMost[T](
-      future: Future[T],
+      startFuture: () => Future[T],
       duration: Duration,
       ec: ExecutionContext
   ): Future[T] = {
-    future
+    startFuture()
   }
 
   // Scala Native does not support looking up annotations at runtime.

munit/shared/src/main/scala/munit/FunSuite.scala

@@ -34,7 +34,7 @@ trait BaseFunSuite
         options.name,
         { () =>
           try {
-            waitForCompletion(munitValueTransform(body))
+            waitForCompletion(() => munitValueTransform(body))
           } catch {
             case NonFatal(e) =>
               Future.failed(e)
@@ -47,7 +47,7 @@ trait BaseFunSuite
   }
 
   def munitTimeout: Duration = new FiniteDuration(30, TimeUnit.SECONDS)
-  private final def waitForCompletion[T](f: Future[T]) =
+  private final def waitForCompletion[T](f: () => Future[T]) =
     PlatformCompat.waitAtMost(f, munitTimeout, munitExecutionContext)
 
 }

tests/js/src/test/scala/munit/TimeoutSuite.scala

@@ -0,0 +1,28 @@
+package munit
+
+import scala.scalajs.js.timers._
+import scala.concurrent.Promise
+import scala.concurrent.duration.Duration
+
+class TimeoutSuite extends BaseSuite {
+  override def munitTimeout: Duration = Duration(3, "ms")
+  test("setTimeout-exceeds".fail) {
+    val promise = Promise[Unit]()
+    setTimeout(1000) {
+      promise.success(())
+    }
+    promise.future
+  }
+  test("setTimeout-passes") {
+    val promise = Promise[Unit]()
+    setTimeout(1) {
+      promise.success(())
+    }
+    promise.future
+  }
+
+  // We can't use an infinite loop because it blocks the main thread preventing the test from completing.
+  //   test("infinite-loop".fail) {
+  //     ThrottleCpu.run()
+  //   }
+}

tests/jvm/src/main/scala/munit/ThrottleCpu.scala

@@ -0,0 +1,18 @@
+package munit
+
+object ThrottleCpu {
+  def run(): Unit = {
+    while (true) {
+      // Some computationally intensive calculation
+      1.to(1000).foreach(i => fib(i))
+      println("Loop")
+    }
+  }
+
+  private final def fib(n: Int): Int = {
+    if (n < 1) 0
+    else if (n == 1) n
+    else fib(n - 1) + fib(n - 2)
+  }
+
+}

tests/jvm/src/test/scala/munit/TimeoutSuite.scala

@@ -26,21 +26,24 @@ class TimeoutSuite extends munit.FunSuite {
   }
   test("infinite-loop".fail) {
     Future {
-      while (true) {
-        def fib(n: Int): Int = {
-          if (n < 1) 0
-          else if (n == 1) n
-          else fib(n - 1) + fib(n - 2)
-        }
-        // Some computationally intensive calculation
-        1.to(1000).foreach(i => fib(i))
-        println("Loop")
-      }
+      ThrottleCpu.run()
     }
   }
   test("fast-3") {
     Future {
       Thread.sleep(1)
     }
   }
+  // NOTE(olafurpg): The test below times out on CI but not on my local Macbook
+  // test("slow-non-future".fail) {
+  //   ThrottleCpu.run()
+  // }
+  test("slow-non-future-sleep".fail) {
+    Thread.sleep(1000)
+  }
+  test("fast-4") {
+    Future {
+      Thread.sleep(1)
+    }
+  }
 }