Skip to content

Test can be skipped #15463

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

Merged
merged 3 commits into from
Oct 25, 2022
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
4 changes: 4 additions & 0 deletions compiler/test/dotty/tools/dotc/reporting/TestReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ extends Reporter with UniqueMessagePositions with HideNonSensicalMessages with M
private var _didCrash = false
final def compilerCrashed: Boolean = _didCrash

private var _skip: Boolean = false
final def setSkip(): Unit = _skip = true
final def skipped: Boolean = _skip

protected final def inlineInfo(pos: SourcePosition)(using Context): String =
if (pos.exists) {
if (pos.outer.exists)
Expand Down
110 changes: 60 additions & 50 deletions compiler/test/dotty/tools/vulpix/ParallelTesting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -281,10 +281,12 @@ trait ParallelTesting extends RunnerOrchestration { self =>
private final def onComplete(testSource: TestSource, reportersOrCrash: Try[Seq[TestReporter]], logger: LoggedRunnable): Unit =
reportersOrCrash match {
case TryFailure(exn) => onFailure(testSource, Nil, logger, Some(s"Fatal compiler crash when compiling: ${testSource.title}:\n${exn.getMessage}${exn.getStackTrace.map("\n\tat " + _).mkString}"))
case TrySuccess(reporters) => maybeFailureMessage(testSource, reporters) match {
case Some(msg) => onFailure(testSource, reporters, logger, Option(msg).filter(_.nonEmpty))
case None => onSuccess(testSource, reporters, logger)
}
case TrySuccess(reporters) if !reporters.exists(_.skipped) =>
maybeFailureMessage(testSource, reporters) match {
case Some(msg) => onFailure(testSource, reporters, logger, Option(msg).filter(_.nonEmpty))
case None => onSuccess(testSource, reporters, logger)
}
case _ =>
}

/**
Expand Down Expand Up @@ -391,6 +393,10 @@ trait ParallelTesting extends RunnerOrchestration { self =>
/** Number of failed tests */
def failureCount: Int = _failureCount

private var _skipCount = 0
protected final def registerSkip(): Unit = synchronized { _skipCount += 1 }
def skipCount: Int = _skipCount

protected def logBuildInstructions(testSource: TestSource, reporters: Seq[TestReporter]) = {
val (errCount, warnCount) = countErrorsAndWarnings(reporters)
val errorMsg = testSource.buildInstructions(errCount, warnCount)
Expand Down Expand Up @@ -464,13 +470,13 @@ trait ParallelTesting extends RunnerOrchestration { self =>
val toolArgs = toolArgsFor(files.toList.map(_.toPath), getCharsetFromEncodingOpt(flags0))

val spec = raw"(\d+)(\+)?".r
val testFilter = toolArgs.get(ToolName.Test) match
val testIsFiltered = toolArgs.get(ToolName.Test) match
case Some("-jvm" :: spec(n, more) :: Nil) =>
if more == "+" then isJavaAtLeast(n) else javaSpecVersion == n
case Some(args) => throw new IllegalStateException(args.mkString("unknown test option: ", ", ", ""))
case None => true

def scalacOptions = toolArgs.get(ToolName.Scalac).getOrElse(Nil)
def scalacOptions = toolArgs.getOrElse(ToolName.Scalac, Nil)

val flags = flags0
.and(scalacOptions: _*)
Expand Down Expand Up @@ -509,7 +515,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>

val allArgs = flags.all

if testFilter then
if testIsFiltered then
// If a test contains a Java file that cannot be parsed by Dotty's Java source parser, its
// name must contain the string "JAVA_ONLY".
val dottyFiles = files.filterNot(_.getName.contains("JAVA_ONLY")).map(_.getPath)
Expand All @@ -523,6 +529,9 @@ trait ParallelTesting extends RunnerOrchestration { self =>
echo(s"\njava compilation failed: \n${ javaErrors.get }")
fail(failure = JavaCompilationFailure(javaErrors.get))
}
else
registerSkip()
reporter.setSkip()
end if

reporter
Expand Down Expand Up @@ -724,7 +733,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
}

private def verifyOutput(checkFile: Option[JFile], dir: JFile, testSource: TestSource, warnings: Int, reporters: Seq[TestReporter], logger: LoggedRunnable) = {
if (Properties.testsNoRun) addNoRunWarning()
if Properties.testsNoRun then addNoRunWarning()
else runMain(testSource.runClassPath, testSource.allToolArgs) match {
case Success(output) => checkFile match {
case Some(file) if file.exists => diffTest(testSource, file, output.linesIterator.toList, reporters, logger)
Expand All @@ -748,7 +757,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
extends Test(testSources, times, threadLimit, suppressAllOutput) {
override def suppressErrors = true

override def maybeFailureMessage(testSource: TestSource, reporters: Seq[TestReporter]): Option[String] = {
override def maybeFailureMessage(testSource: TestSource, reporters: Seq[TestReporter]): Option[String] =
def compilerCrashed = reporters.exists(_.compilerCrashed)
lazy val (errorMap, expectedErrors) = getErrorMapAndExpectedCount(testSource.sourceFiles.toIndexedSeq)
lazy val actualErrors = reporters.foldLeft(0)(_ + _.errorCount)
Expand All @@ -772,20 +781,21 @@ trait ParallelTesting extends RunnerOrchestration { self =>
else if !errorMap.isEmpty then s"\nExpected error(s) have {<error position>=<unreported error>}: $errorMap"
else null
}
}
end maybeFailureMessage

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

def reporterOutputLines(reporters: Seq[TestReporter]): List[String] =
reporters.flatMap(_.consoleOutput.split("\n")).toList

// In neg-tests we allow two types of error annotations,
// "nopos-error" which doesn't care about position and "error" which
// has to be annotated on the correct line number.
// In neg-tests we allow two or three types of error annotations.
// Normally, `// error` must be annotated on the correct line number.
// `// nopos-error` allows for an error reported without a position.
// `// anypos-error` allows for an error reported with a position that can't be annotated in the check file.
//
// We collect these in a map `"file:row" -> numberOfErrors`, for
// nopos errors we save them in `"file" -> numberOfNoPosErrors`
// nopos and anypos errors we save them in `"file" -> numberOfNoPosErrors`
def getErrorMapAndExpectedCount(files: Seq[JFile]): (HashMap[String, Integer], Int) =
val comment = raw"//( *)(nopos-|anypos-)?error".r
val errorMap = new HashMap[String, Integer]()
Expand Down Expand Up @@ -950,8 +960,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
* ===============
* Since this is a parallel test suite, it is essential to be able to
* compose tests to take advantage of the concurrency. This is done using
* the `+` function. This function will make sure that tests being combined
* are compatible according to the `require`s in `+`.
* `aggregateTests` in the companion, which will ensure that aggregation is allowed.
*/
final class CompilationTest private (
private[ParallelTesting] val targets: List[TestSource],
Expand All @@ -969,6 +978,14 @@ trait ParallelTesting extends RunnerOrchestration { self =>
def this(targets: List[TestSource]) =
this(targets, 1, true, None, false, false)

def copy(targets: List[TestSource],
times: Int = times,
shouldDelete: Boolean = shouldDelete,
threadLimit: Option[Int] = threadLimit,
shouldFail: Boolean = shouldFail,
shouldSuppressOutput: Boolean = shouldSuppressOutput): CompilationTest =
CompilationTest(targets, times, shouldDelete, threadLimit, shouldFail, shouldSuppressOutput)

/** Creates a "pos" test run, which makes sure that all tests pass
* compilation without generating errors and that they do not crash the
* compiler
Expand All @@ -981,31 +998,29 @@ trait ParallelTesting extends RunnerOrchestration { self =>
if (!shouldFail && test.didFail) {
fail(s"Expected no errors when compiling, failed for the following reason(s):\n${reasonsForFailure(test)}\n")
}
else if (shouldFail && !test.didFail) {
else if (shouldFail && !test.didFail && test.skipCount == 0) {
fail("Pos test should have failed, but didn't")
}

this
}

/** Creates a "neg" test run, which makes sure that each test generates the
* correct amount of errors at the correct positions. It also makes sure
* that none of these tests crash the compiler
* correct number of errors at the correct positions. It also makes sure
* that none of these tests crashes the compiler.
*/
def checkExpectedErrors()(implicit summaryReport: SummaryReporting): this.type = {
def checkExpectedErrors()(implicit summaryReport: SummaryReporting): this.type =
val test = new NegTest(targets, times, threadLimit, shouldFail || shouldSuppressOutput).executeTestSuite()

cleanup()

if (shouldFail && !test.didFail) {
if shouldFail && !test.didFail && test.skipCount == 0 then
fail(s"Neg test shouldn't have failed, but did. Reasons:\n${ reasonsForFailure(test) }")
}
else if (!shouldFail && test.didFail) {
else if !shouldFail && test.didFail then
fail("Neg test should have failed, but did not")
}

this
}
end checkExpectedErrors

/** Creates a "fuzzy" test run, which makes sure that each test compiles (or not) without crashing */
def checkNoCrash()(implicit summaryReport: SummaryReporting): this.type = {
Expand All @@ -1030,12 +1045,10 @@ trait ParallelTesting extends RunnerOrchestration { self =>

cleanup()

if (!shouldFail && test.didFail) {
if !shouldFail && test.didFail then
fail(s"Run test failed, but should not, reasons:\n${ reasonsForFailure(test) }")
}
else if (shouldFail && !test.didFail) {
else if shouldFail && !test.didFail && test.skipCount == 0 then
fail("Run test should have failed, but did not")
}

this
}
Expand Down Expand Up @@ -1160,35 +1173,32 @@ trait ParallelTesting extends RunnerOrchestration { self =>
}
}

object CompilationTest {
object CompilationTest:

/** Compose test targets from `tests`
*
* It does this, only if the two tests are compatible. Otherwise it throws
* an `IllegalArgumentException`.
*
* Grouping tests together like this allows us to take advantage of the
* concurrency offered by this test suite as each call to an executing
* method (`pos()` / `checkExpectedErrors()`/ `run()`) will spin up a thread pool with the
* maximum allowed level of concurrency. Doing this for only a few targets
* does not yield any real benefit over sequential compilation.
*
* As such, each `CompilationTest` should contain as many targets as
* possible.
*/
def aggregateTests(tests: CompilationTest*): CompilationTest = {
*
* It does this, only if all the tests are mutally compatible.
* Otherwise it throws an `IllegalArgumentException`.
*
* Grouping tests together like this allows us to take advantage of the
* concurrency offered by this test suite, as each call to an executing
* method (`pos()` / `checkExpectedErrors()`/ `run()`) will spin up a thread pool with the
* maximum allowed level of concurrency. Doing this for only a few targets
* does not yield any real benefit over sequential compilation.
*
* As such, each `CompilationTest` should contain as many targets as
* possible.
*/
def aggregateTests(tests: CompilationTest*): CompilationTest =
assert(tests.nonEmpty)
def aggregate(test1: CompilationTest, test2: CompilationTest) = {
def aggregate(test1: CompilationTest, test2: CompilationTest) =
require(test1.times == test2.times, "can't combine tests that are meant to be benchmark compiled")
require(test1.shouldDelete == test2.shouldDelete, "can't combine tests that differ on deleting output")
require(test1.shouldFail == test2.shouldFail, "can't combine tests that have different expectations on outcome")
require(test1.shouldSuppressOutput == test2.shouldSuppressOutput, "can't combine tests that both suppress and don't suppress output")
new CompilationTest(test1.targets ++ test2.targets, test1.times, test1.shouldDelete, test1.threadLimit, test1.shouldFail, test1.shouldSuppressOutput)
}
test1.copy(test1.targets ++ test2.targets) // what if thread limit differs? currently threads are limited on aggregate only
tests.reduce(aggregate)
}

}
end CompilationTest

/** Create out directory for directory `d` */
def createOutputDirsForDir(d: JFile, sourceDir: JFile, outDir: String): JFile = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// note that scala's java parser doesn't care about the platform version
class SCALA_ONLY_Invalid1 {

public static final String badOpeningDelimiter = """non-whitespace // error
Expand Down
File renamed without changes.
File renamed without changes.