F[Trace[F]], and I'd like to flatMap the
Trace[F]into an implicit argument for the test body. I'm using
[info]prefix to the output that messes up with how MUnit formats error messages
+lname is cryptic, I can't remember exactly why it's not self-descriptive. The flag parsing logic started as a fork of sbt/junit-interface that uses single character flag names
flush()method that's called from
RichLogger. popCurrentTestClassName()might be prone to race conditions in parallel execution.
private final ConcurrentHashMap<Description, ConcurrentLinkedDeque<String>> buffers = new ConcurrentHashMap<>();
System.outis already synchronized so everything should work in a concurrent setting as long as you use a call
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
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.
EventDispatcherand the result is: 1) I get one instance per suite, 2) the
reportedSuitesproperty 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
RichLoggermay not be necessary.
JUnitReporteris the Scala-equiavalent for
EventDispatcher(they should probably be named the same)
RichLoggershould 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.