From 11ee9d1ceef700940a805c3b52d360705c7f83d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 16 Oct 2021 14:42:38 +0200 Subject: [PATCH] Respect `munitTimeout` for non-Future tests (#435) --- docs/tests.md | 71 +++++++++++-------- .../scala/munit/internal/PlatformCompat.scala | 21 +++++- .../scala/munit/internal/PlatformCompat.scala | 42 ++++++----- .../scala/munit/internal/PlatformCompat.scala | 4 +- .../src/main/scala/munit/FunSuite.scala | 4 +- .../src/test/scala/munit/TimeoutSuite.scala | 28 ++++++++ .../src/main/scala/munit/ThrottleCpu.scala | 18 +++++ .../src/test/scala/munit/TimeoutSuite.scala | 23 +++--- 8 files changed, 143 insertions(+), 68 deletions(-) create mode 100644 tests/js/src/test/scala/munit/TimeoutSuite.scala create mode 100644 tests/jvm/src/main/scala/munit/ThrottleCpu.scala diff --git a/docs/tests.md b/docs/tests.md index 3463d7fa..3dfd9ea4 100644 --- a/docs/tests.md +++ b/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 diff --git a/munit/js/src/main/scala/munit/internal/PlatformCompat.scala b/munit/js/src/main/scala/munit/internal/PlatformCompat.scala index fa16b03d..598c8bee 100644 --- a/munit/js/src/main/scala/munit/internal/PlatformCompat.scala +++ b/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. diff --git a/munit/jvm/src/main/scala/munit/internal/PlatformCompat.scala b/munit/jvm/src/main/scala/munit/internal/PlatformCompat.scala index 4eb2918e..84a628da 100644 --- a/munit/jvm/src/main/scala/munit/internal/PlatformCompat.scala +++ b/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 = diff --git a/munit/native/src/main/scala/munit/internal/PlatformCompat.scala b/munit/native/src/main/scala/munit/internal/PlatformCompat.scala index c93d87e3..d5b2a31c 100644 --- a/munit/native/src/main/scala/munit/internal/PlatformCompat.scala +++ b/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. diff --git a/munit/shared/src/main/scala/munit/FunSuite.scala b/munit/shared/src/main/scala/munit/FunSuite.scala index efe80e62..97b10629 100644 --- a/munit/shared/src/main/scala/munit/FunSuite.scala +++ b/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) } diff --git a/tests/js/src/test/scala/munit/TimeoutSuite.scala b/tests/js/src/test/scala/munit/TimeoutSuite.scala new file mode 100644 index 00000000..83b373f2 --- /dev/null +++ b/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() + // } +} diff --git a/tests/jvm/src/main/scala/munit/ThrottleCpu.scala b/tests/jvm/src/main/scala/munit/ThrottleCpu.scala new file mode 100644 index 00000000..474f9e96 --- /dev/null +++ b/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) + } + +} diff --git a/tests/jvm/src/test/scala/munit/TimeoutSuite.scala b/tests/jvm/src/test/scala/munit/TimeoutSuite.scala index e8f6b480..e82bf8de 100644 --- a/tests/jvm/src/test/scala/munit/TimeoutSuite.scala +++ b/tests/jvm/src/test/scala/munit/TimeoutSuite.scala @@ -26,16 +26,7 @@ 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") { @@ -43,4 +34,16 @@ class TimeoutSuite extends munit.FunSuite { 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) + } + } }