Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Location macro to be hermetic #823

Merged
merged 5 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions munit/js/src/main/scala/munit/internal/io/Files.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,6 @@ object Files {
}
result
}
def exists(path: MunitPath): Boolean =
JSIO.exists(path.toString)
}
3 changes: 3 additions & 0 deletions munit/js/src/main/scala/munit/internal/io/PlatformIO.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ object PlatformIO {
object Files {
def readAllLines(path: Path): java.util.List[String] =
munit.internal.io.Files.readAllLines(path)
def exists(path: Path): Boolean =
munit.internal.io.Files.exists(path)
}

type Path = MunitPath
val Path = MunitPath
val Paths = munit.internal.io.Paths
}
5 changes: 5 additions & 0 deletions munit/jvm/src/main/scala/munit/internal/io/PlatformIO.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ object PlatformIO {
object Files {
def readAllLines(path: Path): java.util.List[String] =
java.nio.file.Files.readAllLines(path)
def exists(path: Path): Boolean =
java.nio.file.Files.exists(path)
}

type Path = java.nio.file.Path
object Path {
def workingDirectory: Path = Paths.get(sys.props("user.dir"))
}
object Paths {
def get(path: String): Path = java.nio.file.Paths.get(path)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ object PlatformIO {
object Files {
def readAllLines(path: Path): java.util.List[String] =
java.nio.file.Files.readAllLines(path)
def exists(path: Path): Boolean =
java.nio.file.Files.exists(path)
}

type Path = java.nio.file.Path
object Path {
def workingDirectory: Path = Paths.get(sys.props("user.dir"))
}
object Paths {
def get(path: String): Path = java.nio.file.Paths.get(path)
}
Expand Down
14 changes: 12 additions & 2 deletions munit/shared/src/main/scala-3/munit/internal/MacroCompat.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ import scala.quoted._
import scala.language.experimental.macros

object MacroCompat {
private val workingDirectory: String = {
val sep = java.io.File.separator
val cwd = sys.props("user.dir")
if (cwd.endsWith(sep)) cwd
else cwd + sep
}

trait LocationMacro {
inline implicit def generate: Location = ${ locationImpl() }
Expand All @@ -15,11 +21,15 @@ object MacroCompat {
def locationImpl()(using Quotes): Expr[Location] = {
import quotes.reflect._
val pos = Position.ofMacroExpansion
val path = pos.sourceFile.getJPath
val path0 = pos.sourceFile.getJPath
.map(_.toString())
.getOrElse(pos.sourceFile.path)
val relativePath =
if (path0.startsWith(workingDirectory))
path0.drop(workingDirectory.length)
else path0
val startLine = pos.startLine + 1
'{ new Location(${ Expr(path) }, ${ Expr(startLine) }) }
'{ new Location(${ Expr(relativePath) }, ${ Expr(startLine) }) }
}

trait ClueMacro {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,15 @@ object MacroCompatScala2 {

def locationImpl(c: Context): c.Tree = {
import c.universe._
val workingDirectory: String =
sys.props("user.dir") + java.io.File.separator
val line = Literal(Constant(c.enclosingPosition.line))
val path = Literal(Constant(c.enclosingPosition.source.path))
val path0 = c.enclosingPosition.source.path
val relativePath =
if (path0.startsWith(workingDirectory))
path0.drop(workingDirectory.length)
else path0
val path = Literal(Constant(relativePath))
New(c.mirror.staticClass(classOf[Location].getName()), path, line)
}

Expand Down
21 changes: 20 additions & 1 deletion munit/shared/src/main/scala/munit/internal/console/Lines.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,28 @@ class Lines extends Serializable {
}
def formatPath(location: Location): String =
location.path

def findPath(cwd: String, path: String, max: Int): Path = {
val p = Paths.get(cwd).resolve(path)
def getParentPath(somePath: String, sep: String): String = {
val somePath1 =
if (somePath.endsWith(sep)) somePath.dropRight(sep.length)
else somePath
val sep1 =
if (sep == "\\") "\\\\"
else sep
somePath1.split(sep1).dropRight(1).mkString(sep)
}
if (Files.exists(p)) p
else if (max < 1) sys.error(s"$path was not found")
else if (cwd.contains("\\"))
findPath(getParentPath(cwd, "\\"), path, max - 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about using https://docs.oracle.com/javase/8/docs/api/java/io/File.html#separator instead of looking at the current path ? Or would this only work on the JVM ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, for native and JS it might be simpler to just look for \.

else findPath(getParentPath(cwd, "/"), path, max - 1)
}

def formatLine(location: Location, message: String, clues: Clues): String = {
try {
val path = Paths.get(location.path)
val path = findPath(Path.workingDirectory.toString, location.path, 3)
val lines = filecache.getOrElseUpdate(
path, {
Files.readAllLines(path).asScala.toArray
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ class AssertionsFrameworkSuite extends FunSuite {
object AssertionsFrameworkSuite
extends FrameworkTest(
classOf[AssertionsFrameworkSuite],
"""|==> failure munit.AssertionsFrameworkSuite.equal-tostring - /scala/munit/AssertionsFrameworkSuite.scala:11 values are not equal even if they have the same `toString()`: C
"""|==> failure munit.AssertionsFrameworkSuite.equal-tostring - tests/shared/src/main/scala/munit/AssertionsFrameworkSuite.scala:11 values are not equal even if they have the same `toString()`: C
|10: }
|11: assertEquals[Any, Any](new A(), new B())
|12: }
|==> failure munit.AssertionsFrameworkSuite.case-class-productPrefix - /scala/munit/AssertionsFrameworkSuite.scala:21 values are not equal even if they have the same `toString()`: A()
|==> failure munit.AssertionsFrameworkSuite.case-class-productPrefix - tests/shared/src/main/scala/munit/AssertionsFrameworkSuite.scala:21 values are not equal even if they have the same `toString()`: A()
|20: }
|21: assertEquals[Any, Any](a.A(), b.A())
|22: }
|==> failure munit.AssertionsFrameworkSuite.different-toString - /scala/munit/AssertionsFrameworkSuite.scala:35
|==> failure munit.AssertionsFrameworkSuite.different-toString - tests/shared/src/main/scala/munit/AssertionsFrameworkSuite.scala:35
|34: }
|35: assertEquals[Any, Any](a.A(), b.A())
|36: }
Expand All @@ -61,7 +61,7 @@ object AssertionsFrameworkSuite
|=> Diff (- obtained, + expected)
|-a.A()
|+b.B()
|==> failure munit.AssertionsFrameworkSuite.toString-has-different-whitespace - /scala/munit/AssertionsFrameworkSuite.scala:39 values are not equal, even if their text representation only differs in leading/trailing whitespace and ANSI escape characters: foo
|==> failure munit.AssertionsFrameworkSuite.toString-has-different-whitespace - tests/shared/src/main/scala/munit/AssertionsFrameworkSuite.scala:39 values are not equal, even if their text representation only differs in leading/trailing whitespace and ANSI escape characters: foo
|38: test("toString-has-different-whitespace") {
|39: assertEquals[Any, Any]("foo", "foo ")
|40: }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ object AsyncFunFixtureFrameworkSuite
classOf[AsyncFunFixtureFrameworkSuite],
"""|==> failure munit.AsyncFunFixtureFrameworkSuite.fail when setup fails - failure in setup
|==> failure munit.AsyncFunFixtureFrameworkSuite.fail when teardown fails - failure in teardown
|==> failure munit.AsyncFunFixtureFrameworkSuite.fail when test and teardown fail - /scala/munit/AsyncFunFixtureFrameworkSuite.scala:28 failure in test
|==> failure munit.AsyncFunFixtureFrameworkSuite.fail when test and teardown fail - tests/shared/src/main/scala/munit/AsyncFunFixtureFrameworkSuite.scala:28 failure in test
|27: failingTeardown.test("fail when test and teardown fail") { _ =>
|28: fail("failure in test")
|29: }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class BoxedFrameworkSuite extends FunSuite {
object BoxedFrameworkSuite
extends FrameworkTest(
classOf[BoxedFrameworkSuite],
"""|==> failure munit.BoxedFrameworkSuite.exist issue - /scala/munit/BoxedFrameworkSuite.scala:15 assertion failed
"""|==> failure munit.BoxedFrameworkSuite.exist issue - tests/shared/src/main/scala/munit/BoxedFrameworkSuite.scala:15 assertion failed
|14: )
|15: assert(values.exists(outer => outer.data.exists(inner => inner.value > 90)))
|16: }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class CiOnlyFrameworkSuite extends FunSuite {
object CiOnlyFrameworkSuite
extends FrameworkTest(
classOf[CiOnlyFrameworkSuite],
"""|==> failure munit.CiOnlyFrameworkSuite.only - /scala/munit/CiOnlyFrameworkSuite.scala:5 'Only' tag is not allowed when `isCI=true`
"""|==> failure munit.CiOnlyFrameworkSuite.only - tests/shared/src/main/scala/munit/CiOnlyFrameworkSuite.scala:5 'Only' tag is not allowed when `isCI=true`
|4: override def isCI: Boolean = true
|5: test("only".only) {
|6: println("pass")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class DiffProductFrameworkSuite extends FunSuite {
object DiffProductFrameworkSuite
extends FrameworkTest(
classOf[DiffProductFrameworkSuite],
"""|==> failure munit.DiffProductFrameworkSuite.pass - /scala/munit/DiffProductFrameworkSuite.scala:9
"""|==> failure munit.DiffProductFrameworkSuite.pass - tests/shared/src/main/scala/munit/DiffProductFrameworkSuite.scala:9
|8: val john2 = User("John", 43, 2.to(2).toList)
|9: assertEquals(john2, john)
|10: }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ object DuplicateNameFrameworkSuite
extends FrameworkTest(
classOf[DuplicateNameFrameworkSuite],
"""|==> success munit.DuplicateNameFrameworkSuite.a
|==> failure munit.DuplicateNameFrameworkSuite.a-1 - /scala/munit/DuplicateNameFrameworkSuite.scala:9 boom
|==> failure munit.DuplicateNameFrameworkSuite.a-1 - tests/shared/src/main/scala/munit/DuplicateNameFrameworkSuite.scala:9 boom
|8: check("a")(() => ())
|9: check("a")(() => fail("boom"))
|10: check("a")(() => fail("boom"))
|==> failure munit.DuplicateNameFrameworkSuite.a-2 - /scala/munit/DuplicateNameFrameworkSuite.scala:10 boom
|==> failure munit.DuplicateNameFrameworkSuite.a-2 - tests/shared/src/main/scala/munit/DuplicateNameFrameworkSuite.scala:10 boom
|9: check("a")(() => fail("boom"))
|10: check("a")(() => fail("boom"))
|11: check("a")(() => ())
Expand Down
2 changes: 1 addition & 1 deletion tests/shared/src/main/scala/munit/FailFrameworkSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class FailFrameworkSuite extends FunSuite {
object FailFrameworkSuite
extends FrameworkTest(
classOf[FailFrameworkSuite],
"""|==> failure munit.FailFrameworkSuite.pass - /scala/munit/FailFrameworkSuite.scala:4 expected failure but test passed
"""|==> failure munit.FailFrameworkSuite.pass - tests/shared/src/main/scala/munit/FailFrameworkSuite.scala:4 expected failure but test passed
|3:class FailFrameworkSuite extends FunSuite {
|4: test("pass".fail) {
|5: // println("pass")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ object FailSuiteFrameworkSuite
extends FrameworkTest(
classOf[FailSuiteFrameworkSuite],
"""|==> success munit.FailSuiteFrameworkSuite.pass
|==> failure munit.FailSuiteFrameworkSuite.fail - /scala/munit/FailSuiteFrameworkSuite.scala:8 Oops, can not do anything.
|==> failure munit.FailSuiteFrameworkSuite.fail - tests/shared/src/main/scala/munit/FailSuiteFrameworkSuite.scala:8 Oops, can not do anything.
|7: test("fail") {
|8: failSuite("Oops, can not do anything.")
|9: }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Issue179FrameworkSuite extends FunSuite {
object Issue179FrameworkSuite
extends FrameworkTest(
classOf[Issue179FrameworkSuite],
"""|==> failure munit.Issue179FrameworkSuite.issue-179 - /scala/munit/Issue179FrameworkSuite.scala:5
"""|==> failure munit.Issue179FrameworkSuite.issue-179 - tests/shared/src/main/scala/munit/Issue179FrameworkSuite.scala:5
|4: test("issue-179") {
|5: assertNoDiff("/n", "A/n")
|6: }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ object FullStackTraceFrameworkSuite
extends BaseStackTraceFrameworkSuite(
Array("-F"),
"""|at munit.Assertions:failComparison
|==> failure munit.StackTraceFrameworkSuite.fail - /scala/munit/StackTraceFrameworkSuite.scala:5
|==> failure munit.StackTraceFrameworkSuite.fail - tests/shared/src/main/scala/munit/StackTraceFrameworkSuite.scala:5
|4: test("fail") {
|5: assertNoDiff("a", "b")
|6: }
Expand All @@ -45,7 +45,7 @@ object SmallStackTraceFrameworkSuite
extends BaseStackTraceFrameworkSuite(
Array(),
"""|at munit.Assertions:failComparison
|==> failure munit.StackTraceFrameworkSuite.fail - /scala/munit/StackTraceFrameworkSuite.scala:5
|==> failure munit.StackTraceFrameworkSuite.fail - tests/shared/src/main/scala/munit/StackTraceFrameworkSuite.scala:5
|4: test("fail") {
|5: assertNoDiff("a", "b")
|6: }
Expand Down
8 changes: 4 additions & 4 deletions tests/shared/src/test/scala/munit/LinesSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class LinesSuite extends FunSuite {
"hello",
Location.generate,
// comment
"""|LinesSuite.scala:8 hello
"""|tests/shared/src/test/scala/munit/LinesSuite.scala:8 hello
|7: "hello",
|8: Location.generate,
|9: // comment
Expand All @@ -19,7 +19,7 @@ class LinesSuite extends FunSuite {
"hello\nworld!",
Location.generate,
// comment
"""|LinesSuite.scala:20
"""|tests/shared/src/test/scala/munit/LinesSuite.scala:20
|19: "hello\nworld!",
|20: Location.generate,
|21: // comment
Expand All @@ -39,14 +39,14 @@ class LinesSuite extends FunSuite {
test(options) {
val obtained = munitLines
.formatLine(location, message)
.replace(location.path, location.filename)
.replace(raw"""tests\shared\src\test\scala\munit\""", "tests/shared/src/test/scala/munit/") // for Windows
assertNoDiff(obtained, expected)
}
}

val line: Int = Location.generate.line + 7
val endOfFileExpected: String =
s"""|LinesSuite.scala:${line} issue-211
s"""|tests/shared/src/test/scala/munit/LinesSuite.scala:${line} issue-211
|${line - 1}: // hello!
|${line}: check("end-of-file", "issue-211", Location.generate, endOfFileExpected ) }
|""".stripMargin
Expand Down