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

Commit 8b5f550

Browse files
committed
Fail after partest.timeout
The result `TestState` is determined when the test is run (rather than throwing exceptions to enclosing infrastructure). Tests which haven't completed are deemed pending. Previously, tests that hadn't completed were deemed skipped. But skipping tests doesn't fail the build. Sample output with very short partest.timeout: ``` Selected 1815 tests drawn from 1 named test categories ok 1 - run/Course-2002-01.scala Thread pool timeout elapsed before all tests were complete! !! 2 - run/Course-2002-03.scala [non-zero exit code] !! 4 - run/Course-2002-02.scala [non-zero exit code] !! 3 - run/Course-2002-04.scala [non-zero exit code] !! 5 - run/Course-2002-05.scala [non-zero exit code] !! 6 - run/Course-2002-06.scala [non-zero exit code] !! 7 - run/Course-2002-08.scala [non-zero exit code] partest --update-check \ /home/apm/projects/snytt/test/files/run/Course-2002-02.scala \ /home/apm/projects/snytt/test/files/run/Course-2002-03.scala \ /home/apm/projects/snytt/test/files/run/Course-2002-04.scala \ /home/apm/projects/snytt/test/files/run/Course-2002-05.scala \ /home/apm/projects/snytt/test/files/run/Course-2002-06.scala \ /home/apm/projects/snytt/test/files/run/Course-2002-07.scala \ /home/apm/projects/snytt/test/files/run/Course-2002-08.scala \ /home/apm/projects/snytt/test/files/run/Course-2002-09.scala \ /home/apm/projects/snytt/test/files/run/Course-2002-10.scala \ /home/apm/projects/snytt/test/files/run/Course-2002-13.scala \ /home/apm/projects/snytt/test/files/run/Meter.scala \ /home/apm/projects/snytt/test/files/run/MeterCaseClass.scala \ /home/apm/projects/snytt/test/files/run/MutableListTest.scala \ /home/apm/projects/snytt/test/files/run/NestedClasses.scala \ /home/apm/projects/snytt/test/files/run/OrderingTest.scala \ /home/apm/projects/snytt/test/files/run/Predef.readLine.scala \ [etc] ``` Incomplete tests (in the category running when interrupted) are reported as pending: ``` ok 38 - pos/cls1.scala Thread pool timeout elapsed before all tests were complete! ok 39 - pos/cls.scala !! 40 - pos/collectGenericCC.scala [compilation failed] !! 41 - pos/chang [timed out] ok 42 - pos/clsrefine.scala [error] Failed: Total 41, Failed 2, Errors 0, Passed 39, Pending 1371 [error] Failed tests: [error] partest [error] (test/it:testOnly) sbt.TestsFailedException: Tests unsuccessful [error] Total time: 14 s, completed Apr 12, 2017 10:37:58 AM ```
1 parent f56d0ee commit 8b5f550

File tree

4 files changed

+68
-61
lines changed

4 files changed

+68
-61
lines changed

src/main/scala/scala/tools/partest/nest/AbstractRunner.scala

+23-18
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ abstract class AbstractRunner {
6363
oempty(p, f, s) mkString ", "
6464
}
6565

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

