Skip to content

Commit 5906ccf

Browse files
nicolasstuckinicolasstucki
authored
Merge pull request #6508 from dotty-staging/vulpix-checkfiles-identical
Make Vulpix checkfiles identical to the actual console output
2 parents 0e66a88 + 7924421 commit 5906ccf

21 files changed

+508
-307
lines changed

‎compiler/test/dotty/tools/dotc/reporting/TestReporter.scala

+7-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package dotty.tools
22
package dotc
33
package reporting
44

5-
import java.io.{ PrintStream, PrintWriter, File => JFile, FileOutputStream }
5+
import java.io.{ PrintStream, PrintWriter, File => JFile, FileOutputStream, StringWriter }
66
import java.text.SimpleDateFormat
77
import java.util.Date
88
import core.Decorators._
@@ -26,6 +26,10 @@ extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with M
2626
protected final val _messageBuf = mutable.ArrayBuffer.empty[String]
2727
final def messages: Iterator[String] = _messageBuf.iterator
2828

29+
protected final val _consoleBuf = new StringWriter
30+
protected final val _consoleReporter = new ConsoleReporter(null, new PrintWriter(_consoleBuf))
31+
final def consoleOutput: String = _consoleBuf.toString
32+
2933
private[this] var _didCrash = false
3034
final def compilerCrashed: Boolean = _didCrash
3135

@@ -63,6 +67,7 @@ extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with M
6367
}
6468

