Repository: effekt-lang/effekt (scala)

Issue:
`check` files are sensitive to CRLF / LF lineendings
I edited my `.check` files on windows so they had CRLF line endings which caused the test to declare failure.

Pull request:
Moving to munit has a few benefits:

- the library itself is a lot simpler (and might be easier to get into for students)
- the reported diffs for strings are much better (this is especially helpful, since we do a lot of golden-testing)
- it comes with support for ANSI/windows normalized string comparison, which hopefully fixes #168 
- it allows to filter easily based on the file name: 
![image](https://user-images.githubusercontent.com/408265/199960674-db9dbe89-87fa-4a31-84a8-8a5b1d902104.png)

There are also a few downsides:
- tests cannot be grouped nicely anymore, like they was with scala tests `describe`. Instead I am using many different test suites, now.
- do to the missing grouping, test output is not hierarchical anymore and might be harder to read.

For example, here is a run that shows munit:
![image](https://user-images.githubusercontent.com/408265/199960986-04c7de87-56e8-44c3-9360-757657f361b9.png)

previously, the tests were grouped by folders.

I am happy to accept these downsides. WDYT?

Patch:
diff --git a/build.sbt b/build.sbt
index b6538b076..386dfcfb5 100644
--- a/build.sbt
+++ b/build.sbt
@@ -45,8 +45,8 @@ lazy val lspDependencies = Seq(
 )
 
 lazy val testingDependencies = Seq(
-  "org.scala-sbt" %% "io" % "1.6.0" % "test",
-  "org.scalatest" %% "scalatest" % "3.2.9" % "test"
+  "org.scala-sbt" %% "io" % "1.6.0" % Test,
+  "org.scalameta" %% "munit" % "0.7.29" % Test
 )
 
 lazy val kiama: CrossProject = crossProject(JSPlatform, JVMPlatform).in(file("kiama"))


Test patch:
diff --git a/effekt/jvm/src/test/scala/effekt/ChezSchemeTests.scala b/effekt/jvm/src/test/scala/effekt/ChezSchemeTests.scala
index d02802b2c..1f868a818 100644
--- a/effekt/jvm/src/test/scala/effekt/ChezSchemeTests.scala
+++ b/effekt/jvm/src/test/scala/effekt/ChezSchemeTests.scala
@@ -5,11 +5,9 @@ import java.io.File
 import sbt.io._
 import sbt.io.syntax._
 
-import org.scalatest.funspec.AnyFunSpec
-
 import scala.language.implicitConversions
 
-class ChezSchemeTests extends EffektTests {
+abstract class ChezSchemeTests extends EffektTests {
 
   // Test files which are to be ignored (since features are missing or known bugs exist)
   override lazy val ignored: List[File] = List(
@@ -45,21 +43,6 @@ class ChezSchemeTests extends EffektTests {
     examplesDir / "pos" / "io", // async io is only implemented for monadic JS
   )
 
-  def runTestFor(f: File, expected: String) = {
-    it(f.getName + " (callcc)") {
-      val out = interpretCS(f, "callcc")
-      assert(expected == out)
-    }
-    it(f.getName + " (lift)") {
-      val out = interpretCS(f, "lift")
-      assert(expected == out)
-    }
-    it(f.getName + " (monadic)") {
-      val out = interpretCS(f, "monadic")
-      assert(expected == out)
-    }
-  }
-
   def interpretCS(file: File, variant: String): String = {
     val compiler = new effekt.Driver {}
     val configs = compiler.createConfig(Seq(
@@ -71,6 +54,32 @@ class ChezSchemeTests extends EffektTests {
     ))
     configs.verify()
     compiler.compileFile(file.getPath, configs)
-    removeAnsiColors(configs.stringEmitter.result())
+    configs.stringEmitter.result()
+  }
+}
+
+class ChezSchemeMonadicTests extends ChezSchemeTests {
+  def runTestFor(input: File, check: File, expected: String): Unit = {
+    test(input.getPath + " (monadic)") {
+      val out = interpretCS(input, "monadic")
+      assertNoDiff(out, expected)
+    }
+  }
+}
+
+class ChezSchemeCallCCTests extends ChezSchemeTests {
+  def runTestFor(input: File, check: File, expected: String): Unit = {
+    test(input.getPath + " (callcc)") {
+      val out = interpretCS(input, "callcc")
+      assertNoDiff(out, expected)
+    }
+  }
+}
+class ChezSchemeLiftTests extends ChezSchemeTests {
+  def runTestFor(input: File, check: File, expected: String): Unit = {
+    test(input.getPath + " (lift)") {
+      val out = interpretCS(input, "lift")
+      assertNoDiff(out, expected)
+    }
   }
 }
diff --git a/effekt/jvm/src/test/scala/effekt/ConstraintTests.scala b/effekt/jvm/src/test/scala/effekt/ConstraintTests.scala
index e05aa9bb5..d378f1f33 100644
--- a/effekt/jvm/src/test/scala/effekt/ConstraintTests.scala
+++ b/effekt/jvm/src/test/scala/effekt/ConstraintTests.scala
@@ -1,16 +1,15 @@
 package effekt
 package typer
+package constraints
 
 import effekt.source.NoSource
 import effekt.symbols.*
-import effekt.typer.*
 import effekt.util.messages.{ DebugMessaging, ErrorReporter, FatalPhaseError }
 import kiama.util.Positions
-import org.scalatest.funspec.AnyFunSpec
 
 import scala.language.implicitConversions
 
-class ConstraintTests extends AnyFunSpec {
+abstract class ConstraintTests extends munit.FunSuite {
 
   object messages extends DebugMessaging
 
@@ -43,10 +42,11 @@ class ConstraintTests extends AnyFunSpec {
     messages.clear()
     new Constraints
   }
+}
 
+class TestSimpleFlow extends ConstraintTests {
 
-  describe("Simple flow") {
-    it ("should propagate captures through lower bounds") {
+    test("should propagate captures through lower bounds") {
       val graph = freshGraph()
       import graph.*
 
@@ -62,7 +62,7 @@ class ConstraintTests extends AnyFunSpec {
       assert(B.upper == None)
     }
 
-    it ("should transitively propagate captures through lower bounds") {
+    test("should transitively propagate captures through lower bounds") {
       val graph = freshGraph()
       import graph.*
 
@@ -81,7 +81,7 @@ class ConstraintTests extends AnyFunSpec {
       assert(C.upper == None)
     }
 
-    it ("should propagate captures after connecting") {
+    test("should propagate captures after connecting") {
       val graph = freshGraph()
       import graph.*
 
@@ -97,7 +97,7 @@ class ConstraintTests extends AnyFunSpec {
       assert(B.upper == None)
     }
 
-    it ("should transitively propagate captures through lower bounds after connecting") {
+    test("should transitively propagate captures through lower bounds after connecting") {
       val graph = freshGraph()
       import graph.*
 
@@ -117,25 +117,24 @@ class ConstraintTests extends AnyFunSpec {
       assert(B.upper == None)
       assert(C.upper == None)
     }
-  }
-
-  describe("Errors") {
+}
 
-    it ("should report an error when conflicting bounds flow") {
+class TestErrors extends ConstraintTests {
+    test("should report an error when conflicting bounds flow") {
       val graph = freshGraph()
       import graph.*
 
       connect(A, B)
       requireUpper(Set(y), B)
 
-      assertThrows[FatalPhaseError] {
+      intercept[FatalPhaseError] {
         requireLower(Set(x), A)
       }
     }
-  }
+}
 
-  describe("Substitutions") {
-    it ("should add a substitution for a capture variable, when leaving the scope") {
+class TestSubstitutions extends ConstraintTests {
+    test("should add a substitution for a capture variable, when leaving the scope") {
       val graph = freshGraph()
       import graph.*
 
@@ -146,7 +145,7 @@ class ConstraintTests extends AnyFunSpec {
       assert(subst.get(A) == Some(CaptureSet(x)))
     }
 
-    it ("should check consistency with already substituted variables") {
+    test("should check consistency with already substituted variables") {
       val graph = freshGraph()
       import graph.*
 
@@ -157,14 +156,14 @@ class ConstraintTests extends AnyFunSpec {
       assert(B.lower == Some(Set(x)))
 
       requireUpper(Set(y), C)
-      assertThrows[FatalPhaseError] {
+      intercept[FatalPhaseError] {
         connect(A, C)
       }
     }
-  }
+}
 
-  describe("Subtracting") {
-    it ("should not propagate filtered captures into bounds") {
+class TestSubtracting extends ConstraintTests {
+    test ("should not propagate filtered captures into bounds") {
       val graph = freshGraph()
       import graph.*
 
@@ -179,7 +178,7 @@ class ConstraintTests extends AnyFunSpec {
       assert(A.upper == None)
       assert(B.upper == None)
     }
-    it ("should not propagate filtered existing captures into bounds") {
+    test ("should not propagate filtered existing captures into bounds") {
       val graph = freshGraph()
       import graph.*
 
@@ -194,7 +193,7 @@ class ConstraintTests extends AnyFunSpec {
       assert(A.upper == None)
       assert(B.upper == None)
     }
-    it ("should not conflict with solved substitutions") {
+    test ("should not conflict with solved substitutions") {
       val graph = freshGraph()
       import graph.*
 
@@ -209,7 +208,7 @@ class ConstraintTests extends AnyFunSpec {
       // should not affect upper bounds
       assert(B.upper == None)
     }
-    it ("if bounded from both sides it should still filter, appropriately") {
+    test ("if bounded from both sides it should still filter, appropriately") {
       val graph = freshGraph()
       import graph.*
 
@@ -228,7 +227,7 @@ class ConstraintTests extends AnyFunSpec {
       // should not affect upper bounds
       assert(B.upper == None)
     }
-    it ("filtering from above should admit *more* capabilities, not less") {
+    test ("filtering from above should admit *more* capabilities, not less") {
       val graph = freshGraph()
       import graph.*
 
@@ -251,7 +250,7 @@ class ConstraintTests extends AnyFunSpec {
       assert(B.lower == Some(Set()))
       assert(B.upper == Some(Set(x)))
     }
-    it ("filtering from above transitively should admit *more* capabilities, not less") {
+    test ("filtering from above transitively should admit *more* capabilities, not less") {
       val graph = freshGraph()
       import graph.*
 
@@ -279,5 +278,4 @@ class ConstraintTests extends AnyFunSpec {
       // all three flow as upper bound to A
       assert(A.upper == Some(Set(x, y, z)))
     }
-  }
-}
\ No newline at end of file
+}
diff --git a/effekt/jvm/src/test/scala/effekt/EffektTests.scala b/effekt/jvm/src/test/scala/effekt/EffektTests.scala
index 3e4378ccd..b70511917 100644
--- a/effekt/jvm/src/test/scala/effekt/EffektTests.scala
+++ b/effekt/jvm/src/test/scala/effekt/EffektTests.scala
@@ -5,11 +5,9 @@ import java.io.File
 import sbt.io._
 import sbt.io.syntax._
 
-import org.scalatest.funspec.AnyFunSpec
-
 import scala.language.implicitConversions
 
-trait EffektTests extends AnyFunSpec {
+trait EffektTests extends munit.FunSuite {
 
   // The sources of all testfiles are stored here:
   lazy val examplesDir = new File("examples")
@@ -20,11 +18,11 @@ trait EffektTests extends AnyFunSpec {
   // Folders to discover and run tests in
   lazy val included: List[File] = List(examplesDir)
 
-  def runTestFor(f: File, expectedResult: String): Unit
+  def runTestFor(input: File, check: File, expectedResult: String): Unit
 
   def runTests() = included.foreach(runPositiveTestsIn)
 
-  def runPositiveTestsIn(dir: File): Unit = describe(dir.getName) {
+  def runPositiveTestsIn(dir: File): Unit = //describe(dir.getName) {
     dir.listFiles.foreach {
       case f if f.isDirectory && !ignored.contains(f) =>
         runPositiveTestsIn(f)
@@ -39,16 +37,14 @@ trait EffektTests extends AnyFunSpec {
         }
 
         if (ignored.contains(f)) {
-          ignore(f.getName) { () }
+          test(f.getName.ignore) { () }
         } else {
-          runTestFor(f, IO.read(checkfile).toString)
+          val contents = IO.read(checkfile)
+          runTestFor(f, checkfile, contents)
         }
 
       case _ => ()
     }
-  }
-
-  def removeAnsiColors(text: String): String = text.replaceAll("\u001B\\[[;\\d]*m", "")
 
   runTests()
 }
diff --git a/effekt/jvm/src/test/scala/effekt/JavaScriptTests.scala b/effekt/jvm/src/test/scala/effekt/JavaScriptTests.scala
index 9ce092c26..f1170f489 100644
--- a/effekt/jvm/src/test/scala/effekt/JavaScriptTests.scala
+++ b/effekt/jvm/src/test/scala/effekt/JavaScriptTests.scala
@@ -7,20 +7,19 @@ import sbt.io.syntax._
 
 import scala.util.matching._
 
-import org.scalatest.funspec.AnyFunSpec
-
 import scala.language.implicitConversions
 
+
 class JavaScriptTests extends EffektTests {
 
   override lazy val ignored: List[File] = List(
     examplesDir / "llvm"
   )
 
-  def runTestFor(f: File, expected: String) =
-    it(f.getName) {
-      val out = interpretJS(f)
-      assert(expected == out)
+  def runTestFor(input: File, check: File, expected: String): Unit =
+    test(input.getPath) {
+      val out = interpretJS(input)
+      assertNoDiff(out, expected, s"Output running '${input.getPath}' differed from check file '${check.getPath}'.")
     }
 
   def interpretJS(file: File): String = {
@@ -30,11 +29,14 @@ class JavaScriptTests extends EffektTests {
     val configs = compiler.createConfig(Seq("--Koutput", "string", "--lib", "libraries/js/monadic"))
     configs.verify()
     compiler.compileFile(file.getPath, configs)
-    removeAnsiColors(configs.stringEmitter.result())
+    configs.stringEmitter.result()
   }
 }
 
-object TestUtils extends JavaScriptTests {
+object TestUtils {
+
+  object jsTests extends JavaScriptTests
+  import jsTests.*
 
   /**
    * Generates the check files from the actual outputs.
diff --git a/effekt/jvm/src/test/scala/effekt/LLVMTests.scala b/effekt/jvm/src/test/scala/effekt/LLVMTests.scala
index 6ec595b0c..5005af898 100644
--- a/effekt/jvm/src/test/scala/effekt/LLVMTests.scala
+++ b/effekt/jvm/src/test/scala/effekt/LLVMTests.scala
@@ -1,7 +1,6 @@
 package effekt
 
 import java.io.File
-import org.scalatest.funspec.AnyFunSpec
 import sbt.io._
 import sbt.io.syntax._
 import scala.language.implicitConversions
@@ -23,11 +22,12 @@ class LLVMTests extends EffektTests {
     examplesDir / "llvm" / "gids.effekt",
   )
 
-  def runTestFor(f: File, expected: String) =
-    it(f.getName + " (llvm)") {
-      val out = runLLVM(f)
-      assert(expected == out)
+  def runTestFor(input: File, check: File, expected: String): Unit = {
+    test(input.getPath + " (llvm)") {
+      val out = runLLVM(input)
+      assertNoDiff(out, expected)
     }
+  }
 
   def runLLVM(f: File): String = {
     // TODO flaky body
@@ -39,6 +39,6 @@ class LLVMTests extends EffektTests {
     ))
     configs.verify()
     compiler.compileFile(f.getPath, configs)
-    removeAnsiColors(configs.stringEmitter.result())
+    configs.stringEmitter.result()
   }
 }