Skip to content

Commit bfaec0f

Browse files
adpi2adpi2
committed
Improve error reporting
1 parent f14f73b commit bfaec0f

File tree

2 files changed

+104
-75
lines changed

2 files changed

+104
-75
lines changed

‎compiler/test/dotty/tools/debug/DebugStepAssert.scala

Lines changed: 71 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,57 @@
11
package dotty.tools.debug
22

33
import com.sun.jdi.Location
4-
import dotty.tools.io.JFile
4+
import dotty.tools.io.JPath
55
import dotty.tools.readLines
66

7+
import scala.annotation.tailrec
8+
79
/**
810
* A debug step and an associated assertion to validate the step.
911
* A sequence of DebugStepAssert is parsed from the check file in tests/debug
1012
*/
11-
private[debug] case class DebugStepAssert[T](step: DebugStep[T], assert: T => Unit)
13+
private[debug] case class DebugStepAssert[T](step: DebugStep[T], assertion: T => Unit)(
14+
using val location: CheckFileLocation
15+
)
16+
17+
/** A location in the check file */
18+
private[debug] case class CheckFileLocation(checkFile: JPath, line: Int):
19+
override def toString: String = s"$checkFile:$line"
20+
21+
/** When a DebugStepAssert fails it throws a DebugStepException */
22+
private[debug] case class DebugStepException(message: String, location: CheckFileLocation) extends Exception
23+
24+
private[debug] enum DebugStep[T]:
25+
case Break(className: String, line: Int) extends DebugStep[Location]
26+
case Step extends DebugStep[Location]
27+
case Next extends DebugStep[Location]
28+
case Eval(expression: String) extends DebugStep[Either[String, String]]
1229

1330
private[debug] object DebugStepAssert:
31+
private val sym = "[a-zA-Z0-9$.]+"
32+
private val line = raw"\d+"
33+
private val trailing = raw" *(?://.*)?".r // empty or comment
34+
private val break = s"break ($sym) ($line)$trailing".r
35+
private val step = s"step ($sym|$line)$trailing".r
36+
private val next = s"next ($sym|$line)$trailing".r
37+
private val multiLineEval = s"eval$trailing".r
38+
private val eval = s"eval (.*)".r
39+
private val result = "result (.*)".r
40+
private val error = "error (.*)".r
41+
private val multiLineError = s"error$trailing".r
42+
1443
import DebugStep.*
15-
def parseCheckFile(checkFile: JFile): Seq[DebugStepAssert[?]] =
16-
val sym = "[a-zA-Z0-9$.]+"
17-
val line = "\\d+"
18-
val trailing = s"\\s*(?:\\/\\/.*)?".r // empty or comment
19-
val break = s"break ($sym) ($line)$trailing".r
20-
val step = s"step ($sym|$line)$trailing".r
21-
val next = s"next ($sym|$line)$trailing".r
22-
val multiLineEval = s"eval$trailing".r
23-
val eval = s"eval (.*)".r
24-
val result = "result (.*)".r
25-
val error = "error (.*)".r
26-
val multiLineError = s"error$trailing".r
44+
def parseCheckFile(checkFile: JPath): Seq[DebugStepAssert[?]] =
45+
val allLines = readLines(checkFile.toFile)
2746

47+
@tailrec
2848
def loop(lines: List[String], acc: List[DebugStepAssert[?]]): List[DebugStepAssert[?]] =
49+
given location: CheckFileLocation = CheckFileLocation(checkFile, allLines.size - lines.size + 1)
2950
lines match
3051
case Nil => acc.reverse
3152
case break(className , lineStr) :: tail =>
32-
val line = lineStr.toInt
33-
val step = DebugStepAssert(Break(className, line), checkClassAndLine(className, line))
53+
val breakpointLine = lineStr.toInt
54+
val step = DebugStepAssert(Break(className, breakpointLine), checkClassAndLine(className, breakpointLine))
3455
loop(tail, step :: acc)
3556
case step(pattern) :: tail =>
3657
val step = DebugStepAssert(Step, checkLineOrMethod(pattern))
@@ -49,70 +70,70 @@ private[debug] object DebugStepAssert:
4970
val step = DebugStepAssert(Eval(expr), assertion)
5071
loop(tail2, step :: acc)
5172
case trailing() :: tail => loop(tail, acc)
52-
case invalid :: tail => throw new Exception(s"Cannot parse debug step: $invalid")
73+
case invalid :: tail =>
74+
throw new Exception(s"Cannot parse debug step: $invalid ($location)")
5375

