flush() method that's called from EvenDispatcher https://github.com/scalameta/munit/blob/0354373d463fc691a3560c2085d35d8783b5708b/junit-interface/src/main/java/munit/internal/junitinterface/EventDispatcher.java
RichLogger. popCurrentTestClassName() might be prone to race conditions in parallel execution.
private final ConcurrentHashMap<Description, ConcurrentLinkedDeque<String>> buffers = new ConcurrentHashMap<>();
System.out is already synchronized so everything should work in a concurrent setting as long as you use a call println() or write() once per test suite
Ah great, I certainly would have needed much longer. I'll play with this on the weekend.
My main hurdle for quickly understanding the fairly simple code of RichLogger or EventDispatcher was merely the scoping, meaning how many instance of those exists, whether they are global, per suite, per test or some other scope. Which determines how they need to be synchronized (if at all). The use of synchronization in EventDispatcher suggests it is not "per suite", however it is only ever instantiated in JUnitTask.execute which kind of felt to me like it is scoped to a single suite.
EventDispatcher and the result is: 1) I get one instance per suite, 2) the reportedSuites property never has more than one item in it (which is always the suite the dispatcher belongs to), 3) even with parallel execution each dispatcher is always invoked by the same thread. It's not super-important performance-wise, but from this observation it seems that some of the synchronization in EventDispatcher and RichLogger may not be necessary.
JUnitReporter is the Scala-equiavalent for EventDispatcher (they should probably be named the same)
EventDispatcher and RichLogger should be cleaned up. They are implemented for multi-suite, multi-thread use, but they are A) not used in that way in practice, B) would be buggy if used that way as there is a race condition.
@olafurpg I did the PR for buffered logging here: scalameta/munit#397 - I did not make this option the default yet, that's kind of your call, happy to add that change if you want. I'm also open to duplicate the work for JS and Native, maybe in a separate PR.
I also opened an issue for several concurrency issues I found in the junit-interface module (more than the ones I already mentioned here): scalameta/munit#399. It's easy to fix those in the way suggested in the ticket, but I did not start yet, as I wanted to check whether you have any objections. The work is also best done after buffered logging is merged.
I have a multi module with munit like:
lazy val commonSettings = Seq(
scalaVersion := "2.13.7",
scalacOptions ++= Seq("-unchecked", "-deprecation", "-feature"),
libraryDependencies ++= Seq(Dependencies.munit),
testFrameworks += new TestFramework("munit.Framework")
)
lazy val framework = project.in(file("framework"))
.settings(commonSettings: _*)
.enablePlugins(ScalaJSPlugin)
.settings(libraryDependencies ++= Seq(Dependencies.cask) ++ Dependencies.scalaJs.value)And, I have a test in framework/src/test/scala/SomeTest.scala. But when I do sbt> framework/test it does not pick up the tests but the compilation does (if I make a syntax error in the SomeTest.scala) it picks up the compile error. What am I doing wrong?
%% instead of %%% but that's outside the control of MUnit
Hi folks! I'm working on transitioning some of our unit tests from ScalaTest to MUnit out of a need to support Async Generator-driven checks. Similar to Pathikrit's issue above, I'm unable to run any MUnit suites from SBT using test. IntelliJ is able to properly discover and run the unit tests, but SBT (v1.6.2) does not detect any tests and outputs No tests were executed.. In my case, I'm not using ScalaJS or PortableScala. My submodule includes this:
.settings(
testFrameworks += TestFrameworks.MUnit,
libraryDependencies ++= Seq(
"org.scalameta" %% "munit" % "0.7.29" % Test,
"org.scalameta" %% "munit-scalacheck" % "0.7.29" % Test,
"org.typelevel" %% "munit-cats-effect-3" % "1.0.7" % Test,
"org.typelevel" %% "scalacheck-effect-munit" % "1.0.4" % Test
)
)Has anyone else experienced this issue?
3 replies