6868
def issueSummaryReport() {
6969
// Don't run twice
@@ -87,12 +87,12 @@ abstract class AbstractRunner {
8787
echo(state.transcriptString + "\n")
8888
}
8989
}
90-
91-
def files_s = failed0.map(_.testFile).mkString(""" \""" + "\n ")
92-
echo("# Failed test paths (this command will update checkfiles)")
93-
echo(partestCmd + " --update-check \\\n " + files_s + "\n")
90+
if (nestUI.verbose || failed0.size <= 50) {
91+
def files_s = failed0.map(_.testFile).mkString(""" \""" + "\n ")
92+
echo("# Failed test paths (this command will update checkfiles)")
93+
echo(partestCmd + " --update-check \\\n " + files_s + "\n")
94+
}
9495
}
95-
9696
if (printSummary) {
9797
echo(message)
9898
levyJudgment()
@@ -118,10 +118,7 @@ abstract class AbstractRunner {
118118
if (!nestUI.terse)
119119
nestUI.echo(suiteRunner.banner)
120120

121-
val partestTests = (
122-
if (config.optSelfTest) TestKinds.testsForPartest
123-
else Nil
124-
)
121+
val partestTests = if (config.optSelfTest) TestKinds.testsForPartest else Nil
125122

126123
val grepExpr = config.optGrep getOrElse ""
127124

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

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

170+
var limping = false
171171
val (_, millis) = timed {
172172
for ((kind, paths) <- grouped) {
173173
val num = paths.size
174174
val ss = if (num == 1) "" else "s"
175175
comment(s"starting $num test$ss in $kind")
176-
val results = suiteRunner.runTestsForFiles(paths map (_.jfile.getAbsoluteFile))
177-
val (passed, failed) = results partition (_.isOk)
178-
179-
passedTests ++= passed
180-
failedTests ++= failed
181-
if (failed.nonEmpty) {
182-
comment(passFailString(passed.size, failed.size, 0) + " in " + kind)
176+
if (limping) comment(s"suite already in failure, skipping") else {
177+
val results = suiteRunner.runTestsForFiles(paths.map(_.jfile.getAbsoluteFile)) match {
178+
case Left((_, states)) => limping = true ; states
179+
case Right(states) => states
180+
}
181+
val (passed, failed) = results partition (_.isOk)
182+
183+
passedTests ++= passed
184+
failedTests ++= failed
185+
if (failed.nonEmpty) {
186+
comment(passFailString(passed.size, failed.size, 0) + " in " + kind)
187+
}
183188
}
184189
echo("")
185190
}

src/main/scala/scala/tools/partest/nest/AntRunner.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ abstract class AntRunner(srcDir: String, testClassLoader: URLClassLoader, javaCm
3333
else {
3434
log(s"Running ${files.length} tests in '$kind' at $now")
3535
// log(s"Tests: ${files.toList}")
36-
val results = runTestsForFiles(files)
36+
val results = runTestsForFiles(files).fold(x => x._2, ss => ss)
3737
val (passed, failed) = results partition (_.isOk)
3838
val numPassed = passed.size
3939
val numFailed = failed.size

src/main/scala/scala/tools/partest/nest/Runner.scala

+43-41
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ package scala.tools.partest
66
package nest
77

88
import java.io.{Console => _, _}
9-
import java.util.concurrent.Executors
9+
import java.util.concurrent.{Executors, ExecutionException}
1010
import java.util.concurrent.TimeUnit.NANOSECONDS
1111
import scala.collection.mutable.ListBuffer
1212
import scala.concurrent.duration.Duration
@@ -63,11 +63,11 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
6363
// except for a . per passing test to show progress.
6464
def isEnumeratedTest = false
6565

66-
private var _lastState: TestState = null
67-
private val _transcript = new TestTranscript
66+
private[this] var _lastState: TestState = null
67+
private[this] val _transcript = new TestTranscript
6868

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

@@ -141,7 +141,7 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
141141
*/
142142
def nextTestAction[T](body: => T)(failFn: PartialFunction[T, TestState]): T = {
143143
val result = body
144-
setLastState( if (failFn isDefinedAt result) failFn(result) else genPass() )
144+
lastState = failFn.applyOrElse(result, (_: T) => genPass())
145145
result
146146
}
147147
def nextTestActionExpectTrue(reason: String, body: => Boolean): Boolean = (
@@ -204,15 +204,14 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
204204
protected def runCommand(args: Seq[String], outFile: File): Boolean = {
205205
//(Process(args) #> outFile !) == 0 or (Process(args) ! pl) == 0
206206
val pl = ProcessLogger(outFile)
207-
val nonzero = 17 // rounding down from 17.3
208207
def run: Int = {
209208
val p = Process(args) run pl
210209
try p.exitValue
211210
catch {
212-
case _: InterruptedException =>
211+
case ie: InterruptedException =>
213212
nestUI.verbose(s"Interrupted waiting for command to finish (${args mkString " "})")
214213
p.destroy
215-
nonzero
214+
throw ie
216215
case t: Throwable =>
217216
nestUI.verbose(s"Exception waiting for command to finish: $t (${args mkString " "})")
218217
p.destroy
@@ -399,9 +398,6 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
399398
else diff
400399
_transcript append bestDiff
401400
genFail("output differs")
402-
// TestState.fail("output differs", "output differs",
403-
// genFail("output differs")
404-
// TestState.Fail("output differs", bestDiff)
405401
case None => genPass() // redundant default case
406402
} getOrElse true
407403
}
@@ -636,7 +632,7 @@ class Runner(val testFile: File, val suiteRunner: SuiteRunner, val nestUI: NestU
636632
// and just looking at the diff, so I made them all do that
637633
// because this is long enough.
638634
if (!Output.withRedirected(logWriter)(try loop() finally resReader.close()))
639-
setLastState(genPass())
635+
lastState = genPass()
640636

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

648-
if (kind == "neg" || (kind endsWith "-neg")) runNegTest()
649-
else kind match {
650-
case "pos" => runTestCommon(true)
651-
case "ant" => runAntTest()
652-
case "res" => runResidentTest()
653-
case "scalap" => runScalapTest()
654-
case "script" => runScriptTest()
655-
case _ => runTestCommon(execTest(outDir, logFile) && diffIsOk)
656-
}
644+
def runWhatever() =
645+
if (kind == "neg" || (kind endsWith "-neg")) runNegTest()
646+
else kind match {
647+
case "pos" => runTestCommon(true)
648+
case "ant" => runAntTest()
649+
case "res" => runResidentTest()
650+
case "scalap" => runScalapTest()
651+
case "script" => runScriptTest()
652+
case _ => runTestCommon(execTest(outDir, logFile) && diffIsOk)
653+
}
657654

655+
try runWhatever()
656+
catch {
657+
case _: InterruptedException => lastState = genTimeout()
658+
case t: Throwable => lastState = genCrash(t)
659+
}
658660
lastState
659661
}
660662

@@ -731,7 +733,7 @@ class SuiteRunner(
731733
val javaOpts: String = PartestDefaults.javaOpts,
732734
val scalacOpts: String = PartestDefaults.scalacOpts) {
733735

734-
import PartestDefaults.{ numThreads, waitTime }
736+
import PartestDefaults.{numThreads, waitTime}
735737

736738
setUncaughtHandler
737739

@@ -770,47 +772,47 @@ class SuiteRunner(
770772
if (failed && !runner.logFile.canRead)
771773
runner.genPass()
772774
else {
773-
val (state, _) =
774-
try timed(runner.run())
775-
catch {
776-
case t: Throwable => throw new RuntimeException(s"Error running $testFile", t)
777-
}
775+
val (state, time) = timed(runner.run())
776+
nestUI.verbose(s"$testFile completed in $time")
778777
nestUI.reportTest(state, runner)
779778
runner.cleanup()
780779
state
781780
}
782781
onFinishTest(testFile, state)
783782
}
784783

785-
def runTestsForFiles(kindFiles: Array[File]): Array[TestState] = {
784+
def runTestsForFiles(kindFiles: Array[File]): Either[(Exception, Array[TestState]), Array[TestState]] = {
786785
nestUI.resetTestNumber(kindFiles.size)
787786

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

791790
pool.shutdown()
792-
Try (pool.awaitTermination(waitTime) {
793-
throw TimeoutException(waitTime)
794-
}) match {
795-
case Success(_) => futures map (_.get)
796-
case Failure(e) =>
791+
Try (pool.awaitTermination(waitTime)(throw TimeoutException(waitTime))) match {
792+
case Success(_) => Right(futures.map(_.get))
793+
case Failure(e: Exception) =>
797794
e match {
798795
case TimeoutException(_) =>
799-
nestUI.warning("Thread pool timeout elapsed before all tests were complete!")
796+
nestUI.echoWarning("Thread pool timeout elapsed before all tests were complete!")
800797
case ie: InterruptedException =>
801-
nestUI.warning("Thread pool was interrupted")
798+
nestUI.echoWarning("Thread pool was interrupted")
802799
ie.printStackTrace()
803800
}
804-
pool.shutdownNow() // little point in continuing
805-
// try to get as many completions as possible, in case someone cares
806-
val results = for (f <- futures) yield {
801+
pool.shutdownNow()
802+
val states = (kindFiles, futures).zipped.map {(testFile, future) =>
807803
try {
808-
Some(f.get(0, NANOSECONDS))
804+
if (future.isDone) future.get(0, NANOSECONDS) else {
805+
val s = Uninitialized(testFile)
806+
onFinishTest(testFile, s) // reported as pending
807+
s
808+
}
809809
} catch {
810-
case _: Throwable => None
810+
case e: ExecutionException => new Runner(testFile, this, nestUI).genCrash(e.getCause)
811+
case _: Throwable => new Runner(testFile, this, nestUI).genTimeout()
811812
}
812813
}
813-
results.flatten
814+
Left((e, states))
815+
case Failure(t) => throw t
814816
}
815817
}
816818

src/main/scala/scala/tools/partest/package.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ package object partest {
104104

105105
implicit class ExecutorOps(val executor: ExecutorService) {
106106
def awaitTermination[A](wait: Duration)(failing: => A = ()): Option[A] = (
107-
if (executor awaitTermination (wait.length, wait.unit)) None
107+
if (executor.awaitTermination(wait.length, wait.unit)) None
108108
else Some(failing)
109109
)
110110
}

0 commit comments

Comments
 (0)