From b46256976184d35eca9a30bb6692fceeb893e75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 16 Oct 2021 14:37:16 +0200 Subject: [PATCH] Add reproduction for #285 The bug appears to be fixed with the recent changes to async fixtures. --- .../scala/munit/internal/PlatformCompat.scala | 13 ++-- .../scala/munit/internal/PlatformCompat.scala | 10 +++ .../scala/munit/internal/PlatformCompat.scala | 6 ++ .../scala/munit/Issue285FrameworkSuite.scala | 62 +++++++++++++++++++ tests/shared/src/main/scala/munit/Tags.scala | 3 +- .../test/scala/munit/BaseFrameworkSuite.scala | 2 +- .../src/test/scala/munit/BaseSuite.scala | 2 + .../src/test/scala/munit/FrameworkSuite.scala | 1 + 8 files changed, 93 insertions(+), 6 deletions(-) create mode 100644 tests/shared/src/main/scala/munit/Issue285FrameworkSuite.scala diff --git a/munit/js/src/main/scala/munit/internal/PlatformCompat.scala b/munit/js/src/main/scala/munit/internal/PlatformCompat.scala index 598c8bee..3c238de7 100644 --- a/munit/js/src/main/scala/munit/internal/PlatformCompat.scala +++ b/munit/js/src/main/scala/munit/internal/PlatformCompat.scala @@ -10,8 +10,7 @@ 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 scala.scalajs.js.timers import java.util.concurrent.TimeoutException object PlatformCompat { @@ -31,7 +30,7 @@ object PlatformCompat { ec: ExecutionContext ): Future[T] = { val onComplete = Promise[T]() - val timeoutHandle = setTimeout(duration.toMillis) { + val timeoutHandle = timers.setTimeout(duration.toMillis) { onComplete.tryFailure( new TimeoutException(s"test timed out after $duration") ) @@ -40,13 +39,19 @@ object PlatformCompat { def run(): Unit = { startFuture().onComplete { result => onComplete.tryComplete(result) - clearTimeout(timeoutHandle) + timers.clearTimeout(timeoutHandle) }(ec) } }) onComplete.future } + def setTimeout(ms: Int)(body: => Unit): () => Unit = { + val timeoutHandle = timers.setTimeout(ms)(body) + + () => timers.clearTimeout(timeoutHandle) + } + // Scala.js does not support looking up annotations at runtime. def isIgnoreSuite(cls: Class[_]): Boolean = false diff --git a/munit/jvm/src/main/scala/munit/internal/PlatformCompat.scala b/munit/jvm/src/main/scala/munit/internal/PlatformCompat.scala index 84a628da..d8b85a31 100644 --- a/munit/jvm/src/main/scala/munit/internal/PlatformCompat.scala +++ b/munit/jvm/src/main/scala/munit/internal/PlatformCompat.scala @@ -53,6 +53,16 @@ object PlatformCompat { onComplete.future } + def setTimeout(ms: Int)(body: => Unit): () => Unit = { + val scheduled = sh.schedule[Unit]( + () => body, + ms, + TimeUnit.MILLISECONDS + ) + + () => scheduled.cancel(false) + } + def isIgnoreSuite(cls: Class[_]): Boolean = cls.getAnnotationsByType(classOf[munit.IgnoreSuite]).nonEmpty def isJVM: Boolean = true diff --git a/munit/native/src/main/scala/munit/internal/PlatformCompat.scala b/munit/native/src/main/scala/munit/internal/PlatformCompat.scala index d5b2a31c..c08636e4 100644 --- a/munit/native/src/main/scala/munit/internal/PlatformCompat.scala +++ b/munit/native/src/main/scala/munit/internal/PlatformCompat.scala @@ -26,6 +26,12 @@ object PlatformCompat { ): Future[T] = { startFuture() } + def setTimeout(ms: Int)(body: => Unit): () => Unit = { + Thread.sleep(ms) + body + + () => () + } // Scala Native does not support looking up annotations at runtime. def isIgnoreSuite(cls: Class[_]): Boolean = false diff --git a/tests/shared/src/main/scala/munit/Issue285FrameworkSuite.scala b/tests/shared/src/main/scala/munit/Issue285FrameworkSuite.scala new file mode 100644 index 00000000..856a8cff --- /dev/null +++ b/tests/shared/src/main/scala/munit/Issue285FrameworkSuite.scala @@ -0,0 +1,62 @@ +package munit + +import scala.concurrent.duration.Duration +import munit.internal.PlatformCompat +import scala.concurrent.Promise + +class Issue285FrameworkSuite extends FunSuite { + def println(msg: String): Unit = TestingConsole.out.println(msg) + val hello: Fixture[Unit] = new Fixture[Unit]("hello") { + def apply(): Unit = () + override def beforeAll(): Unit = { + println("beforeAll") + } + override def beforeEach(context: BeforeEach): Unit = { + println("beforeEach - " + context.test.name) + } + override def afterEach(context: AfterEach): Unit = { + println("afterEach - " + context.test.name) + } + override def afterAll(): Unit = { + println("afterAll") + } + } + override def munitFixtures: List[Fixture[Unit]] = List(hello) + override def munitTimeout: Duration = Duration(1, "ms") + test("issue-285-ok") { + () + } + test("issue-285-fail") { + val promise = Promise[Unit]() + PlatformCompat.setTimeout(3) { + promise.trySuccess(()) + } + promise.future + } + test("issue-285-ok") { + () + } +} + +object Issue285FrameworkSuite + extends FrameworkTest( + classOf[Issue285FrameworkSuite], + """|munit.Issue285FrameworkSuite: + |beforeAll + |beforeEach - issue-285-ok + |afterEach - issue-285-ok + | + issue-285-ok + |beforeEach - issue-285-fail + |afterEach - issue-285-fail + |==> X munit.Issue285FrameworkSuite.issue-285-fail java.util.concurrent.TimeoutException: test timed out after 1 millisecond + |beforeEach - issue-285-ok + |afterEach - issue-285-ok + | + issue-285-ok-1 + |afterAll + |""".stripMargin, + tags = Set( + // Skipped on Native because we don't support `PlatformCompat.setTimeout` + NoNative + ), + format = StdoutFormat + ) diff --git a/tests/shared/src/main/scala/munit/Tags.scala b/tests/shared/src/main/scala/munit/Tags.scala index b7ed879f..b81797be 100644 --- a/tests/shared/src/main/scala/munit/Tags.scala +++ b/tests/shared/src/main/scala/munit/Tags.scala @@ -1,5 +1,6 @@ package munit -object OnlyJVM extends Tag("OnlyJVm") {} +object OnlyJVM extends Tag("OnlyJVM") {} object NoDotty extends Tag("NoDotty") {} object Only213 extends Tag("Only213") {} +object NoNative extends Tag("NoNative") {} diff --git a/tests/shared/src/test/scala/munit/BaseFrameworkSuite.scala b/tests/shared/src/test/scala/munit/BaseFrameworkSuite.scala index 748d462f..b24cd9e9 100644 --- a/tests/shared/src/test/scala/munit/BaseFrameworkSuite.scala +++ b/tests/shared/src/test/scala/munit/BaseFrameworkSuite.scala @@ -83,7 +83,7 @@ abstract class BaseFrameworkSuite extends BaseSuite { } implicit val ec = munitExecutionContext val elapsedTimePattern = - Pattern.compile(" \\d+\\.\\d+s$", Pattern.MULTILINE) + Pattern.compile(" \\d+\\.\\d+s ?", Pattern.MULTILINE) TestingConsole.out = out TestingConsole.err = out for { diff --git a/tests/shared/src/test/scala/munit/BaseSuite.scala b/tests/shared/src/test/scala/munit/BaseSuite.scala index ba35cedf..94f22ee5 100644 --- a/tests/shared/src/test/scala/munit/BaseSuite.scala +++ b/tests/shared/src/test/scala/munit/BaseSuite.scala @@ -19,6 +19,8 @@ class BaseSuite extends FunSuite { test.tag(Ignore) } else if (test.tags(OnlyJVM) && !PlatformCompat.isJVM) { test.tag(Ignore) + } else if (test.tags(NoNative) && PlatformCompat.isNative) { + test.tag(Ignore) } else { test } diff --git a/tests/shared/src/test/scala/munit/FrameworkSuite.scala b/tests/shared/src/test/scala/munit/FrameworkSuite.scala index 573af70c..0f9a35ab 100644 --- a/tests/shared/src/test/scala/munit/FrameworkSuite.scala +++ b/tests/shared/src/test/scala/munit/FrameworkSuite.scala @@ -28,6 +28,7 @@ class FrameworkSuite extends BaseFrameworkSuite { SmallStackTraceFrameworkSuite, AssertionsFrameworkSuite, Issue179FrameworkSuite, + Issue285FrameworkSuite, ScalaCheckExceptionFrameworkSuite, BoxedFrameworkSuite )