5476
def parseEvalAssertion(lines: List[String]): (Either[String, String] => Unit, List[String]) =
77+
given location: CheckFileLocation = CheckFileLocation(checkFile, allLines.size - lines.size + 1)
5578
lines match
5679
case Nil => throw new Exception(s"Missing result or error")
80+
case trailing() :: tail => parseEvalAssertion(tail)
5781
case result(expected) :: tail => (checkResult(expected), tail)
5882
case error(expected) :: tail => (checkError(Seq(expected)), tail)
5983
case multiLineError() :: tail0 =>
6084
val (expected, tail1) = tail0.span(_.startsWith(" "))
6185
(checkError(expected.map(_.stripPrefix(" "))), tail1)
62-
case invalid :: _ => throw new Exception(s"Cannot parse as result or error: $invalid")
86+
case invalid :: _ =>
87+
throw new Exception(s"Cannot parse as result or error: $invalid ($location)")
6388

64-
loop(readLines(checkFile), Nil)
89+
loop(allLines, Nil)
6590
end parseCheckFile
6691

67-
private def checkClassAndLine(className: String, line: Int)(location: Location): Unit =
68-
assert(className == location.declaringType.name, s"obtained ${location.declaringType.name}, expected ${className}")
69-
checkLine(line)(location)
92+
private def checkClassAndLine(className: String, breakpointLine: Int)(using CheckFileLocation)(location: Location): Unit =
93+
debugStepAssertEquals(location.declaringType.name, className)
94+
checkLine(breakpointLine)(location)
7095

71-
private def checkLineOrMethod(pattern: String): Location => Unit =
72-
if "(\\d+)".r.matches(pattern) then checkLine(pattern.toInt) else checkMethod(pattern)
96+
private def checkLineOrMethod(pattern: String)(using CheckFileLocation): Location => Unit =
97+
pattern.toIntOption.map(checkLine).getOrElse(checkMethod(pattern))
7398

74-
private def checkLine(line: Int)(location: Location): Unit =
75-
assert(location.lineNumber == line, s"obtained ${location.lineNumber}, expected $line")
99+
private def checkLine(expected: Int)(using CheckFileLocation)(location: Location): Unit =
100+
debugStepAssertEquals(location.lineNumber, expected)
76101

77-
private def checkMethod(methodName: String)(location: Location): Unit = assert(methodName == location.method.name)
102+
private def checkMethod(expected: String)(using CheckFileLocation)(location: Location): Unit =
103+
debugStepAssertEquals(location.method.name, expected)
78104

79-
private def checkResult(expected: String)(obtained: Either[String, String]): Unit =
105+
private def checkResult(expected: String)(using CheckFileLocation)(obtained: Either[String, String]): Unit =
80106
obtained match
81107
case Left(obtained) =>
82-
val message =
108+
debugStepFailed(
83109
s"""|Evaluation failed:
84110
|${obtained.replace("\n", "\n|")}""".stripMargin
85-
throw new AssertionError(message)
86-
case Right(obtained) =>
87-
val message =
88-
s"""|Expected: $expected
89-
|Obtained: $obtained""".stripMargin
90-
assert(expected.r.matches(obtained.toString), message)
111+
)
112+
case Right(obtained) => debugStepAssertEquals(obtained, expected)
91113

92-
private def checkError(expected: Seq[String])(obtained: Either[String, String]): Unit =
114+
private def checkError(expected: Seq[String])(using CheckFileLocation)(obtained: Either[String, String]): Unit =
93115
obtained match
94116
case Left(obtained) =>
95-
val message =
117+
debugStepAssert(
118+
expected.forall(e => e.r.findFirstMatchIn(obtained).isDefined),
96119
s"""|Expected:
97-
|${expected.mkString("\n")}
120+
|${expected.mkString("\n|")}
98121
|Obtained:
99122
|${obtained.replace("\n", "\n|")}""".stripMargin
100-
assert(expected.forall(e => e.r.findFirstMatchIn(obtained).isDefined), message)
123+
)
101124
case Right(obtained) =>
102-
val message =
125+
debugStepFailed(
103126
s"""|Evaluation succeeded but failure expected.
104127
|Obtained: $obtained
105128
|""".stripMargin
106-
throw new AssertionError(message)
107-
108-
109-
end DebugStepAssert
110-
111-
private[debug] enum DebugStep[T]:
112-
case Break(className: String, line: Int) extends DebugStep[Location]
113-
case Step extends DebugStep[Location]
114-
case Next extends DebugStep[Location]
115-
case Eval(expression: String) extends DebugStep[Either[String, String]]
129+
)
116130

