Skip to content
This repository was archived by the owner on Sep 8, 2022. It is now read-only.

Issue/55 timedout #85

Closed
wants to merge 4 commits into from
Closed
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
1 change: 1 addition & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ scalaVersionsByJvm in ThisBuild := {
scalaXmlVersion := "1.0.6"

scalacOptions += "-Xfatal-warnings"
scalacOptions ++= "-Xelide-below" :: "0" :: "-Ywarn-unused" :: Nil
enableOptimizer

// dependencies
Expand Down
2 changes: 1 addition & 1 deletion project/build.properties
Original file line number Diff line number Diff line change
@@ -1 +1 @@
sbt.version=0.13.15
sbt.version=0.13.16
41 changes: 23 additions & 18 deletions src/main/scala/scala/tools/partest/nest/AbstractRunner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ abstract class AbstractRunner {
oempty(p, f, s) mkString ", "
}

private[this] def isSuccess = failedTests.size == expectedFailures
private[this] def isSuccess = failedTests.size == expectedFailures && failedTests.size + passedTests.size > 0

def issueSummaryReport() {
// Don't run twice
Expand All @@ -87,12 +87,12 @@ abstract class AbstractRunner {
echo(state.transcriptString + "\n")
}
}

def files_s = failed0.map(_.testFile).mkString(""" \""" + "\n ")
echo("# Failed test paths (this command will update checkfiles)")
echo(partestCmd + " --update-check \\\n " + files_s + "\n")
if (nestUI.verbose || failed0.size <= 50) {
def files_s = failed0.map(_.testFile).mkString(""" \""" + "\n ")
echo("# Failed test paths (this command will update checkfiles)")
echo(partestCmd + " --update-check \\\n " + files_s + "\n")
}
}

if (printSummary) {
echo(message)
levyJudgment()
Expand All @@ -118,10 +118,7 @@ abstract class AbstractRunner {
if (!nestUI.terse)
nestUI.echo(suiteRunner.banner)

val partestTests = (
if (config.optSelfTest) TestKinds.testsForPartest
else Nil
)
val partestTests = if (config.optSelfTest) TestKinds.testsForPartest else Nil

val grepExpr = config.optGrep getOrElse ""

Expand All @@ -140,9 +137,11 @@ abstract class AbstractRunner {
def miscTests = partestTests ++ individualTests ++ greppedTests ++ rerunTests

val givenKinds = standardKinds filter config.parsed.isSet
// named kinds to run, or else --all kinds, unless individual tests were specified
// (by path, --grep, --failed, even if there were invalid files specified)
val kinds = (
if (givenKinds.nonEmpty) givenKinds
else if (miscTests.isEmpty && invalid.isEmpty) standardKinds // If no kinds, --grep, or individual tests were given, assume --all, unless there were invalid files specified
else if (miscTests.isEmpty && invalid.isEmpty) standardKinds
else Nil
)
val kindsTests = kinds flatMap testsFor
Expand All @@ -168,18 +167,24 @@ abstract class AbstractRunner {
val expectedFailureMessage = if (expectedFailures == 0) "" else s" (expecting $expectedFailures to fail)"
echo(s"Selected $totalTests tests drawn from $testContributors$expectedFailureMessage\n")

var limping = false
val (_, millis) = timed {
for ((kind, paths) <- grouped) {
val num = paths.size
val ss = if (num == 1) "" else "s"
comment(s"starting $num test$ss in $kind")
val results = suiteRunner.runTestsForFiles(paths map (_.jfile.getAbsoluteFile), kind)
val (passed, failed) = results partition (_.isOk)

passedTests ++= passed
failedTests ++= failed
if (failed.nonEmpty) {
comment(passFailString(passed.size, failed.size, 0) + " in " + kind)
if (limping) comment(s"suite already in failure, skipping") else {
val results = suiteRunner.runTestsForFiles(paths.map(_.jfile.getAbsoluteFile)) match {
case Left((_, states)) => limping = true ; states
case Right(states) => states
}
val (passed, failed) = results partition (_.isOk)

passedTests ++= passed
failedTests ++= failed
if (failed.nonEmpty) {
comment(passFailString(passed.size, failed.size, 0) + " in " + kind)
}
}
echo("")
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/scala/tools/partest/nest/AntRunner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ abstract class AntRunner(srcDir: String, testClassLoader: URLClassLoader, javaCm
else {
log(s"Running ${files.length} tests in '$kind' at $now")
// log(s"Tests: ${files.toList}")
val results = runTestsForFiles(files, kind)
val results = runTestsForFiles(files).fold(x => x._2, ss => ss)
val (passed, failed) = results partition (_.isOk)
val numPassed = passed.size
val numFailed = failed.size
Expand Down
6 changes: 3 additions & 3 deletions src/main/scala/scala/tools/partest/nest/DirectCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ package scala.tools.partest
package nest

import scala.collection.mutable.ListBuffer
import scala.tools.nsc.{ Global, Settings, CompilerCommand }
import scala.tools.nsc.reporters.{ Reporter, ConsoleReporter }
import scala.tools.nsc.{Global, Settings, CompilerCommand}
import scala.tools.nsc.reporters.{Reporter, ConsoleReporter}
import scala.reflect.io.AbstractFile
import java.io.{ PrintWriter, FileWriter }
import java.io.{PrintWriter, FileWriter}

class ExtConsoleReporter(settings: Settings, val writer: PrintWriter) extends ConsoleReporter(settings, Console.in, writer) {
shortname = true
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/scala/tools/partest/nest/FileManager.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
package scala.tools.partest
package nest

import java.io.{ File, IOException }
import java.io.{File, IOException}
import java.net.URLClassLoader

object FileManager {
Expand Down
4 changes: 2 additions & 2 deletions src/main/scala/scala/tools/partest/nest/PathSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ object PathSettings {
// Directory <root>/test/files or .../scaladoc
def srcDir = Directory(testRoot / testSourcePath toCanonical)

def srcSpecLib = findJar("instrumented", Directory(srcDir / "speclib"))
def srcCodeLib = findJar("code", Directory(srcDir / "codelib"), Directory(testRoot / "files" / "codelib") /* work with --srcpath pending */)
def srcSpecLib = findJar("instrumented", Directory(srcDir / "speclib"))
def srcCodeLib = findJar("code", Directory(srcDir / "codelib"), Directory(testRoot / "files" / "codelib") /* work with --srcpath pending */)
}
110 changes: 56 additions & 54 deletions src/main/scala/scala/tools/partest/nest/Runner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,23 @@
package scala.tools.partest
package nest

import java.io.{ Console => _, _ }
import java.util.concurrent.Executors
import java.io.{Console => _, _}
import java.util.concurrent.{Executors, ExecutionException}
import java.util.concurrent.TimeUnit.NANOSECONDS
import scala.collection.mutable.ListBuffer
import scala.concurrent.duration.Duration
import scala.reflect.internal.FatalError
import scala.reflect.internal.util.ScalaClassLoader
import scala.sys.process.{ Process, ProcessLogger }
import scala.tools.nsc.Properties.{ envOrNone, isWin, javaHome, propOrEmpty, versionMsg, javaVmName, javaVmVersion, javaVmInfo }
import scala.tools.nsc.{ Settings, CompilerCommand, Global }
import scala.sys.process.{Process, ProcessLogger}
import scala.tools.nsc.Properties.{envOrNone, isWin, javaHome, propOrEmpty, versionMsg, javaVmName, javaVmVersion, javaVmInfo}
import scala.tools.nsc.{Settings, CompilerCommand, Global}
import scala.tools.nsc.reporters.ConsoleReporter
import scala.tools.nsc.util.stackTraceString
import scala.util.{ Try, Success, Failure }
import scala.util.{Try, Success, Failure}
import ClassPath.join
import TestState.{ Pass, Fail, Crash, Uninitialized, Updated }
import TestState.{Pass, Fail, Crash, Uninitialized, Updated}

import FileManager.{ compareContents, joinPaths, withTempFile }
import FileManager.{compareContents, joinPaths, withTempFile}

trait TestInfo {
/** pos/t1234 */
Expand Down Expand Up @@ -63,11 +63,11 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
// except for a . per passing test to show progress.
def isEnumeratedTest = false

private var _lastState: TestState = null
private val _transcript = new TestTranscript
private[this] var _lastState: TestState = null
private[this] val _transcript = new TestTranscript

def lastState = if (_lastState == null) Uninitialized(testFile) else _lastState
def setLastState(s: TestState) = _lastState = s
def lastState_=(s: TestState) = _lastState = s
def transcript: List[String] = _transcript.fail ++ logFile.fileLines
def pushTranscript(msg: String) = _transcript add msg

Expand All @@ -93,7 +93,7 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
System.err.println(stackTraceString(t))
}
protected def crashHandler: PartialFunction[Throwable, TestState] = {
case t: InterruptedException =>
case _: InterruptedException =>
genTimeout()
case t: Throwable =>
showCrashInfo(t)
Expand Down Expand Up @@ -141,7 +141,7 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
*/
def nextTestAction[T](body: => T)(failFn: PartialFunction[T, TestState]): T = {
val result = body
setLastState( if (failFn isDefinedAt result) failFn(result) else genPass() )
lastState = failFn.applyOrElse(result, (_: T) => genPass())
result
}
def nextTestActionExpectTrue(reason: String, body: => Boolean): Boolean = (
Expand Down Expand Up @@ -204,15 +204,14 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
protected def runCommand(args: Seq[String], outFile: File): Boolean = {
//(Process(args) #> outFile !) == 0 or (Process(args) ! pl) == 0
val pl = ProcessLogger(outFile)
val nonzero = 17 // rounding down from 17.3
def run: Int = {
val p = Process(args) run pl
try p.exitValue
catch {
case e: InterruptedException =>
case ie: InterruptedException =>
nestUI.verbose(s"Interrupted waiting for command to finish (${args mkString " "})")
p.destroy
nonzero
throw ie
case t: Throwable =>
nestUI.verbose(s"Exception waiting for command to finish: $t (${args mkString " "})")
p.destroy
Expand Down Expand Up @@ -312,7 +311,7 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU

"\n" + diff
}
catch { case t: Exception => None }
catch { case _: Exception => None }
}

/** Normalize the log output by applying test-specific filters
Expand Down Expand Up @@ -399,9 +398,6 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
else diff
_transcript append bestDiff
genFail("output differs")
// TestState.fail("output differs", "output differs",
// genFail("output differs")
// TestState.Fail("output differs", bestDiff)
case None => genPass() // redundant default case
} getOrElse true
}
Expand Down Expand Up @@ -636,7 +632,7 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
// and just looking at the diff, so I made them all do that
// because this is long enough.
if (!Output.withRedirected(logWriter)(try loop() finally resReader.close()))
setLastState(genPass())
lastState = genPass()

(diffIsOk, LogContext(logFile, swr, wr))
}
Expand All @@ -645,16 +641,22 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
// javac runner, for one, would merely append to an existing log file, so just delete it before we start
logFile.delete()

if (kind == "neg" || (kind endsWith "-neg")) runNegTest()
else kind match {
case "pos" => runTestCommon(true)
case "ant" => runAntTest()
case "res" => runResidentTest()
case "scalap" => runScalapTest()
case "script" => runScriptTest()
case _ => runTestCommon(execTest(outDir, logFile) && diffIsOk)
}
def runWhatever() =
if (kind == "neg" || (kind endsWith "-neg")) runNegTest()
else kind match {
case "pos" => runTestCommon(true)
case "ant" => runAntTest()
case "res" => runResidentTest()
case "scalap" => runScalapTest()
case "script" => runScriptTest()
case _ => runTestCommon(execTest(outDir, logFile) && diffIsOk)
}

try runWhatever()
catch {
case _: InterruptedException => lastState = genTimeout()
case t: Throwable => lastState = genCrash(t)
}
lastState
}

Expand Down Expand Up @@ -731,7 +733,7 @@ class SuiteRunner(
val javaOpts: String = PartestDefaults.javaOpts,
val scalacOpts: String = PartestDefaults.scalacOpts) {

import PartestDefaults.{ numThreads, waitTime }
import PartestDefaults.{numThreads, waitTime}

setUncaughtHandler

Expand Down Expand Up @@ -759,7 +761,7 @@ class SuiteRunner(
// |Java Classpath: ${sys.props("java.class.path")}
}

def onFinishTest(testFile: File, result: TestState): TestState = result
def onFinishTest(testFile: File, result: TestState): TestState = { unused(testFile) ; result }
Copy link
Member

Choose a reason for hiding this comment

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

This is also wrong, imho..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dwijnand do you prefer pre or post syntax? an annotation to ignore or a reporter that knows to ignore it? This is a middle solution.

Copy link
Member

Choose a reason for hiding this comment

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

I prefer the pre-syntax. IMO the unused param checker shouldn't warn on non-effectively final methods.

(Sorry I'm a little salty atm as I really want to use "warn unused"/Xlint in sbt/sbt but I've been dealing with lots and lots of these..)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finding the sweet spot for usability is hard, so input is appreciated. Warning on params was always assumed to be dicey. It sounds like: warn on final, unless asking for non-strict. Then, let me disable it with an annotation if I want.

I proposed this solution here because there's no extra machinery.

Copy link
Member

Choose a reason for hiding this comment

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

Btw another false positive (ish) is enriching objects, e.g implicit def enrichFoo(f: Foo.type).


def runTest(testFile: File): TestState = {
val runner = new Runner(testFile, this, nestUI)
Expand All @@ -770,47 +772,47 @@ class SuiteRunner(
if (failed && !runner.logFile.canRead)
runner.genPass()
else {
val (state, _) =
try timed(runner.run())
catch {
case t: Throwable => throw new RuntimeException(s"Error running $testFile", t)
}
val (state, time) = timed(runner.run())
nestUI.verbose(s"$testFile completed in $time")
nestUI.reportTest(state, runner)
runner.cleanup()
state
}
onFinishTest(testFile, state)
}

def runTestsForFiles(kindFiles: Array[File], kind: String): Array[TestState] = {
def runTestsForFiles(kindFiles: Array[File]): Either[(Exception, Array[TestState]), Array[TestState]] = {
nestUI.resetTestNumber(kindFiles.size)

val pool = Executors newFixedThreadPool numThreads
val futures = kindFiles map (f => pool submit callable(runTest(f.getAbsoluteFile)))
val pool = Executors.newFixedThreadPool(numThreads)
val futures = kindFiles.map(f => pool.submit(callable(runTest(f.getAbsoluteFile))))

pool.shutdown()
Try (pool.awaitTermination(waitTime) {
throw TimeoutException(waitTime)
}) match {
case Success(_) => futures map (_.get)
case Failure(e) =>
Try (pool.awaitTermination(waitTime)(throw TimeoutException(waitTime))) match {
case Success(_) => Right(futures.map(_.get))
case Failure(e: Exception) =>
e match {
case TimeoutException(d) =>
nestUI.warning("Thread pool timeout elapsed before all tests were complete!")
case TimeoutException(_) =>
nestUI.echoWarning("Thread pool timeout elapsed before all tests were complete!")
case ie: InterruptedException =>
nestUI.warning("Thread pool was interrupted")
nestUI.echoWarning("Thread pool was interrupted")
ie.printStackTrace()
}
pool.shutdownNow() // little point in continuing
// try to get as many completions as possible, in case someone cares
val results = for (f <- futures) yield {
pool.shutdownNow()
val states = (kindFiles, futures).zipped.map {(testFile, future) =>
try {
Some(f.get(0, NANOSECONDS))
if (future.isDone) future.get(0, NANOSECONDS) else {
val s = Uninitialized(testFile)
onFinishTest(testFile, s) // reported as pending
s
}
} catch {
case _: Throwable => None
case e: ExecutionException => new Runner(testFile, this, nestUI).genCrash(e.getCause)
case _: Throwable => new Runner(testFile, this, nestUI).genTimeout()
}
}
results.flatten
Left((e, states))
case Failure(t) => throw t
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/main/scala/scala/tools/partest/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

package scala.tools

import java.util.concurrent.{ Callable, ExecutorService }
import java.util.concurrent.{Callable, ExecutorService}
import scala.annotation.elidable, elidable.ALL
import scala.concurrent.duration.Duration
import scala.sys.process.javaVmArguments
import scala.tools.nsc.util.Exceptional
Expand Down Expand Up @@ -104,7 +105,7 @@ package object partest {

implicit class ExecutorOps(val executor: ExecutorService) {
def awaitTermination[A](wait: Duration)(failing: => A = ()): Option[A] = (
if (executor awaitTermination (wait.length, wait.unit)) None
if (executor.awaitTermination(wait.length, wait.unit)) None
else Some(failing)
)
}
Expand Down Expand Up @@ -180,4 +181,7 @@ package object partest {
import scala.collection.JavaConverters._
System.getProperties.asScala.toList.sorted map { case (k, v) => "%s -> %s\n".format(k, v) } mkString ""
}

// consume a value elidably to avoid unused warning
@elidable(ALL) def unused[A](a: A): A = a
}