6569
override def doReport(m: MessageContainer)(implicit ctx: Context): Unit = {
70+
6671
// Here we add extra information that we should know about the error message
6772
val extra = m.contained() match {
6873
case pm: PatternMatchExhaustivity => s": ${pm.uncovered}"
@@ -72,6 +77,7 @@ extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with M
7277
m match {
7378
case m: Error => {
7479
_errorBuf.append(m)
80+
_consoleReporter.doReport(m)
7581
printMessageAndPos(m, extra)
7682
}
7783
case m =>

‎compiler/test/dotty/tools/vulpix/ParallelTesting.scala

+36-40
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
5858
def flags: TestFlags
5959
def sourceFiles: Array[JFile]
6060

61-
def runClassPath: String = outDir.getAbsolutePath + JFile.pathSeparator + flags.runClassPath
61+
def runClassPath: String = outDir.getPath + JFile.pathSeparator + flags.runClassPath
6262

6363
def title: String = self match {
6464
case self: JointCompilationSource =>
@@ -86,16 +86,18 @@ trait ParallelTesting extends RunnerOrchestration { self =>
8686
val sb = new StringBuilder
8787
val maxLen = 80
8888
var lineLen = 0
89+
val delimiter = " "
8990

9091
sb.append(
9192
s"""|
9293
|Test '$title' compiled with $errors error(s) and $warnings warning(s),
93-
|the test can be reproduced by running:""".stripMargin
94+
|the test can be reproduced by running from SBT (prefix it with ./bin/ if you
95+
|want to run from the command line):""".stripMargin
9496
)
95-
sb.append("\n\n./bin/dotc ")
97+
sb.append("\n\ndotc ")
9698
flags.all.foreach { arg =>
9799
if (lineLen > maxLen) {
98-
sb.append(" \\\n ")
100+
sb.append(delimiter)
99101
lineLen = 4
100102
}
101103
sb.append(arg)
@@ -105,8 +107,8 @@ trait ParallelTesting extends RunnerOrchestration { self =>
105107

106108
self match {
107109
case source: JointCompilationSource => {
108-
source.sourceFiles.map(_.getAbsolutePath).foreach { path =>
109-
sb.append("\\\n ")
110+
source.sourceFiles.map(_.getPath).foreach { path =>
111+
sb.append(delimiter)
110112
sb.append(path)
111113
sb += ' '
112114
}
@@ -117,7 +119,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
117119
val fsb = new StringBuilder(command)
118120
self.compilationGroups.foreach { files =>
119121
files.map(_.getPath).foreach { path =>
120-
fsb.append("\\\n ")
122+
fsb.append(delimiter)
121123
lineLen = 8
122124
fsb.append(path)
123125
fsb += ' '
@@ -215,21 +217,20 @@ trait ParallelTesting extends RunnerOrchestration { self =>
215217
*/
216218
final def checkFile(testSource: TestSource): Option[JFile] = (testSource match {
217219
case ts: JointCompilationSource =>
218-
ts.files.collectFirst { case f if !f.isDirectory => new JFile(f.getAbsolutePath.replaceFirst("\\.scala$", ".check")) }
220+
ts.files.collectFirst { case f if !f.isDirectory => new JFile(f.getPath.replaceFirst("\\.scala$", ".check")) }
219221

220222
case ts: SeparateCompilationSource =>
221-
Option(new JFile(ts.dir.getAbsolutePath + ".check"))
223+
Option(new JFile(ts.dir.getPath + ".check"))
222224
}).filter(_.exists)
223225

224226
/**
225227
* Checks if the given actual lines are the same as the ones in the check file.
226228
* If not, fails the test.
227229
*/
228-
final def diffTest(testSource: TestSource, checkFile: JFile, actual: List[String]) = {
230+
final def diffTest(testSource: TestSource, checkFile: JFile, actual: List[String], reporters: Seq[TestReporter], logger: LoggedRunnable) = {
229231
val expected = Source.fromFile(checkFile, "UTF-8").getLines().toList
230232
for (msg <- diffMessage(testSource.title, actual, expected)) {
231-
echo(msg)
232-
failTestSource(testSource)
233+
onFailure(testSource, reporters, logger, Some(msg))
233234
dumpOutputToFile(checkFile, actual)
234235
}
235236
}
@@ -318,9 +319,9 @@ trait ParallelTesting extends RunnerOrchestration { self =>
318319
if (!testFilter.isDefined) testSources
319320
else testSources.filter {
320321
case JointCompilationSource(_, files, _, _, _, _) =>
321-
files.exists(file => file.getAbsolutePath.contains(testFilter.get))
322+
files.exists(file => file.getPath.contains(testFilter.get))
322323
case SeparateCompilationSource(_, dir, _, _) =>
323-
dir.getAbsolutePath.contains(testFilter.get)
324+
dir.getPath.contains(testFilter.get)
324325
}
325326

326327
/** Total amount of test sources being compiled by this test */
@@ -422,9 +423,8 @@ trait ParallelTesting extends RunnerOrchestration { self =>
422423
}
423424

424425
protected def compile(files0: Array[JFile], flags0: TestFlags, suppressErrors: Boolean, targetDir: JFile): TestReporter = {
425-
426-
val flags = flags0.and("-d", targetDir.getAbsolutePath)
427-
.withClasspath(targetDir.getAbsolutePath)
426+
val flags = flags0.and("-d", targetDir.getPath)
427+
.withClasspath(targetDir.getPath)
428428

429429
def flattenFiles(f: JFile): Array[JFile] =
430430
if (f.isDirectory) f.listFiles.flatMap(flattenFiles)
@@ -468,10 +468,10 @@ trait ParallelTesting extends RunnerOrchestration { self =>
468468

469469
// If a test contains a Java file that cannot be parsed by Dotty's Java source parser, its
470470
// name must contain the string "JAVA_ONLY".
471-
val dottyFiles = files.filterNot(_.getName.contains("JAVA_ONLY")).map(_.getAbsolutePath)
471+
val dottyFiles = files.filterNot(_.getName.contains("JAVA_ONLY")).map(_.getPath)
472472
driver.process(allArgs ++ dottyFiles, reporter = reporter)
473473

474-
val javaFiles = files.filter(_.getName.endsWith(".java")).map(_.getAbsolutePath)
474+
val javaFiles = files.filter(_.getName.endsWith(".java")).map(_.getPath)
475475
val javaErrors = compileWithJavac(javaFiles)
476476

477477
if (javaErrors.isDefined) {
@@ -485,7 +485,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
485485
protected def compileFromTasty(flags0: TestFlags, suppressErrors: Boolean, targetDir: JFile): TestReporter = {
486486
val tastyOutput = new JFile(targetDir.getPath + "_from-tasty")
487487
tastyOutput.mkdir()
488-
val flags = flags0 and ("-d", tastyOutput.getAbsolutePath) and "-from-tasty"
488+
val flags = flags0 and ("-d", tastyOutput.getPath) and "-from-tasty"
489489

490490
def tastyFileToClassName(f: JFile): String = {
491491
val pathStr = targetDir.toPath.relativize(f.toPath).toString.replace(JFile.separatorChar, '.')
@@ -609,11 +609,11 @@ trait ParallelTesting extends RunnerOrchestration { self =>
609609
}
610610
}
611611

612-
private def verifyOutput(checkFile: Option[JFile], dir: JFile, testSource: TestSource, warnings: Int) = {
612+
private def verifyOutput(checkFile: Option[JFile], dir: JFile, testSource: TestSource, warnings: Int, reporters: Seq[TestReporter], logger: LoggedRunnable) = {
613613
if (Properties.testsNoRun) addNoRunWarning()
614614
else runMain(testSource.runClassPath) match {
615615
case Success(output) => checkFile match {
616-
case Some(file) if file.exists => diffTest(testSource, file, output.linesIterator.toList)
616+
case Some(file) if file.exists => diffTest(testSource, file, output.linesIterator.toList, reporters, logger)
617617
case _ =>
618618
}
619619
case Failure(output) =>
@@ -627,7 +627,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
627627
}
628628

629629
override def onSuccess(testSource: TestSource, reporters: Seq[TestReporter], logger: LoggedRunnable) =
630-
verifyOutput(checkFile(testSource), testSource.outDir, testSource, countWarnings(reporters))
630+
verifyOutput(checkFile(testSource), testSource.outDir, testSource, countWarnings(reporters), reporters, logger)
631631
}
632632

633633
private final class NegTest(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)(implicit summaryReport: SummaryReporting)
@@ -649,11 +649,10 @@ trait ParallelTesting extends RunnerOrchestration { self =>
649649
}
650650

651651
override def onSuccess(testSource: TestSource, reporters: Seq[TestReporter], logger: LoggedRunnable): Unit =
652-
checkFile(testSource).foreach(diffTest(testSource, _, reporterOutputLines(reporters)))
652+
checkFile(testSource).foreach(diffTest(testSource, _, reporterOutputLines(reporters), reporters, logger))
653653

654654
def reporterOutputLines(reporters: Seq[TestReporter]): List[String] =
655-
reporters.flatMap(_.allErrors).sortBy(_.pos.source.toString).flatMap { error =>
656-
(error.pos.span.toString + " in " + error.pos.source.file.name) :: error.getMessage().linesIterator.toList }.toList
655+
reporters.flatMap(_.consoleOutput.split("\n")).toList
657656

658657
// In neg-tests we allow two types of error annotations,
659658
// "nopos-error" which doesn't care about position and "error" which
@@ -668,7 +667,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
668667
Source.fromFile(file, "UTF-8").getLines().zipWithIndex.foreach { case (line, lineNbr) =>
669668
val errors = line.sliding("// error".length).count(_.mkString == "// error")
670669
if (errors > 0)
671-
errorMap.put(s"${file.getAbsolutePath}:${lineNbr}", errors)
670+
errorMap.put(s"${file.getPath}:${lineNbr}", errors)
672671

673672
val noposErrors = line.sliding("// nopos-error".length).count(_.mkString == "// nopos-error")
674673
if (noposErrors > 0) {
@@ -686,7 +685,9 @@ trait ParallelTesting extends RunnerOrchestration { self =>
686685

687686
def getMissingExpectedErrors(errorMap: HashMap[String, Integer], reporterErrors: Iterator[MessageContainer]) = !reporterErrors.forall { error =>
688687
val key = if (error.pos.exists) {
689-
val fileName = error.pos.source.file.toString
688+
def toRelative(path: String): String = // For some reason, absolute paths leak from the compiler itself...
689+
path.split("/").dropWhile(_ != "tests").mkString("/")
690+
val fileName = toRelative(error.pos.source.file.toString)
690691
s"$fileName:${error.pos.line}"
691692

692693
} else "nopos"
@@ -715,16 +716,11 @@ trait ParallelTesting extends RunnerOrchestration { self =>
715716
def linesMatch =
716717
(outputLines, checkLines).zipped.forall(_ == _)
717718

718-
if (outputLines.length != checkLines.length || !linesMatch) {
719-
// Print diff to files and summary:
720-
val diff = DiffUtil.mkColoredLineDiff(checkLines :+ DiffUtil.EOF, outputLines :+ DiffUtil.EOF)
721-
722-
Some(
723-
s"""|Output from '$sourceTitle' did not match check file.
724-
|Diff (expected on the left, actual right):
725-
|""".stripMargin + diff + "\n")
726-
} else None
727-
719+
if (outputLines.length != checkLines.length || !linesMatch) Some(
720+
s"""|Output from '$sourceTitle' did not match check file. Actual output:
721+
|${outputLines.mkString(EOL)}
722+
|""".stripMargin + "\n")
723+
else None
728724
}
729725

730726
/** The `CompilationTest` is the main interface to `ParallelTesting`, it
@@ -943,7 +939,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
943939
* and if so copying recursively
944940
*/
945941
private def copyToDir(dir: JFile, file: JFile): JFile = {
946-
val target = Paths.get(dir.getAbsolutePath, file.getName)
942+
val target = Paths.get(dir.getPath, file.getName)
947943
Files.copy(file.toPath, target, REPLACE_EXISTING)
948944
if (file.isDirectory) file.listFiles.map(copyToDir(target.toFile, _))
949945
target.toFile
@@ -1221,7 +1217,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
12211217
val (dirs, files) = compilationTargets(sourceDir, fromTastyFilter)
12221218

12231219
val filteredFiles = testFilter match {
1224-
case Some(str) => files.filter(_.getAbsolutePath.contains(str))
1220+
case Some(str) => files.filter(_.getPath.contains(str))
12251221
case None => files
12261222
}
12271223

‎docs/docs/contributing/testing.md

+24
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,30 @@ You can also run all paths of classes of a certain name:
5454
> testOnly *.TreeTransformerTest
5555
```
5656

57+
### Testing with checkfiles
58+
Some tests support checking the output of the run or the compilation against a checkfile. A checkfile is a file in which the expected output of the compilation or run is defined. A test against a checkfile fails if the actual output mismatches the expected output.
59+
60+
Currently, the `run` and `neg` (compilation must fail for the test to succeed) tests support the checkfiles. `run`'s checkfiles contain an expected run output of the successfully compiled program. `neg`'s checkfiles contain an expected error output during compilation.
61+
62+
Absence of a checkfile is **not** a condition for the test failure. E.g. if a `neg` test fails with the expected number of errors and there is no checkfile for it, the test still passes.
63+
64+
Checkfiles are located in the same directories as the tests they check, have the same name as these tests with the extension `*.check`. E.g. if you have a test named `tests/neg/foo.scala`, you can create a checkfile for it named `tests/neg/foo.check`. And if you have a test composed of several files in a single directory, e.g. `tests/neg/manyScalaFiles`, the checkfile will be `tests/neg/manyScalaFiles.check`.
65+
66+
If the actual output mismatches the expected output, the test framework will dump the actual output in the file `*.check.out` and fail the test suite. It will also output the instructions to quickly replace the expected output with the actual output, in the following format:
67+
68+
```
69+
Test output dumped in: tests/playground/neg/Sample.check.out
70+
See diff of the checkfile
71+
> diff tests/playground/neg/Sample.check tests/playground/neg/Sample.check.out
72+
Replace checkfile with current output output
73+
> mv tests/playground/neg/Sample.check.out tests/playground/neg/Sample.check
74+
```
75+
76+
To create a checkfile for a test, you can do one of the following:
77+
78+
- Create a dummy checkfile with a random content, run the test, and, when it fails, use the `mv` command reported by the test to replace the dummy checkfile with the actual output.
79+
- Manually compile the file you are testing with `dotc` and copy-paste whatever console output the compiler produces to the checkfile.
80+
5781
## Integration tests
5882
These tests are Scala source files expected to compile with Dotty (pos tests),
5983
along with their expected output (run tests) or errors (neg tests).

‎tests/neg-macros/i6432.check

+16-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
1-
[61..64] in Test_2.scala
2-
fgh
3-
[50..53] in Test_2.scala
4-
xyz
5-
[39..42] in Test_2.scala
6-
abc
1+
2+
-- Error: tests/neg-macros/i6432/Test_2.scala:4:6 ----------------------------------------------------------------------
3+
4 | foo"abc${"123"}xyz${"456"}fgh" // error // error // error
4+
| ^^^
5+
| abc
6+
| This location is in code that was inlined at Test_2.scala:4
7+
-- Error: tests/neg-macros/i6432/Test_2.scala:4:17 ---------------------------------------------------------------------
8+
4 | foo"abc${"123"}xyz${"456"}fgh" // error // error // error
9+
| ^^^
10+
| xyz
11+
| This location is in code that was inlined at Test_2.scala:4
12+
-- Error: tests/neg-macros/i6432/Test_2.scala:4:28 ---------------------------------------------------------------------
13+
4 | foo"abc${"123"}xyz${"456"}fgh" // error // error // error
14+
| ^^^
15+
| fgh
16+
| This location is in code that was inlined at Test_2.scala:4

‎tests/neg-macros/i6432b.check

+16-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
1-
[63..66] in Test_2.scala
2-
fgh
3-
[52..55] in Test_2.scala
4-
xyz
5-
[41..44] in Test_2.scala
6-
abc
1+
2+
-- Error: tests/neg-macros/i6432b/Test_2.scala:4:8 ---------------------------------------------------------------------
3+
4 | foo"""abc${"123"}xyz${"456"}fgh""" // error // error // error
4+
| ^^^
5+
| abc
6+
| This location is in code that was inlined at Test_2.scala:4
7+
-- Error: tests/neg-macros/i6432b/Test_2.scala:4:19 --------------------------------------------------------------------
8+
4 | foo"""abc${"123"}xyz${"456"}fgh""" // error // error // error
9+
| ^^^
10+
| xyz
11+
| This location is in code that was inlined at Test_2.scala:4
12+
-- Error: tests/neg-macros/i6432b/Test_2.scala:4:30 --------------------------------------------------------------------
13+
4 | foo"""abc${"123"}xyz${"456"}fgh""" // error // error // error
14+
| ^^^
15+
| fgh
16+
| This location is in code that was inlined at Test_2.scala:4

‎tests/neg/classOf.check

+14-8
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
1-
[181..202] in classOf.scala
2-
Test.C{I = String} is not a class type
3-
[116..117] in classOf.scala
4-
T is not a class type
5-
6-
where: T is a type in method f2 with bounds <: String
7-
[72..73] in classOf.scala
8-
T is not a class type
1+
-- Error: tests/neg/classOf.scala:6:22 ---------------------------------------------------------------------------------
2+
6 | def f1[T] = classOf[T] // error
3+
| ^
4+
| T is not a class type
5+
-- Error: tests/neg/classOf.scala:7:32 ---------------------------------------------------------------------------------
6+
7 | def f2[T <: String] = classOf[T] // error
7+
| ^
8+
| T is not a class type
9+
|
10+
| where: T is a type in method f2 with bounds <: String
11+
-- Error: tests/neg/classOf.scala:9:18 ---------------------------------------------------------------------------------
12+
9 | val y = classOf[C { type I = String }] // error
13+
| ^^^^^^^^^^^^^^^^^^^^^
14+
| Test.C{I = String} is not a class type

0 commit comments

Comments
 (0)