131+
private def debugStepAssertEquals[T](obtained: T, expected: T)(using CheckFileLocation): Unit =
132+
debugStepAssert(obtained == expected, s"Obtained $obtained, Expected: $expected")
117133

134+
private def debugStepAssert(assertion: Boolean, message: String)(using CheckFileLocation): Unit =
135+
if !assertion then debugStepFailed(message)
118136

137+
private def debugStepFailed(message: String)(using location: CheckFileLocation): Unit =
138+
throw DebugStepException(message, location)
139+
end DebugStepAssert

‎compiler/test/dotty/tools/debug/DebugTests.scala

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -45,32 +45,26 @@ object DebugTests extends ParallelTesting:
4545
private def verifyDebug(dir: JFile, testSource: TestSource, warnings: Int, reporters: Seq[TestReporter], logger: LoggedRunnable) =
4646
if Properties.testsNoRun then addNoRunWarning()
4747
else
48-
val checkFile = testSource.checkFile.getOrElse(throw new Exception("Missing check file"))
48+
val checkFile = testSource.checkFile.getOrElse(throw new Exception("Missing check file")).toPath
4949
val debugSteps = DebugStepAssert.parseCheckFile(checkFile)
50-
val expressionEvaluator = ExpressionEvaluator(testSource.sourceFiles, testSource.flags, testSource.runClassPath, testSource.outDir)
51-
val status = debugMain(testSource.runClassPath): debuggee =>
52-
val debugger = Debugger(debuggee.jdiPort, expressionEvaluator, maxDuration/* , verbose = true */)
53-
// configure the breakpoints before starting the debuggee
54-
val breakpoints = debugSteps.map(_.step).collect { case b: DebugStep.Break => b }
55-
for b <- breakpoints do debugger.configureBreakpoint(b.className, b.line)
56-
try
57-
debuggee.launch()
58-
playDebugSteps(debugger, debugSteps/* , verbose = true */)
59-
finally
60-
// stop debugger to let debuggee terminate its execution
61-
debugger.dispose()
62-
status match
63-
case Success(output) => ()
64-
case Failure(output) =>
65-
if output == "" then
66-
echo(s"Test '${testSource.title}' failed with no output")
67-
else
68-
echo(s"Test '${testSource.title}' failed with output:")
69-
echo(output)
70-
failTestSource(testSource)
71-
case Timeout =>
72-
echo("failed because test " + testSource.title + " timed out")
73-
failTestSource(testSource, TimeoutFailure(testSource.title))
50+
val expressionEvaluator =
51+
ExpressionEvaluator(testSource.sourceFiles, testSource.flags, testSource.runClassPath, testSource.outDir)
52+
try
53+
val status = debugMain(testSource.runClassPath): debuggee =>
54+
val debugger = Debugger(debuggee.jdiPort, expressionEvaluator, maxDuration/* , verbose = true */)
55+
// configure the breakpoints before starting the debuggee
56+
val breakpoints = debugSteps.map(_.step).collect { case b: DebugStep.Break => b }
57+
for b <- breakpoints do debugger.configureBreakpoint(b.className, b.line)
58+
try
59+
debuggee.launch()
60+
playDebugSteps(debugger, debugSteps/* , verbose = true */)
61+
finally
62+
// stop debugger to let debuggee terminate its execution
63+
debugger.dispose()
64+
reportDebuggeeStatus(testSource, status)
65+
catch case DebugStepException(message, location) =>
66+
echo(s"\nDebug step failed: $location\n" + message)
67+
failTestSource(testSource)
7468
end verifyDebug
7569

7670
private def playDebugSteps(debugger: Debugger, steps: Seq[DebugStepAssert[?]], verbose: Boolean = false): Unit =
@@ -109,4 +103,18 @@ object DebugTests extends ParallelTesting:
109103
if verbose then println(s"eval $expr $result")
110104
assert(result)
111105
end playDebugSteps
106+
107+
private def reportDebuggeeStatus(testSource: TestSource, status: Status): Unit =
108+
status match
109+
case Success(output) => ()
110+
case Failure(output) =>
111+
if output == "" then
112+
echo(s"Test '${testSource.title}' failed with no output")
113+
else
114+
echo(s"Test '${testSource.title}' failed with output:")
115+
echo(output)
116+
failTestSource(testSource)
117+
case Timeout =>
118+
echo("failed because test " + testSource.title + " timed out")
119+
failTestSource(testSource, TimeoutFailure(testSource.title))
112120
end DebugTest

0 commit comments

Comments
 (0)