sbt reports success when forked test suite fails with an exception

[smarter]

See https://scala-ci.typesafe.com/job/dotty-master-validate-junit/1000/console for an example:

Internal error when running tests: java.lang.OutOfMemoryError: Java heap space
Exception in thread "Thread-7" java.io.EOFException
    at java.io.ObjectInputStream$BlockDataInputStream.peekByte(ObjectInputStream.java:2601)
    at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1319)
    at java.io.ObjectInputStream.readArray(ObjectInputStream.java:1707)
    at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1345)
    at java.io.ObjectInputStream.readArray(ObjectInputStream.java:1707)
    at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1345)
    at java.io.ObjectInputStream.readArray(ObjectInputStream.java:1707)
    at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1345)
    at java.io.ObjectInputStream.readArray(ObjectInputStream.java:1707)
    at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1345)
    at java.io.ObjectInputStream.readArray(ObjectInputStream.java:1707)
    at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1345)
    at java.io.ObjectInputStream.readArray(ObjectInputStream.java:1707)
    at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1345)
    at java.io.ObjectInputStream.readArray(ObjectInputStream.java:1707)
    at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1345)
    at java.io.ObjectInputStream.readArray(ObjectInputStream.java:1707)
    at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1345)
    at java.io.ObjectInputStream.readObject(ObjectInputStream.java:371)
    at sbt.React.react(ForkTests.scala:114)
    at sbt.ForkTests$$anonfun$mainTestTask$1$Acceptor$2$.run(ForkTests.scala:74)
    at java.lang.Thread.run(Thread.java:745)
[info] Passed: Total 8, Failed 0, Errors 0, Passed 8
[success] Total time: 1721 s, completed Feb 4, 2016 8:53:52 PM

[SethTisue]

this makes me a bit nervous about switching scala/scala PR validation from ant to sbt, since sbt will be running some tests forked. did you do something special to work around this for Dotty?

[smarter]

No, I just fixed the underlying issue that led to all these OutOfMemoryErrors. This looks like something that can only really be fixed in sbt itself.

[smarter]

Note that this should be pretty rare: it happens when the test suite itself fails with an exception, not when a test fails with an exception.

[eed3si9n]

There was similar report with ScalaTest #1890 / scalatest/scalatest#432.
Not sure what they did (maybe scalatest/scalatest@a3cf999) but ScalaTest 2.2.6 apparently fixed it.

[smarter]

SBT does not deal with exceptions in threads correctly, see:

sbt/main/actions/src/main/scala/sbt/ForkTests.scala

Lines 39 to 79 in a92e509

	object Acceptor extends Runnable {
	val resultsAcc = mutable.Map.empty[String, SuiteResult]
	lazy val result = TestOutput(overall(resultsAcc.values.map(_.result)), resultsAcc.toMap, Iterable.empty)
	def run() {
	val socket =
	try {
	server.accept()
	} catch {
	case e: java.net.SocketException =>
	log.error("Could not accept connection from test agent: " + e.getClass + ": " + e.getMessage)
	log.trace(e)
	server.close()
	return
	}
	val os = new ObjectOutputStream(socket.getOutputStream)
	// Must flush the header that the constructor writes, otherwise the ObjectInputStream on the other end may block indefinitely
	os.flush()
	val is = new ObjectInputStream(socket.getInputStream)
	try {
	val config = new ForkConfiguration(log.ansiCodesSupported, parallel)
	os.writeObject(config)
	val taskdefs = opts.tests.map(t => new TaskDef(t.name, forkFingerprint(t.fingerprint), t.explicitlySpecified, t.selectors))
	os.writeObject(taskdefs.toArray)
	os.writeInt(runners.size)
	for ((testFramework, mainRunner) <- runners) {
	os.writeObject(testFramework.implClassNames.toArray)
	os.writeObject(mainRunner.args)
	os.writeObject(mainRunner.remoteArgs)
	}
	os.flush()
	new React(is, os, log, opts.testListeners, resultsAcc).react()
	} finally {
	is.close(); os.close(); socket.close()
	}
	}
	}

Acceptor is an instance of Runnable, in its run method you do not catch any exception, which mean that if one occurs, the thread dies, but nothing will be reported to the caller because you're just using Thread#run and Thread#join which don't report anything about how a Thread died.
I don't know much about Java concurrency but I guess you could use https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.UncaughtExceptionHandler.html

[parkan]

Seeing this with copycat + sbt as well

https://circleci.com/gh/mediachain/mediachain/48

Definitely doesn't seem like the correct behavior

[kevinmeredith]

but ScalaTest 2.2.6 apparently fixed it.

Does this gist indicate its success, @eed3si9n?

[eed3si9n]

@kevinmeredith Reading this issue over today, I now think #1890 is unrelated.
As @smarter wrote:

it happens when the test suite itself fails with an exception, not when a test fails with an exception.

[dwijnand]

Fixed in #2730 by @eed3si9n